Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify a new document's URL #4205

Merged
merged 3 commits into from
Dec 18, 2018
Merged

Conversation

jyasskin
Copy link
Member

@jyasskin jyasskin commented Nov 26, 2018

The "URL that was originally to be fetched" isn't clear about which URL
that actually is.

I believe the reasonable choices here are the following, of which I'm guessing browsers do the third, but I'm not very confident I've got this right:

  • The request's URL: The original URL before any redirects.
  • The request's current URL: The final URL after any redirects, and
    treating a Service Worker's response as from that URL.
  • The response's URL: Either the final URL after any redirects, or, if a
    Service Worker intercepted the fetch and returned a response from a
    different URL, that different URL.

Fixes #3953.

I suspect this is tested in the Service Worker tests, but I haven't yet found the specific test. I'm sending this PR anyway in case you already know it is or isn't tested.


/browsing-the-web.html ( diff )
/infrastructure.html ( diff )

@annevk
Copy link
Member

annevk commented Nov 27, 2018

Yeah, I think that's correct. Note also that request can be null, e.g., for srcdoc, so that those wouldn't work universally anyway.

@wanderview usually has good insights on this so copying to get this checked twice.

@wanderview
Copy link
Member

wanderview commented Nov 27, 2018

I'm pretty sure we do not use the service worker final response URL for the document URL. This was explicitly designed to allow a separate "offline" page to be loaded by service worker without changing the URL bar.

So I think its the second option, maybe?

@annevk
Copy link
Member

annevk commented Nov 27, 2018

Oooh right, it was different for navigations, indeed. Thanks! So yeah, it's the final request URL, but that also means there needs to be accounting here for callers such as srcdoc (and javascript: maybe).

@wanderview
Copy link
Member

wanderview commented Nov 27, 2018

I would expect the document URL to remain about:srcdoc, about:blank, javascript: etc when a service worker controller is inherited from the parent.

@jyasskin
Copy link
Member Author

@wanderview, do you know where the SW behavior is or should be tested? service-workers/service-worker/navigation-redirect.https.html looks like where I'd expect, but nothing there uses sw=fetch-url.

mfalken pushed a commit to web-platform-tests/wpt that referenced this pull request Nov 30, 2018
…14288)

The resulting document's location should be the originally requested
URL. See also whatwg/html#4205
@jyasskin
Copy link
Member Author

I've updated the change to use the "request's current URL", and the SW implication is now tested. srcdocs seem to be covered by https://github.com/web-platform-tests/wpt/blob/master/html/browsers/history/the-location-interface/location_hash.html#L24. javascript: URLs are mostly covered by https://github.com/web-platform-tests/wpt/blob/master/html/semantics/embedded-content/the-iframe-element/iframe_javascript_url_01.htm, and I'm adding a test for one more case in web-platform-tests/wpt#14316.

@jyasskin
Copy link
Member Author

jyasskin commented Nov 30, 2018

The behavior of javascript: URLs isn't spec'ed correctly... In https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate, the 'If resource is a request whose url's scheme is "javascript"' steps initialize a variable named address to the iframe's old address, but that variable's never used. With this PR, I think we get Chrome's result by appending address to request's URL list, and I think we get Firefox's by doing nothing. What would you like me to spec?

@jyasskin
Copy link
Member Author

The use of address here seems to have gone away in https://github.com/whatwg/html/pull/3946/files#diff-36cd38f49b9afa08222c0dc9ebfe35ebL82043, which led to @TimothyGu filing #3953, which this PR is trying to fix. :)

@domenic
Copy link
Member

domenic commented Nov 30, 2018

Probably also related: web-platform-tests/wpt#14262

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

This looks like a strict improvement to me from the current spec. @annevk, do you agree?

@annevk
Copy link
Member

annevk commented Dec 11, 2018

Yeah, thanks!

@domenic
Copy link
Member

domenic commented Dec 11, 2018

Oh wait, upon re-reading I realized we didn't actually answer @jyasskin's question about javascript: URLs:

With this PR, I think we get Chrome's result by appending address to request's URL list, and I think we get Firefox's by doing nothing. What would you like me to spec?

Based on the discussions in #1565 and https://bugzilla.mozilla.org/show_bug.cgi?id=1281375#c7 it sounds like Chrome's behavior is probably more web-compatible... Let's do that?

The "URL that was originally to be fetched" isn't clear about which URL
that actually is.

The choices here are:

* The request's URL: The original URL before any redirects.
* The request's current URL: The final URL after any redirects, and
  treating a Service Worker's response as from that URL.
* The response's URL: Either the final URL after any redirects, or, if a
  Service Worker intercepted the fetch and returned a response from a
  different URL, that different URL.

Fixes whatwg#3953.
Per wanderview's comment. On a null request, I just fall back to the
response's URL. I still haven't located or written web-platform-tests
for this.
@jyasskin
Copy link
Member Author

@domenic @annevk, I've specified what I think is Chrome's behavior in the most recent change.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 14, 2018
… a different URL, a=testonly

