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

Set request's history-navigation flag for history traversal #3674

Merged
merged 3 commits into from
May 25, 2018

Conversation

yutakahirano
Copy link
Member

@yutakahirano yutakahirano commented May 8, 2018

See w3c/ServiceWorker#1167 for the discussion that led to this change and whatwg/fetch#718 for the infrastructure in Fetch this builds upon.

Tests: web-platform-tests/wpt#10909


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

@yutakahirano yutakahirano changed the title Set request's history-navigation flag for reloads Set request's history-navigation flag for history traversal May 8, 2018
@annevk annevk requested a review from domenic May 14, 2018 10:27
@domenic
Copy link
Member

domenic commented May 14, 2018

Editorially this looks fine, but I have a hard time telling if the step is inserted at the right place to cause the intended effect. For example, I'm unsure whether we want this flag set for all navigations to new HTML documents (which invoke "update the session history with the new page" and thus this algorithm), and I'm unsure whether we want to exclude setting it for bfcache navigations (i.e. cases where step 1 of this algorithm is not true).

@annevk, I thought you were more involved with this; can you help confirm?

@annevk
Copy link
Member

annevk commented May 15, 2018

Well, I'm still skeptical about the whole navigate algorithm being correct, but you all wanted to move ahead with these things so changes to HTML I'm trying not to say too much about.

@annevk
Copy link
Member

annevk commented May 15, 2018

(Those do sound like problems though.)

@yutakahirano
Copy link
Member Author

Thank you!

For example, I'm unsure whether we want this flag set for all navigations to new HTML documents (which invoke "update the session history with the new page" and thus this algorithm),

You're right. What I was trying to set the flag was requests via traverse the history by a delta, so I fixed the change.

and I'm unsure whether we want to exclude setting it for bfcache navigations (i.e. cases where step 1 of this algorithm is not true).

If entry holds a Document, Navigate algorithm is not called and service workers cannot see the request, right?

@yutakahirano
Copy link
Member Author

@domenic , what do you think about the latest change?

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.

Sorry for the delay; I had TC39 this week. The latest version looks good to me. I made further editorial tweaks around how the different flags to that algorithm are used; we don't have a fully consistent style across the spec, but this one seems a bit better.

@domenic domenic merged commit 437ae8e into whatwg:master May 25, 2018
annevk pushed a commit to web-platform-tests/wpt that referenced this pull request May 29, 2018
annevk pushed a commit to whatwg/fetch that referenced this pull request May 29, 2018
And also a member on the request concept (history-navigation flag) to support this API. See w3c/ServiceWorker#1167 for the discussion that led to this.

Tests: web-platform-tests/wpt#10909.

Corresponding HTML change: whatwg/html#3674.
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jun 10, 2018
…n, a=testonly

Automatic update from web-platform-testsFetch: tests for Request's isHistoryNavigation

See whatwg/fetch#718, whatwg/html#3674, and discussion in w3c/ServiceWorker#1167.
--

wpt-commits: 8f805ef0b6a3ddb06c0266cc56557d05e36f3a3d
wpt-pr: 10909
xeonchen pushed a commit to xeonchen/gecko-cinnabar that referenced this pull request Jun 12, 2018
…n, a=testonly

Automatic update from web-platform-testsFetch: tests for Request's isHistoryNavigation

See whatwg/fetch#718, whatwg/html#3674, and discussion in w3c/ServiceWorker#1167.
--

wpt-commits: 8f805ef0b6a3ddb06c0266cc56557d05e36f3a3d
wpt-pr: 10909
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 3, 2019
…n, a=testonly

Automatic update from web-platform-testsFetch: tests for Request's isHistoryNavigation

See whatwg/fetch#718, whatwg/html#3674, and discussion in w3c/ServiceWorker#1167.
--

wpt-commits: 8f805ef0b6a3ddb06c0266cc56557d05e36f3a3d
wpt-pr: 10909

UltraBlame original commit: 28a4916ff9d5655c7b816da2a9d4c7746f664304
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
…n, a=testonly

Automatic update from web-platform-testsFetch: tests for Request's isHistoryNavigation

See whatwg/fetch#718, whatwg/html#3674, and discussion in w3c/ServiceWorker#1167.
--

wpt-commits: 8f805ef0b6a3ddb06c0266cc56557d05e36f3a3d
wpt-pr: 10909

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

Automatic update from web-platform-testsFetch: tests for Request's isHistoryNavigation

See whatwg/fetch#718, whatwg/html#3674, and discussion in w3c/ServiceWorker#1167.
--

wpt-commits: 8f805ef0b6a3ddb06c0266cc56557d05e36f3a3d
wpt-pr: 10909

UltraBlame original commit: 28a4916ff9d5655c7b816da2a9d4c7746f664304
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.

3 participants