Skip to content
This repository has been archived by the owner on Sep 25, 2021. It is now read-only.

Inject custom Javascript files #9

Closed

Conversation

joemasilotti
Copy link
Contributor

This PR enables developers to easily inject custom Javascript in to the WebView managed by Turbolinks.

All injection logic was first extracted to a new, public class, TurbolinksJavascriptInjector. This exposes the original method to inject the Turbolinks bridge and adds a new one. The new method takes in the path of the Javascript file to be injected.

As noted in the update to the README, this creates an opportunity to add Javascript event handlers. A direct use-case of this is to set custom HTTP headers for each Turbolinks request.

@Override
public void onReceiveValue(String s) {
if (turbolinksSession != null) {
turbolinksSession.turbolinksBridgeInjected = Boolean.parseBoolean(s);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block served a specific purpose of setting a flag in the TurbolinksSession, specific to the turbolinks_bridge.js being successfully injected. This would need to be set only for when that was injected properly.

Also, the caller of this method would probably want some kind of asynch callback to know its state, so you'd want a way to handle that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good catch, I missed the turbolinksBridgeInjected flag. I'll remove that when anything other than the Turbolinks bridge is injected.

And good point regarding the callback. I'll add that in as well.

@lateplate
Copy link
Contributor

Thanks for this @joemasilotti. Appreciate the enthusiasm to improve!

Overall I'd shy away from this change. Sorry, I know this could make it much easier to inject your own JS, but that's really not the intended job of this library.

Even though we are confident in the injection mechanism, it's still feels very hacky to have to build a "script injection format", run JS on the webview to inject it, then wait for an asynch evaluation to get a result. Unfortunately, this is the only workaround we have on Android (that I'm aware of).

So while I'm comfortable with how it works internally/private to the library, I wouldn't be so comfortable making it a public API that people begin to rely on and that we'd have to maintain. WebView is one of the more inconsistently behaving components of Android, so I am purposely cautious about any APIs we build on top of it, especially if they start to fall outside of our primary goals.

However, feel free to borrow/extract the relevant code that you need and drop it into your app -- it should work fine with TurbolinksSession.getWebView.evaluateJavascript().

Thanks again for continuing to try new ideas!

@joemasilotti
Copy link
Contributor Author

Thanks for the honest response. I understand your reluctance to add new functionality on WebView. Especially if it is at all similar to WKWebView on iOS.

The main reason for opening this PR was that evaluateJavascript() doesn't work for my use case. The end goal is to modify the XHR and add HTTP headers. To do so I need to hook in to the Turbolinks turbolinks:request-start event.

To my knowledge, this handler has to be set up as soon as the page finishes loading. Injecting the script via the existing Turbolinks interface is too late and the callbacks never fire. And because TurbolinksSession creates the web view internally, no hooks are publicly exposed to for a consumer to hook on and do it manually.

@lateplate
Copy link
Contributor

I think you should be able to use TurbolinksAdapter's onPageFinished callback to inject an event listener.

onPageFinished() happens (approximately) as the same time as we inject our bridge, which I think is the timing you're looking for -- right after the page's first full cold boot is considered completed by WebView.

Here's some more detail in the README.

@joemasilotti
Copy link
Contributor Author

Beautiful! Thanks for the insight, this is a much cleaner than the approach I proposed. I'm going to close this 😄

Let me know if you think there is anything worth adding to the README. I'm more than happy to document some of this.

@lateplate
Copy link
Contributor

Would be great if you wanted to expand the README a bit for that onPageFinished section.

Another (maybe better?) idea could be to create a wiki page so you have a little more room to write and point the README to that new wiki page.

Thanks!

@joemasilotti joemasilotti deleted the inject-custom-js-files branch September 2, 2016 13:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants