Swizzle it just a little bit

[tony--]

Fixes #179

This approach amounts to a simplified version of hackishlyHidesInputAccessoryView, but removes all instances of the string UIWebBrowserView (as I believe this string is what was causing the App Store rejections).

This version retains classname checking but only checks for the "UIWeb" prefix in order to avoid referencing the string UIWebBrowserView. The classname check is needed to avoid swizzling every view in webView.scrollView.subviews (originally I thought I would be able to limit this via respondsToSelector: @selector(inputAccessoryView), but then I realized that every UIView responds to @selector(inputAccessoryView)).

According to this SO post, this classname check will not cause problems with app store.
However, if it is still a concern, the classname check could be removed but this might have a performance impact on some apps that have a lot of subviews (e.g., apps with a lot of images).

[tlancina]

Thanks for the PR! I'm wondering if just replacing the inputAccessoryView method on the existing UIWebBrowserView class is less intrusive that replacing the class entirely? Looking at something like https://github.com/cjpearson/cordova-plugin-keyboard/blob/master/src/ios/CDVKeyboard.m#L118-L146, I feel like it's less code and fails pretty gracefully. Also it's potentially less of a red flag to Apple if at runtime the class itself doesn't change, although from what I can tell they are just searching for string matches.

In the interest of transparency it might make sense to pull this out into its own plugin. So if you do want to hide the bar, you are explicitly opting in to potential future rejection, considering there is no way to do this with public APIs at the moment. Any thoughts?

[tony--]

I thought about just replacing the method, but wasn't sure how to do it. Now that I see the example, it looks pretty straightforward! Personally, I don't think it will be less of a red flag - I think the rejections are based on string matching, but I like that it is simpler.

I see where you are coming from in wanting to shield users from possible rejection. Personally, I think avoiding strings that match non-public API and class names will be sufficient to avoid rejections, but isolating this functionality might be reassuring for some users.

Feel free to close this if you want to go with cjpearson's simpler approach and/or want to pull this out into a separate plugin.
Also let me know if you'd like me to update this PR to swizzle in the method only (instead of swapping classes).

[tlancina]

Yeah the reason I thought there might be more than just string matching was because there were no reports (that I know of) of the other plugin, which had the same string, causing rejections. But I suppose that could be explained by the difference in usage numbers.

I'll admit to probably overthinking this. If apps were to start getting rejected again (meaning they are detecting it with more than just string matching) I think we would be justified in asking our users to open Radars to prod Apple into exposing, at a minimum, inputAccessoryView for UI/WKWebView.

If you don't mind updating the PR to only swizzle the method I'll merge it in and release 2.2.0. Thanks!

On Tue, May 3, 2016 at 12:06 PM Tony Homer notifications@github.com wrote:

I thought about just replacing the method, but wasn't sure how to do it. Now that I see the example, it looks pretty straightforward! Personally, I don't think it will be less of a red flag - I think the rejections are based on string matching, but I like that it is simpler.

I see where you are coming from in wanting to shield users from possible rejection. Personally, I think avoiding strings that match non-public API and class names will be sufficient to avoid rejections, but isolating this functionality might be reassuring for some users.

Feel free to close this if you want to go with cjpearson's simpler approach and/or want to pull this out into a separate plugin.

Also let me know if you'd like me to update this PR to swizzle in the method only (instead of swapping classes).

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/driftyco/ionic-plugin-keyboard/pull/197#issuecomment-216597220

[tony--]

The one thing I don't like about this approach is the inclusion of the obfuscated class name strings. I experimented with some other approaches, but they all had side-effects. In the end I came back to this approach, including the obfuscated class name strings, but I tweaked it a bit to fix some potential issues and/or (in my view) clean it up.
Specifically, I moved the static IMP references to member variables (and added Method member variables in order to avoid looking them up multiple times) and I moved the initialization to pluginInitialize. I wanted to avoid the possibility of overwriting the original IMP on multiple calls to hideKeyboardAccessoryBar.

[jacquesdev]

Hi @tony-- and @tlancina, really sorry to be such a pain, but I just wanted to check if you can resolve the conflict and merge it with 2.2.0? I am only asking because it seems the fix is imminent, and I would love to get this back in to our app.

[tony--]

@jacquesdev Thanks for the comment! I merged the recent changes from master into my PR branch to resolve the conflict.

[tlancina]

@tony-- looks great! Could you remove the swizzled const? I think it's a relic of the first implementation. Or if it's a hassle I can merge it in right now and can remove it as well.

@jacquesdev for future reference you can install any plugin from github at any time: cordova plugin add https://github.com/tony--/ionic-plugin-keyboard.git#swizzle-it-just-a-little-bit.

[tony--]

@tlancina - right you are, sorry about that! I removed it; also removed the commented out references to UIWebViewExtension from plugin.xml.

[tlancina]

@tony-- thank you for your contribution and patience!

[tony--]

@tlancina glad to have an opportunity to help out a bit!

[jacquesdev]

Thanks to both of you much appreciated :)