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

Change { replace: true } to { history: "replace" } #219

Merged
merged 1 commit into from Mar 25, 2022
Merged

Conversation

domenic
Copy link
Collaborator

@domenic domenic commented Mar 24, 2022

We can then introduce { history: "push" } which explicitly errors if we're in a convert-to-replace situation. Closes #111.

This also fixes a preexisting omission where we were not properly doing replace navigations before the document is loaded.


Preview | Diff

We can then introduce { history: "push" } which explicitly errors if we're in a convert-to-replace situation.

This also fixes a preexisting omission where we were not properly doing replace navigations before the document is loaded.
@@ -787,7 +810,7 @@ An <dfn>navigation API method navigation</dfn> is a [=struct=] with the followin

1. Let |info| be |options|["{{NavigationOptions/info}}"] if it exists; otherwise, undefined.

1. Let |historyHandling| be "<a for="history handling behavior">`replace`</a>" if |options|["{{NavigationNavigateOptions/replace}}"] is true; otherwise, "<a for="history handling behavior">`default`</a>".
1. Let |historyHandling| be "<a for="history handling behavior">`replace`</a>" if |options|["{{NavigationNavigateOptions/history}}"] is "{{NavigationHistoryBehavior/replace}}" or if |document| is not <a spec="HTML">completely loaded</a>; otherwise, "<a for="history handling behavior">`default`</a>".
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is somewhat of a bugfix, in the sense that Chromium definitely already did this conversion, and I believe we have tests for it.

In Chromium, by default all before-load navigations are replacements, with a special exception for anchor clicks: see https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/frame/local_frame.cc;l=454;drc=a24520a700e2581395920c74d59a918aa60bd5c9 . However in the spec, each call site of the navigate algorithm independently changes historyHandling, except anchor clicks which do not.

@domenic domenic merged commit eb76072 into main Mar 25, 2022
@domenic domenic deleted the history-handling branch March 25, 2022 16:07
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Mar 31, 2022
Follows WICG/navigation-api#219.

Bug: 1183545
Change-Id: I1e962de093ece36c0f4cdebfcf83a53b4910ea16
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 1, 2022
Follows WICG/navigation-api#219.

Bug: 1183545
Change-Id: I1e962de093ece36c0f4cdebfcf83a53b4910ea16
aarongable pushed a commit to chromium/chromium that referenced this pull request Apr 1, 2022
Follows WICG/navigation-api#219.

Bug: 1183545
Change-Id: I1e962de093ece36c0f4cdebfcf83a53b4910ea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564502
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988142}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 1, 2022
Follows WICG/navigation-api#219.

Bug: 1183545
Change-Id: I1e962de093ece36c0f4cdebfcf83a53b4910ea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564502
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988142}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 1, 2022
Follows WICG/navigation-api#219.

Bug: 1183545
Change-Id: I1e962de093ece36c0f4cdebfcf83a53b4910ea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564502
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988142}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 11, 2022
…history }, a=testonly

Automatic update from web-platform-tests
Navigation API: change { replace } to { history }

Follows WICG/navigation-api#219.

Bug: 1183545
Change-Id: I1e962de093ece36c0f4cdebfcf83a53b4910ea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564502
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988142}

--

wpt-commits: b905dcaacc78838e9b0a70de4aa7ca9735e2b726
wpt-pr: 33461
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Apr 14, 2022
…history }, a=testonly

Automatic update from web-platform-tests
Navigation API: change { replace } to { history }

Follows WICG/navigation-api#219.

Bug: 1183545
Change-Id: I1e962de093ece36c0f4cdebfcf83a53b4910ea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564502
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988142}

--

wpt-commits: b905dcaacc78838e9b0a70de4aa7ca9735e2b726
wpt-pr: 33461
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
Follows WICG/navigation-api#219.

Bug: 1183545
Change-Id: I1e962de093ece36c0f4cdebfcf83a53b4910ea16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3564502
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988142}
NOKEYCHECK=True
GitOrigin-RevId: 3c68dda182f3e2df5197b3c7e392868d1e060e9e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Behavior of navigate(currentURL, { replace })
2 participants