Permalink
Browse files

Android WebView: Only fire onLoadStart for toplevel page loads

Summary:
Currently, `onLoadStart` fires for a couple of cases:
  1. toplevel page loads (e.g. initial page load, clicking links)
  2. loading of pages within an iframe

The fact that `onLoadStart` fires for case (2) causes some problems. For example, it makes it difficult for the code that uses the WebView to know what URL the WebView is currently rendering. This is because the listener can't distinguish between the toplevel URL and the URL of an iframe. Additionally, this behavior is inconsistent with the behavior on iOS. On iOS, `onLoadStart` only fires for toplevel page loads.

To fix these issues, this change deletes the `doUpdateVisitedHistory` handler so that `onLoadStart` only fires for case (1).

**Test Plan**

Created a test page that has an iframe and loaded it in the WebView. Verified that `onLoadStart` only fires for toplevel page loads.

Adam Comella
Microsoft Corp.
Closes #15554

Differential Revision: D5665979

Pulled By: hramos

fbshipit-source-id: a52e473bc5691a6e180f45f0728e4ad89a7d354f
  • Loading branch information...
rigdern authored and facebook-github-bot committed Aug 19, 2017
1 parent 62a4912 commit ffbd3db61bce21b9332f7783086fce703706031f
@@ -189,17 +189,6 @@ public void onReceivedError(
new TopLoadingErrorEvent(webView.getId(), eventData));
}
@Override
public void doUpdateVisitedHistory(WebView webView, String url, boolean isReload) {
super.doUpdateVisitedHistory(webView, url, isReload);
dispatchEvent(
webView,
new TopLoadingStartEvent(
webView.getId(),
createWebViewEvent(webView, url)));
}
protected void emitFinishEvent(WebView webView, String url) {
dispatchEvent(
webView,

1 comment on commit ffbd3db

@ksegla

This comment has been minimized.

Show comment
Hide comment
@ksegla

ksegla Mar 19, 2018

This fix broke the detection of url from Google AMP pages (and tons of other pages). It was working fine on 0.48.4. Rather than deleting the code, it would be better to have some kind of boolean that would let developers decide what they want from their WebView. I'm looking into it but not sure how to do it yet.

ksegla commented on ffbd3db Mar 19, 2018

This fix broke the detection of url from Google AMP pages (and tons of other pages). It was working fine on 0.48.4. Rather than deleting the code, it would be better to have some kind of boolean that would let developers decide what they want from their WebView. I'm looking into it but not sure how to do it yet.

Please sign in to comment.