-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
App history API to navigation API rename (1/n) #33132
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The review process for this patch is being conducted in the Chromium project.
85459a1
to
b75d3ca
Compare
See WICG/navigation-api#83 and WICG/navigation-api#203 for context. This CL implements the following renames: * window.appHistory → window.navigation * appHistory.current → navigation.currentEntry * appHistory.updateCurrent() → navigation.updateCurrentEntry() * appHistory currentchange event → navigation currententrychange event * appHistory.goTo() → navigation.traverseTo() Notably, window.navigation has to be [Replaceable], as otherwise various internal tests that do `var navigation = ...` start failing. No files are renamed at this point (including test files). No classes/IDL interfaces are renamed either. Thus, the majority of this renaming is done in the web platform tests corpus, and to method names of the AppHistory C++ class. Bug: 1300246 Change-Id: I29a01c6992c06a5d5099d831273ca8041281d557 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3510372 Reviewed-by: Avi Drissman <avi@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Commit-Queue: Domenic Denicola <domenic@chromium.org> Cr-Commit-Position: refs/heads/main@{#980648}
b75d3ca
to
51510ac
Compare
WPT Command: Some affected tests had inconsistent (flaky) results: Unstable results
These may be pre-existing or new flakes. Please try to reproduce (see the above WPT command, though some flags may not be needed when running locally) and determine if your change introduced the flake. If you are unable to reproduce the problem, please tag |
@past Pinged the owner on the chromium CL. Can you pls admin merge this? There is a second CL depending on this. Now the exporter is red. |
Per the author: Firefox does not implement this API, so any flakes there are do to that. It'd be best to just merge it anyways, regardless of Firefox flakes. |
Merged, thanks! |
See WICG/navigation-api#83 and WICG/navigation-api#203 for context.
This CL implements the following renames:
Notably, window.navigation has to be [Replaceable], as otherwise various internal tests that do
var navigation = ...
start failing.No files are renamed at this point (including test files). No classes/IDL interfaces are renamed either.
Thus, the majority of this renaming is done in the web platform tests corpus, and to method names of the AppHistory C++ class.
Bug: 1300246
Change-Id: I29a01c6992c06a5d5099d831273ca8041281d557
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3510372
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Nate Chapin <japhet@chromium.org>
Commit-Queue: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#980648}