-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WKWebView Support #105
WKWebView Support #105
Conversation
Awesome. Thank you!!! It's too late for me to review this now, but I really look forward to merging. Awesome stuff dude! -- while mobile On Tue, Oct 14, 2014 at 11:49 PM, Loki Meyburg notifications@github.com
|
|
||
- (void)webView:(WKWebView *)webView didFinishNavigation:(WKNavigation *)navigation | ||
{ | ||
NSLog(@"1. Finished navigation"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's delete that little log message :)
Hi @lokimeyburg , I am only using WebViewJavascriptBridge for iOS development, so all my comments are limited to that scope. Our app is using a WebViewJavascriptBridge to run js business logic in a webview hidden from the user. Using WebViewJavascriptBridge with your branch has this working fine for me, but using WKWebViewJavascriptBridge is not working the same. Check out the most recent commit on these two branches: I'll keep trying to figure out what's wrong, but any idea quickly looking at my sample project? I think the issue might simply be my use of WebViewJavascriptBridge, but would be happy to have your input if you have a few minutes. Otherwise, the changes seem pretty straightforward. I want to make sure WKWebViewJavascriptBridge is truly a drop-in replacement for WebViewJavascriptBridge, though, and this sample project is showing it may not quite be that way... or that I am just abusing the bridge in bad ways 😄 Thanks! |
Hi @karlbecker, My goal is to have this be a drop-in replacement without having to change your JS. I have it working over on my own project, Stacker, on the |
I have a much bigger and more complicated project using it, too - private repo, though :) |
Note that you'll need to |
🆒 I'll try it out in a few hours. |
Woah, 🆗 , so here's what I've found: javascript To test try it out just add Otherwise, the method that should show the alert is most definitely being called in your code. To make sure I was able to get it to print out some debug logs. It's probably some simple |
I strongly recommend manipulation the dom to show log output for debugging over using alert. Since the alert UI is synchronous and halts the js execution thread, it adds unnecessary complexity with opaque consequences. The right place to test wkwebview additions is in the example app in the github repo. -- while mobile On Mon, Oct 27, 2014 at 9:16 PM, Loki Meyburg notifications@github.com
|
Yup, that's what I did. Here I'm using @karlbecker's project to print DOM elements to the screen to make sure the proper methods are being called. Otherwise, this pull request uses the example project in this repo to make sure everything works as expected (it looks like it does). |
(the |
So the question is ( but isn't the point of this pull request ) does anyone know how to make javascript |
Ah, because you need to implement the |
Thanks for the feedback! And nice find, @lokimeyburg - thanks! I'll test it out a little more, although I have a feeling this will work great now. |
I've been irresponsibly unresponsive here. I'm sorry guys. First off: This is a fantastic start! We have a good working reference implementation. Thanks for doing this. To pull it into the main repo I have 2 big asks, both for maintenance reasons.
I know this is a lot to ask. If we want WVJB to remain stable and strongly maintainable going forwards, I believe these are necessary improvements. I feel strongly about this, but am also very open to hearing dissenting opinions with alternative constructive suggestions. Cheers, and thanks again for taking the time to do this! |
Re: Here's why I thought it made it cluttered: Whenever you call something from the WebViewJavascriptBridge API, like say Also, the methods need to be split up a lot more because WKWebView uses callbacks when evaluating javascript. Compare: Personally, if maintainability is what you're after, I would still keep two separate files: WebViewJavascriptBridge and WKWebViewJavascriptBridge and move all the common snippets into a helper class called Either way, I'll take another stab at it this week/weekend and see where I get and we can make decision then. As for: |
Note that I like @lokimeyburg 's idea of moving common code into
Due to iOS 8's slow adoption, and the (pretty unlikely) potential for the two different js engines to produce different results on the same iOS version ( |
@lokimeyburg: Splitting into multiple files sounds great, as long as 1: there is very little code duplication, and 2: installation remains as simple as dragging a single WebViewJavascriptBridge folder into your project. @karlbecker: Yes; either that or make the iOS & OSX example apps both run UIWebView and WKWebView next to each other at the same time. I prefer the latter. |
Agreed on ease of installation - and of course, it will all work great with cocoapods (no more dragging folders around). @marcuswestin I also prefer just two apps, with the different webviews running side by side. For fun, could have basic javascript performance test to demonstrate how fast one is over the other 😄 |
Thanks for the feedback guys - I like where things are heading! I'll move duplicate code into a separate |
Looks good loki. iOS app held up well during some quick testing. Some quick notes: Nice work dude. |
I don't have a problem with that 😄 |
Great. As soon as there's a workable OS X demo I'm happy to take it from there. -- while mobile On Wed, Nov 12, 2014 at 4:30 PM, Loki Meyburg notifications@github.com
|
I'll be sure to review this, too (and hopefully contribute a little bit), but I have investigated some additional info about WKWebView lately, and unfortunately it integrates worse than UIWebView into other Cocoa networking APIs. So supporting both is definitely wise, since WKWebView is simply a no-go for some WebViewJavascriptBridge users (myself included), due to WKWebView not supporting NSURLProtocol, among other libraries. More info on StackOverflow, and here's an OpenRadar I would encourage us all to submit to Apple to encourage this pretty important integration. |
Waaaaat.... Ugh, apple, I love you, but sometimes... Sigh. Ok. Great research Karl. Thanks for the heads up. It would be super helpful if you could add a note about what you found in the readme to avoid that inevitable gotcha for some people, and have a place to point people when the inevitable related issues happen... -- while mobile On Fri, Nov 14, 2014 at 12:03 PM, Karl Becker notifications@github.com
|
You bet, I can add that in. |
Ugh - super lame. Lacking NSURLCache & NSURLProtocol support is a deal breaker for me when it comes to any apps in production. This is most likely because WKWebView is handled in another process (which is why a lot of the methods use callbacks & blocks). Thanks for the research @karlbecker! Here's a list of other 'gotchas' to watch out for - not sure how reliable the list it is but it's worth a scroll-through: https://github.com/ShingoFukuyama/WKWebViewTips/blob/master/README.md |
Great link @lokimeyburg - this one looks really hairy:
Will the change in this branch break anyone's submittal? |
I've...
|
@karlbecker Did you ever find out if that warning is valid?
|
@edgarchu - I haven't tried submitting an app with both UIWebView and WKWebView, so nope, not sure if Apple would reject, if it wouldn't work, or if it would just be a-ok! |
Any updates on this? Would love to use WebViewJavascriptBridge with WKWebView, and this merge request seems well worked trough. |
@alleus Just wanted to add that we've successfully launched a rather large app with both UIWebview and WKWebview without any issue. @marcuswestin what would it take to merge this PR? Make sure it works on iOS 9? |
👏 👏 👏 And no tricky techniques needed to hide that you used both, correct, @lokimeyburg ? |
Nope no tricks. It's pretty clear in the source code that we're implementing both. |
Sorry for the delay guys. Getting a startup off the ground can be pretty all-consuming :) |
Wanna take some credit for your hard work and add a PR with updates to README that notes support of WKWebView :) |
@lokimeyburg You're now a contributor - very well deserved! Feel free to act on behalf of WVBJ with the best interest of the project in mind :) Keep me in the loop on anything significant though please. Cheers, |
If you're interested in doing more right now, here's an interesting PR to consider: #133 |
Also looking interesting: #129 |
Thanks @marcuswestin! Glad to help! I'll be sure to keep things in order. I'll take a look at those PRs tomorrow. |
Awesome! |
And great job to everyone on an excellent, exciting PR! |
I've added support for WKWebView which was requested in issue #84.
How does it work:
WKWebView support is opt-in. In order to use WKWebView you need to instantiate the
WKWebViewJavascriptBridge
. The rest of theWKWebViewJavascriptBridge
API is the same asWebViewJavascriptBridge
.Where to start
Take a look at the
WKWebViewJavascriptBridge.m
file first as it has most of the logic. The two important differences are that the delegates for WKWebView have been changed and that the response for evaluating Javascript is now returned in a callback.How to test this
Pull down the branch and try to run the example application. Try running it in XCode 5 and 6 to test out iOS7 and iOS8 respectively. If you're using Pods, point to this branch and see if it works in one of your existing apps.
Extra
feature/wkwebkit
branch to make sure WebViewJavascriptBridge works as advertised.WebViewJavascriptBridge
andWKWebViewJavascriptBridge
however I think that should be left for another PR.