imported/w3c/web-platform-tests/navigation-api/navigation-history-entry/entries-after-javascript-url-navigation.html is failing

[cdumez]

cda0ce0

imported/w3c/web-platform-tests/navigation-api/navigation-history-entry/entries-after-javascript-url-navigation.html is failing
https://bugs.webkit.org/show_bug.cgi?id=284011

Reviewed by Rob Buis.

When setting an iframe's src to a javascript URL and the js returns a String, we
replace the document with that String, which counts as a "Replace" navigation
(navigation which replaces the current HistoryItem). The test in question is
expecting this since it expects `navigation.entries` to have 2 entries before
AND after the JS URL navigation.

The logic in FrameLoader::loadURL() was properly setting the NavigationAction's
navigationType to `NavigationNavigationType::Replace`. However, before we would
end up calling executeJavaScriptURL() instead of doing a proper navigation, we
would not end up setting the NavigationAction object as the DocumentLoader's
triggeringAction. The navigationType is supposed to get passed to
Navigation::initializeForNewWindow() to make sure it replaces the current entry
instead of creating a new one in this case. However, initializeForNewWindow() is
called from FrameLoader::didBeginDocument() which gets it from the DocumentLoader's
triggeringAction.

* LayoutTests/imported/w3c/web-platform-tests/navigation-api/navigation-history-entry/entries-after-javascript-url-navigation-expected.txt:
* Source/WebCore/bindings/js/ScriptController.cpp:
(WebCore::ScriptController::executeJavaScriptURL):
* Source/WebCore/bindings/js/ScriptController.h:
* Source/WebCore/loader/FrameLoader.cpp:
(WebCore::FrameLoader::executeJavaScriptURL):

Canonical link: https://commits.webkit.org/287344@main

73cbedc

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
✅ 🛠 🧪 unsafe-merge	✅ 🛠 tv	🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

[webkit-early-warning-system]

EWS run on previous version of this PR (hash 9fff71a)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	❌ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

[rwlbuis]

LGTM.

The sentence in the commit description needs fixing though:
"The test in question is
expecting since since it require navigation.entries to have 2 entries before
AND after the JS URL navigation."

[rwlbuis]

FWIW some time ago I tried to make javascript URL not so special when doing a replace navigation by using SubstituteData. In theory that should fix entries-in-new-javascript-url-iframe.html and maybe entries-after-javascript-url-navigation.html. IIRC it caused some regressions and I dropped it, but I think it still may be nice to do it as it would simplify FrameLoader, so it is still on my list but not sure when I would have time.

Anyway, this bug fix is valid and great, just wanted to give some background.

[webkit-early-warning-system]

EWS run on current version of this PR (hash 73cbedc)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
✅ 🛠 🧪 unsafe-merge	✅ 🛠 tv	🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

[webkit-commit-queue]

Committed 287344@main (cda0ce0): https://commits.webkit.org/287344@main

Reviewed commits have been landed. Closing PR #37400 and removing active labels.