-
Notifications
You must be signed in to change notification settings - Fork 26
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
Editorial: rearrange serialized state management #236
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, we were setting an entry's serialized state as part of the end of the navigate() call. This will work less well after #235; there, we need to set the state right after the commit, but before any handlers are called. So most of this patch is just a for-now-editorial rearrangement, to set the serialized state in a different place in the spec (but observably the same given the current API). One part of this includes rearranging things so that if options["state"] is not given to reload() or navigate(), we now pass through the serialization of undefined, instead of passing through null. This allows null to be used as a sentinel for traverse cases. This change is also not observable, since getState() would turn null into undefined.
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Jun 6, 2022
Spec: WICG/navigation-api#236 Bug: 1183545 Change-Id: I203419f45a3dfba1b4dab3dc2672146b2da92ce3
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
Jun 6, 2022
Spec: WICG/navigation-api#236 Bug: 1183545 Change-Id: I203419f45a3dfba1b4dab3dc2672146b2da92ce3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688183 Commit-Queue: Nate Chapin <japhet@chromium.org> Auto-Submit: Nate Chapin <japhet@chromium.org> Reviewed-by: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/main@{#1011166}
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Jun 6, 2022
Spec: WICG/navigation-api#236 Bug: 1183545 Change-Id: I203419f45a3dfba1b4dab3dc2672146b2da92ce3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688183 Commit-Queue: Nate Chapin <japhet@chromium.org> Auto-Submit: Nate Chapin <japhet@chromium.org> Reviewed-by: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/main@{#1011166}
chromium-wpt-export-bot
pushed a commit
to web-platform-tests/wpt
that referenced
this pull request
Jun 6, 2022
Spec: WICG/navigation-api#236 Bug: 1183545 Change-Id: I203419f45a3dfba1b4dab3dc2672146b2da92ce3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688183 Commit-Queue: Nate Chapin <japhet@chromium.org> Auto-Submit: Nate Chapin <japhet@chromium.org> Reviewed-by: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/main@{#1011166}
moz-v2v-gh
pushed a commit
to mozilla/gecko-dev
that referenced
this pull request
Jun 10, 2022
…g commit, rather than after, a=testonly Automatic update from web-platform-tests Set the NavigationApi state object during commit, rather than after Spec: WICG/navigation-api#236 Bug: 1183545 Change-Id: I203419f45a3dfba1b4dab3dc2672146b2da92ce3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688183 Commit-Queue: Nate Chapin <japhet@chromium.org> Auto-Submit: Nate Chapin <japhet@chromium.org> Reviewed-by: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/main@{#1011166} -- wpt-commits: 388d5f6e5e1c0939c6febccab56d5d1590b9b7b9 wpt-pr: 34320
mjfroman
pushed a commit
to mjfroman/moz-libwebrtc-third-party
that referenced
this pull request
Oct 14, 2022
Spec: WICG/navigation-api#236 Bug: 1183545 Change-Id: I203419f45a3dfba1b4dab3dc2672146b2da92ce3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3688183 Commit-Queue: Nate Chapin <japhet@chromium.org> Auto-Submit: Nate Chapin <japhet@chromium.org> Reviewed-by: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/main@{#1011166} NOKEYCHECK=True GitOrigin-RevId: b7d5ba957ac02eb4aef025b613561d5741011225
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, we were setting an entry's serialized state as part of the end of the navigate() call. This will work less well after #235; there, we need to set the state right after the commit, but before any handlers are called. So this is a for-now-editorial rearrangement, to set the serialized state in a different place in the spec (but observably the same given the current API).
One part of this includes rearranging things so that if options["state"] is not given to reload() or navigate(), we now pass through the serialization of undefined, instead of passing through null. This allows null to be used as a sentinel for traverse cases. This change is also not observable, since getState() would turn null into undefined.
Preview | Diff