Automatic update from web-platform-tests
Test navigation when a SW replies with a fetch from a different URL (#14288)

The resulting document's location should be the originally requested
URL. See also whatwg/html#4205
--

wpt-commits: 38eec61c5e43b9757351d63c55205fa2d8d3bb2d
wpt-pr: 14288
mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Dec 15, 2018
… a different URL, a=testonly

Automatic update from web-platform-tests
Test navigation when a SW replies with a fetch from a different URL (#14288)

The resulting document's location should be the originally requested
URL. See also whatwg/html#4205
--

wpt-commits: 38eec61c5e43b9757351d63c55205fa2d8d3bb2d
wpt-pr: 14288
@domenic domenic merged commit f2c7ea3 into whatwg:master Dec 18, 2018
domenic pushed a commit to web-platform-tests/wpt that referenced this pull request Dec 18, 2018
The document.URL should be about:blank.

Follows whatwg/html#4205.
@domenic
Copy link
Member

domenic commented Dec 18, 2018

@jyasskin jyasskin deleted the specify-doc-url branch December 18, 2018 22:53
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 5, 2019
…ript:...">, a=testonly

Automatic update from web-platform-tests
Test document.URL of <iframe src="javascript:...">

The document.URL should be about:blank.

Follows whatwg/html#4205.
--

wpt-commits: ed74c4f67f0529a7439f7b656ef0a35049873b9e
wpt-pr: 14316
mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Feb 6, 2019
…ript:...">, a=testonly

Automatic update from web-platform-tests
Test document.URL of <iframe src="javascript:...">

The document.URL should be about:blank.

Follows whatwg/html#4205.
--

wpt-commits: ed74c4f67f0529a7439f7b656ef0a35049873b9e
wpt-pr: 14316
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 7, 2019
…ript:...">, a=testonly

Automatic update from web-platform-tests
Test document.URL of <iframe src="javascript:...">

The document.URL should be about:blank.

Follows whatwg/html#4205.
--

wpt-commits: ed74c4f67f0529a7439f7b656ef0a35049873b9e
wpt-pr: 14316
mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Feb 8, 2019
…ript:...">, a=testonly

Automatic update from web-platform-tests
Test document.URL of <iframe src="javascript:...">

The document.URL should be about:blank.

Follows whatwg/html#4205.
--

wpt-commits: ed74c4f67f0529a7439f7b656ef0a35049873b9e
wpt-pr: 14316
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
… a different URL, a=testonly

Automatic update from web-platform-tests
Test navigation when a SW replies with a fetch from a different URL (#14288)

The resulting document's location should be the originally requested
URL. See also whatwg/html#4205
--

wpt-commits: 38eec61c5e43b9757351d63c55205fa2d8d3bb2d
wpt-pr: 14288

UltraBlame original commit: 44cd01e40fe7de4a055a200dcec4c7c40102dba7
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
… a different URL, a=testonly

Automatic update from web-platform-tests
Test navigation when a SW replies with a fetch from a different URL (#14288)

The resulting document's location should be the originally requested
URL. See also whatwg/html#4205
--

wpt-commits: 38eec61c5e43b9757351d63c55205fa2d8d3bb2d
wpt-pr: 14288

UltraBlame original commit: 44cd01e40fe7de4a055a200dcec4c7c40102dba7
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
… a different URL, a=testonly

Automatic update from web-platform-tests
Test navigation when a SW replies with a fetch from a different URL (#14288)

The resulting document's location should be the originally requested
URL. See also whatwg/html#4205
--

wpt-commits: 38eec61c5e43b9757351d63c55205fa2d8d3bb2d
wpt-pr: 14288

UltraBlame original commit: 44cd01e40fe7de4a055a200dcec4c7c40102dba7
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
…ript:...">, a=testonly

Automatic update from web-platform-tests
Test document.URL of <iframe src="javascript:...">

The document.URL should be about:blank.

Follows whatwg/html#4205.
--

wpt-commits: ed74c4f67f0529a7439f7b656ef0a35049873b9e
wpt-pr: 14316

UltraBlame original commit: 5a41d52b6c385a2dfb4d9d50fb5023561363e824
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
…ript:...">, a=testonly

Automatic update from web-platform-tests
Test document.URL of <iframe src="javascript:...">

The document.URL should be about:blank.

Follows whatwg/html#4205.
--

wpt-commits: ed74c4f67f0529a7439f7b656ef0a35049873b9e
wpt-pr: 14316

UltraBlame original commit: 6f5b6857d05659dddc0fa555824569fa4f5f52b5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
…ript:...">, a=testonly

Automatic update from web-platform-tests
Test document.URL of <iframe src="javascript:...">

The document.URL should be about:blank.

Follows whatwg/html#4205.
--

wpt-commits: ed74c4f67f0529a7439f7b656ef0a35049873b9e
wpt-pr: 14316

UltraBlame original commit: 5a41d52b6c385a2dfb4d9d50fb5023561363e824
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
…ript:...">, a=testonly

Automatic update from web-platform-tests
Test document.URL of <iframe src="javascript:...">

The document.URL should be about:blank.

Follows whatwg/html#4205.
--

wpt-commits: ed74c4f67f0529a7439f7b656ef0a35049873b9e
wpt-pr: 14316

UltraBlame original commit: 6f5b6857d05659dddc0fa555824569fa4f5f52b5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…ript:...">, a=testonly

Automatic update from web-platform-tests
Test document.URL of <iframe src="javascript:...">

The document.URL should be about:blank.

Follows whatwg/html#4205.
--

wpt-commits: ed74c4f67f0529a7439f7b656ef0a35049873b9e
wpt-pr: 14316

UltraBlame original commit: 5a41d52b6c385a2dfb4d9d50fb5023561363e824
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 4, 2019
…ript:...">, a=testonly

Automatic update from web-platform-tests
Test document.URL of <iframe src="javascript:...">

The document.URL should be about:blank.

Follows whatwg/html#4205.
--

wpt-commits: ed74c4f67f0529a7439f7b656ef0a35049873b9e
wpt-pr: 14316

UltraBlame original commit: 6f5b6857d05659dddc0fa555824569fa4f5f52b5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants