New issue
Advanced search Search tips

Issue 848305 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug

Blocking:
issue 669885



Sign in to add a comment

WebHistoryItem.getUrl gives incorrect url for page loaded with loadDataWithBaseUrl

Project Member Reported by tsergeant@chromium.org, May 31 2018

Issue description

Device name: Nexus 5X
Android version: 8.1
WebView version:
    Valid package com.android.chrome (versionName: 66.0.3359.158, versionCode: 335915852, targetSdkVersion: 27) is  installed/enabled for all users
    Valid package com.google.android.webview (versionName: 61.0.3163.98, versionCode: 316409800, targetSdkVersion: 27) is NOT installed/enabled for all users

Application: I've made a minimal repro app, see below

Steps to reproduce:
(1) Load a page into the WebView with WebView.loadDataWithBaseUrl
(2) Later, call WebView.copyBackForwardList().getCurrentItem().getUrl()

Expected result:

The URL returned from getUrl should be the "historyUrl" given to loadDataWithBaseUrl

Actual result:

The URL returned from getUrl is always data:text/html;charset=utf-8;base64,

Note that the WebView history still works: when I call goBack and goForward, WebViewClient methods still get called with the correct URL.

Why does this matter?:

I'm trying to write an espresso test of a page loaded with loadDataWithBaseUrl, and espresso waits until the history url matches the result of WebView#getUrl() before executing any actions (see https://android.googlesource.com/platform/frameworks/testing/+/android-support-test/espresso/web/src/main/java/android/support/test/espresso/web/action/JavascriptEvaluation.java#214).

Sample application:

I wrote a minimal app to exercise this bug.
Activity file: https://gist.github.com/tgsergeant/097410e69209506ac5c502979ba9763a
APK: https://drive.google.com/file/d/19gRDIzCNIG0WNorEk_oB-s4ZioZRF6xG/view?usp=sharing

When you run this app and click the FAB button you can see the output from the logcat:
05-31 16:16:53.096 25335-25335/webview.example I/WEBVIEW: Reported Url: https://www.example.com/
05-31 16:16:53.096 25335-25335/webview.example I/WEBVIEW: History url: data:text/html;charset=utf-8;base64,
 

Comment 1 by boliu@chromium.org, May 31 2018

We are planning on deprecating historyUrl and just ignore it at some point in the future. See crbug.com/669885 for the history of that discussion
I don't think this bug is necessarily to do with historyUrl.

If I do

  loadDataWithBaseURL(baseUrl, "<h1>Hello</h1>", "text/html", "UTF-8", null)

then the behavior is still exactly the same. Some things are clearly treating baseUrl as the URL of the page, but WebHistoryItem does not.

Comment 3 by boliu@chromium.org, May 31 2018

True. empty data url is coming from webview's internal "optimization" to avoid parsing and sending data url as a GURL, which turns out is expensive and has certain.. limitations. Shouldn't have let that implementation detail leak out
Cc: torne@chromium.org ntfschr@chromium.org boliu@chromium.org jamwalla@chromium.org
Now that we've deprecated historyUrl, what do we want the behavior to actually be? Do we want to leave as-is? Or, should we use baseUrl for the history entry?

IMO using baseUrl seems like the right choice.

Comment 5 by boliu@chromium.org, Jun 1 2018

all APIs should use baseUrl
Cc: -jamwalla@chromium.org
Blocking: 669885
Owner: changwan@chromium.org
It looks like we haven't deprecated historyUrl yet. To the best of my knowledge (correct me if I am wrong), we need:

1) Deprecate the historyUrl parameter.
2) Use baseUrl for the history entry.
3) Update the documentation of loadDataWithBaseUrl().

Unfortunately James left the team, passing to Changwan for deciding the owner as 669885 was assigned to Changwan.

Mark this issue blocking issue 669885, issue 669885 is only for tracking (3).
Status: Assigned (was: Unconfirmed)
Instead of deprecating the API, I think we should write a wrapper in AndroidX. Although I think getting rid of historyUrl is the right move, I'm still concerned loadData() is a hard API to get right (callers forget to encode data).

I wrote up a design a couple months back, but haven't circulated this yet with the team: https://docs.google.com/document/d/1yLnsvCQaOAKpHfL2xejQmtZODY66cyk1Ekk33StwrfA/edit?usp=sharing

IMO we should sync with the Chrome for iOS team to get their feedback on the WkWebView APIs this is inspired by.

Sign in to add a comment