[Navigation] Dispatch NavigateEvent on same-document navigations #25640
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
TingPing
added
the
WebCore Misc.
This comment was marked as outdated.
This comment was marked as outdated.
rwlbuis
reviewed
Mar 8, 2024
rwlbuis
reviewed
Mar 8, 2024
rwlbuis
reviewed
Mar 8, 2024
rwlbuis
reviewed
Mar 8, 2024
webkit-ews-buildbot
added
the
merging-blocked
TingPing
removed
the
merging-blocked
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
22b6e73
to
c4d5ced
Compare
This comment was marked as outdated.
This comment was marked as outdated.
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
c4d5ced
to
952071f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
952071f
to
d980baf
Compare
|
EWS run on previous version of this PR (hash d980baf) |
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
d980baf
to
d072cf9
Compare
|
EWS run on previous version of this PR (hash d072cf9) |
rwlbuis
reviewed
Mar 14, 2024
rwlbuis
reviewed
Mar 14, 2024
webkit-ews-buildbot
added
the
merging-blocked
TingPing
removed
the
merging-blocked
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
d072cf9
to
a7d3d03
Compare
This comment was marked as outdated.
This comment was marked as outdated.
webkit-ews-buildbot
added
the
merging-blocked
TingPing
removed
the
merging-blocked
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
a7d3d03
to
fb9529e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
webkit-ews-buildbot
added
the
merging-blocked
TingPing
removed
the
merging-blocked
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
fb9529e
to
46ccbee
Compare
This comment was marked as outdated.
This comment was marked as outdated.
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
46ccbee
to
be51663
Compare
|
EWS run on previous version of this PR (hash be51663) |
rwlbuis
reviewed
Mar 18, 2024
rwlbuis
reviewed
Mar 18, 2024
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
be51663
to
683a932
Compare
|
EWS run on previous version of this PR (hash 683a932) |
|
I split this into a few commits. The first two have their own PRs so this will just land the third commit. I can move out the FrameLoader bits too if that makes sense, though then this patch will do very little. |
webkit-ews-buildbot
added
the
merging-blocked
TingPing
removed
the
merging-blocked
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
683a932
to
11e2e89
Compare
achristensen07
approved these changes
Mar 21, 2024
rwlbuis
reviewed
Mar 21, 2024
| // https://html.spec.whatwg.org/multipage/nav-history-apis.html#fire-a-download-request-navigate-event | ||
| bool Navigation::dispatchDownloadNavigateEvent(const URL&, const String& downloadFilename) | ||
| { | ||
| // FIXME |
There was a problem hiding this comment.
When to call this is tricky, since it seems to be handled mostly in FrameLoaderClient. Maybe an option is to send the event in PolicyChecker::checkNavigationPolicy before calling startDownload.
There was a problem hiding this comment.
Yeah, the policychecker was one place I was looking.
All of these dispatches seem tricky to find the ideal spot, the general web navigation specs don't seem super aligned with WebKit's structure to me.
There was a problem hiding this comment.
Yeah, the policychecker was one place I was looking.
All of these dispatches seem tricky to find the ideal spot, the general navigation specs don't seem super aligned with WebKit's structure to me.
Correct, for example the download handling has a quite seperate algorithm, but I would bet that spec was done after the implementation. I would like to work on refactoring that, but it is not crucial for Navigation API.
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
11e2e89
to
c5e46c1
Compare
This comment was marked as outdated.
This comment was marked as outdated.
TingPing
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
c5e46c1
to
64e8f38
Compare
|
EWS run on current version of this PR (hash 64e8f38)
|
https://bugs.webkit.org/show_bug.cgi?id=265401 Reviewed by Alex Christensen. - Partial implementation intercept/finish/scroll/abort in NavigateEvent. - Implement a large part of the dispatching of Traversal and Push/Reload/Replace events. - The FrameLoader dispatches the events in the case of same-document loads. - Gate NavigateEvent.hasUAVisualTransition behind the setting for it. * LayoutTests/TestExpectations: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/commit-behavior/multiple-intercept-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/click-samedocument-crossorigin-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/click-samedocument-crossorigin-sameorigindomain.sub-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/click-samedocument-sameorigin-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/location-samedocument-crossorigin-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/location-samedocument-crossorigin-sameorigindomain.sub-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/location-samedocument-sameorigin-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/open-samedocument-crossorigin-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/open-samedocument-crossorigin-sameorigindomain.sub-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/open-samedocument-sameorigin-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/submit-samedocument-crossorigin-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/submit-samedocument-crossorigin-sameorigindomain.sub-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/cross-window/submit-samedocument-sameorigin-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/intercept-after-dispatch-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/intercept-canceled-event-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/intercept-handler-null-or-undefined-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-anchor-fragment-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-anchor-userInitiated-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-form-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-form-get-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-form-userInitiated-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-history-back-after-fragment-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-history-back-noop-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-history-go-0-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-iframe-location-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-location-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-meta-refresh-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-navigation-back-same-document-in-iframe-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-navigation-navigate-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-svg-anchor-fragment-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-window-open-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/navigate-window-open-self-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/same-url-replace-cross-document-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigate-event/same-url-replace-same-document-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigation-methods/navigate-history-push-not-loaded-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigation-methods/navigate-same-document-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigation-methods/return-value/navigate-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/navigation-api/scroll-behavior/manual-scroll-after-resolve-expected.txt: * Source/WebCore/loader/FrameLoader.cpp: (WebCore::determineNavigationType): (WebCore::FrameLoader::loadInSameDocument): * Source/WebCore/page/NavigateEvent.cpp: (WebCore::NavigateEvent::NavigateEvent): (WebCore::NavigateEvent::create): (WebCore::NavigateEvent::sharedChecks): (WebCore::NavigateEvent::intercept): (WebCore::NavigateEvent::scroll): (WebCore::NavigateEvent::finish): * Source/WebCore/page/NavigateEvent.h: * Source/WebCore/page/Navigation.cpp: (WebCore::Navigation::currentEntry const): (WebCore::Navigation::updateCurrentEntry): (WebCore::Navigation::hasEntriesAndEventsDisabled const): (WebCore::Navigation::updateForNavigation): (WebCore::documentCanHaveURLRewritten): (WebCore::Navigation::innerDispatchNavigateEvent): (WebCore::Navigation::dispatchTraversalNavigateEvent): (WebCore::Navigation::dispatchPushReplaceReloadNavigateEvent): (WebCore::Navigation::dispatchDownloadNavigateEvent): (WebCore::determineNavigationType): Deleted. * Source/WebCore/page/Navigation.h: * Source/WebCore/page/NavigationTransition.h: Canonical link: https://commits.webkit.org/276522@main
webkit-commit-queue
force-pushed
the
pgriffis/navigation-api-next-4
branch
from
64e8f38
to
b3b7f85
Compare
|
Committed 276522@main (b3b7f85): https://commits.webkit.org/276522@main Reviewed commits have been landed. Closing PR #25640 and removing active labels. |
webkit-commit-queue
removed
the
merge-queue
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.
b3b7f85
64e8f38
🧪 wpe-wk2🧪 api-mac🧪 ios-wk2-wpt🧪 gtk-wk2🛠 tv-sim🧪 api-gtk🛠 watch