Skip to content

Commit

Permalink
Android WebView: Only fire onLoadStart for toplevel page loads
Browse files Browse the repository at this point in the history
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 ffbd3db
Showing 1 changed file with 0 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

1 comment on commit ffbd3db

@ksegla
Copy link

@ksegla ksegla commented on ffbd3db Mar 19, 2018

Choose a reason for hiding this comment

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

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.