[Android][WebView] onShouldStartLoadWithRequest callback by pglotov · Pull Request #6478 · react/react-native

pglotov · 2016-03-16T03:11:43Z

This PR implements onShouldStartLoadWithRequest callback for WebView on Android. Similar to iOS approach in PR#3643.

facebook-github-bot · 2016-03-16T03:12:26Z

By analyzing the blame information on this pull request, we identified @mkonicek, @dmmiller and @korDen to be potential reviewers.

facebook-github-bot · 2016-03-16T03:16:18Z

@pglotov updated the pull request.

jordaaash · 2016-03-16T03:17:40Z

I love you so much right now.

ide · 2016-03-16T04:47:31Z

cc @astreet this PR introduces synchronous JS execution by allowing the UI thread to invoke JSC. CCing you since you wrote the Worker implementation. Based on my experience with multicore JSC, calling into the same JSContext from two threads is going to introduce race conditions within JS itself since JSC will try to concurrently interleave two separate JS calls.

@pglotov I don't think this approach is going to work. If the WebView method needs to be synchronous, you should queue up the JS command to be run on the JS thread and then block the UI thread until the JS command is processed.

eslint-bot · 2016-03-16T05:09:58Z

no-extra-bind: The function binding is unnecessary.

pglotov · 2016-03-16T05:10:36Z

@ide Actually it synchronously calls Java from JS thread. UI thread sends an (async) event to JS and blocks. JS processes the event and posts result into Java object (synchronously), unblocking UI thread. Race conditions in Java world can be dealt with.

dmmiller · 2016-03-16T10:49:54Z

Can you look at how iOS does this in WebView.ios.js? Also, would be nice if it had the same property name as in ios as well. Have you checked how this works for back/forward and history? Also shouldOverrideUrlLoading is not called in all instances. For instance, not on the initial load, nor when you explicitly tell the WebView to load a URL, only on link clicks which may be good enough.

pglotov · 2016-03-16T14:37:54Z

@dmmiller Yes I looked at iOS onShouldStartLoadWithRequest before implementing this, and tried to follow that approach. No I didn't check back/forward. I need it to intercept link clicks in webview and open them in browser instead. Makes sense to have a single property name for both platforms.

facebook-github-bot · 2016-03-16T15:53:35Z

@pglotov updated the pull request.

eslint-bot · 2016-03-16T16:08:16Z

+ UIManager.dispatchViewManagerCommandSync(
+ this.getWebViewHandle(),
+ UIManager.RCTWebView.Commands.shouldOverrideWithResult,
+ [shouldOverride]


no-extra-bind: The function binding is unnecessary.

ide · 2016-03-16T20:31:26Z

@pglotov ah sorry I must have misread what is going on

facebook-github-bot · 2016-03-17T00:51:18Z

@pglotov updated the pull request.

facebook-github-bot · 2016-03-17T00:59:18Z

@pglotov updated the pull request.

facebook-github-bot · 2016-03-17T01:00:24Z

@pglotov updated the pull request.

pglotov · 2016-03-17T01:01:09Z

@davidaurelio Thanks, made changes.

davidaurelio · 2016-03-17T13:37:16Z

great, thank you!

dmmiller · 2016-03-17T15:40:55Z

Thanks for cleaning this, but this is really orthagonal. Want to put up a quick PR to fix these and then I can accept and then this one will be easier to read?

Sure, will do it later today.

Created PR for path changes here.

dmmiller · 2016-03-17T15:43:28Z

This question of whether we want to allow synchronous communication this way is pretty big. I'd like @andreicoman11 and @astreet to weigh in on it.

younthu · 2017-01-03T16:41:40Z

Strong need this.

evanidul · 2017-02-01T15:40:17Z

+1

astreet · 2017-02-01T16:11:19Z

Like I mentioned above, I think a reasonable solution for the majority of cases would be to add a prop to WebView which is an array of regex's of urls to block synchronously. We would also want to add a new event for WebViews to be sent to JS when a URL is blocked this way so that JS can respond appropriately. Unfortunately, this isn't something we have a need for at FB so it's going to be up to an external contributor working on it, I can help review the code and offer advice.

younthu · 2017-02-02T16:35:56Z

@astreet , i just implemented it in our project with the regex prop way you described, i will try to extract those code out and send a pr to the FB, hope it help others. Please help to review it then, Thanks.

mattiashalldin · 2017-05-15T10:39:23Z

When will onShouldStartLoadWithRequest be available for Android?

zheng-liu-seattle · 2017-05-22T17:06:56Z

+1 need this.

coyer · 2017-05-27T02:53:23Z

+1 need this

caigehui · 2017-07-17T03:45:41Z

+1 need this

Livyli · 2017-08-15T10:17:22Z

+1 need this

ybolaris · 2017-09-26T17:39:54Z

+1

miguelocarvajal · 2017-10-03T17:01:23Z

+1

tourze · 2017-10-16T05:11:16Z

+1

kyangy · 2017-11-10T21:04:57Z

+1

stanleycyang · 2017-11-21T20:33:17Z

+1

0x5e · 2017-11-29T12:11:13Z

Without onShouldStartLoadWithRequest implemented on Android, the same method on iOS is useless. :-(

wesleymooiman · 2017-12-04T18:35:01Z

+1 we need this

MikePodgorniy · 2017-12-07T10:19:07Z

+1

punksta · 2018-01-08T09:30:11Z

+1

ghost · 2018-01-29T10:57:11Z

+1

shashankwadi · 2018-01-29T13:21:10Z

+1

pulkitsharma99 · 2018-01-29T13:21:12Z

+1

pramodinwadi · 2018-01-29T13:21:20Z

+1

akhilwadi · 2018-01-29T13:21:38Z

+1

manjeetwadi · 2018-01-29T13:21:58Z

+1

simranjeet-sawhney · 2018-01-29T13:26:55Z

+1

rschef · 2018-03-26T23:17:57Z

+1

prademak · 2018-03-27T13:40:11Z

+1

SanlinBlackball · 2018-04-19T17:45:59Z

+1

iamarkdev · 2018-05-08T22:50:44Z

In the meantime, there is a pending PR on the react-native-webview-android module with a working implementation of onShouldStartLoadWithRequest for Android here: lucasferreira/react-native-webview-android#98

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Mar 16, 2016

pglotov mentioned this pull request Mar 16, 2016
[WebView] Message passing between WebView / Javascript #586
Closed

pglotov force-pushed the android-shouldOverrideUrlLoading branch from 243054f to d9e336bCompare March 16, 2016 03:15

eslint-bot reviewed Mar 16, 2016
View reviewed changes

pglotov force-pushed the android-shouldOverrideUrlLoading branch from d9e336b to f497da8Compare March 16, 2016 15:50

pglotov changed the title ~~[Android][WebView] shouldOverrideUrlLoading callback~~[Android][WebView] onShouldStartLoadWithRequest callbackMar 16, 2016

eslint-bot reviewed Mar 16, 2016
View reviewed changes

pglotov force-pushed the android-shouldOverrideUrlLoading branch from f497da8 to 11385baCompare March 17, 2016 00:50

pglotov force-pushed the android-shouldOverrideUrlLoading branch from 11385ba to 39a04bbCompare March 17, 2016 00:58

pglotov force-pushed the android-shouldOverrideUrlLoading branch from 39a04bb to 9c4132aCompare March 17, 2016 01:00

dmmiller reviewed Mar 17, 2016
View reviewed changes

younthu mentioned this pull request Feb 3, 2017
Block url requests with regex prop and notify javascript about the ev… #12179
Closed

Uh oh!

Conversation

pglotov commented Mar 16, 2016

Uh oh!

facebook-github-bot commented Mar 16, 2016

Uh oh!

facebook-github-bot commented Mar 16, 2016

Uh oh!

jordaaash commented Mar 16, 2016

Uh oh!

ide commented Mar 16, 2016

Uh oh!

eslint-botMar 16, 2016

Choose a reason for hiding this comment

Uh oh!

eslint-botMar 16, 2016

Choose a reason for hiding this comment

Uh oh!

pglotov commented Mar 16, 2016

Uh oh!

dmmiller commented Mar 16, 2016

Uh oh!

pglotov commented Mar 16, 2016

Uh oh!

facebook-github-bot commented Mar 16, 2016

Uh oh!

eslint-botMar 16, 2016

Choose a reason for hiding this comment

Uh oh!

ide commented Mar 16, 2016

Uh oh!

facebook-github-bot commented Mar 17, 2016

Uh oh!

facebook-github-bot commented Mar 17, 2016

Uh oh!

facebook-github-bot commented Mar 17, 2016

Uh oh!

pglotov commented Mar 17, 2016

Uh oh!

davidaurelio commented Mar 17, 2016

Uh oh!

dmmillerMar 17, 2016

Choose a reason for hiding this comment

Uh oh!

pglotovMar 17, 2016

Choose a reason for hiding this comment

Uh oh!

pglotovMar 18, 2016

Choose a reason for hiding this comment

Uh oh!

dmmiller commented Mar 17, 2016

Uh oh!

younthu commented Jan 3, 2017

Uh oh!

evanidul commented Feb 1, 2017

Uh oh!

astreet commented Feb 1, 2017

Uh oh!

younthu commented Feb 2, 2017

Uh oh!

mattiashalldin commented May 15, 2017

Uh oh!

zheng-liu-seattle commented May 22, 2017

Uh oh!

coyer commented May 27, 2017

Uh oh!

caigehui commented Jul 17, 2017

Uh oh!

Livyli commented Aug 15, 2017

Uh oh!

ybolaris commented Sep 26, 2017

Uh oh!

miguelocarvajal commented Oct 3, 2017

Uh oh!

tourze commented Oct 16, 2017

Uh oh!

kyangy commented Nov 10, 2017

Uh oh!

stanleycyang commented Nov 21, 2017

Uh oh!

0x5e commented Nov 29, 2017•
edited
Loading

wesleymooiman commented Dec 4, 2017•
edited
Loading