commit 46ea2de4d34ffc65490b6a756301a24384d1e464
parent 7ae181d459f1bea342f698c7f3daf33170a7b5d7
Author: Adam Vandolder <avandolder@mozilla.com>
Date: Wed, 26 Nov 2025 15:35:29 +0000
Bug 2002159 - Update the navigation API entries before scrolling during a same-document navigation. r=dom-core,farre
When performing a same-document navigation, we update the active entry
in the docshell and browsing context, then we call ScrollToAnchor,
and only after scrolling we call UpdateEntriesForSameDocumentNavigation.
However, ScrollToAnchor can trigger script to run, and if this script
performs a same-document replace navigation via replaceState we end up
in state where the docshell's mActiveEntry and the navigation API's
current entry are out of sync, resulting in us triggering the assertion
that guarantees replaced entries have the same navigation key as the
entry replacing them.
This patch fixes this by updating the navigation API entries before
scrolling, which more closely follows the #scroll-to-fragid steps
in the spec.
Differential Revision: https://phabricator.services.mozilla.com/D274048
Diffstat:
3 files changed, 55 insertions(+), 13 deletions(-)
diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
@@ -9318,11 +9318,32 @@ nsresult nsDocShell::HandleSameDocumentNavigation(
nsCOMPtr<nsPIDOMWindowInner> win =
scriptGlobal ? scriptGlobal->GetCurrentInnerWindow() : nullptr;
+ // https://html.spec.whatwg.org/#scroll-to-fragid
+ // 14. Update document for history step application given navigable's
+ // active document, historyEntry, true, scriptHistoryIndex,
+ // scriptHistoryLength, and historyHandling.
+ if (RefPtr navigation = win ? win->Navigation() : nullptr) {
+ MOZ_LOG(gNavigationAPILog, LogLevel::Debug,
+ ("nsDocShell %p triggering a navigation event from "
+ "HandleSameDocumentNavigation",
+ this));
+ // https://html.spec.whatwg.org/#update-document-for-history-step-application
+ // 6.4.2. Update the navigation API entries for a same-document
+ // navigation given navigation, entry, and navigationType.
+ navigation->UpdateEntriesForSameDocumentNavigation(
+ mActiveEntry.get(),
+ NavigationUtils::NavigationTypeFromLoadType(mLoadType).valueOr(
+ NavigationType::Push));
+ }
+
// The check for uninvoked directives must come before ScrollToAnchor() is
// called.
const bool hasTextDirectives =
doc->FragmentDirective()->HasUninvokedDirectives();
+ // https://html.spec.whatwg.org/#scroll-to-fragid
+ // 15. Scroll to the fragment given navigable's active document.
+
// ScrollToAnchor doesn't necessarily cause us to scroll the window;
// the function decides whether a scroll is appropriate based on the
// arguments it receives. But even if we don't end up scrolling,
@@ -9356,19 +9377,6 @@ nsresult nsDocShell::HandleSameDocumentNavigation(
// destroy the docshell, nulling out mScriptGlobal. Hold a stack
// reference to avoid null derefs. See bug 914521.
if (win) {
- if (RefPtr navigation = win->Navigation()) {
- MOZ_LOG(gNavigationAPILog, LogLevel::Debug,
- ("nsDocShell %p triggering a navigation event from "
- "HandleSameDocumentNavigation",
- this));
- // Corresponds to step 6.4.2 from the Updating the document algorithm:
- // https://html.spec.whatwg.org/multipage/browsing-the-web.html#updating-the-document
- navigation->UpdateEntriesForSameDocumentNavigation(
- mActiveEntry.get(),
- NavigationUtils::NavigationTypeFromLoadType(mLoadType).valueOr(
- NavigationType::Push));
- }
-
// Fire a hashchange event URIs differ, and only in their hashes.
// If the fragment contains a directive, compare hasRef.
bool doHashchange = aState.mSameExceptHashes &&
diff --git a/docshell/test/navigation/mochitest.toml b/docshell/test/navigation/mochitest.toml
@@ -305,6 +305,8 @@ skip-if = [
"http3",
]
+["test_replaceState_during_fragment_navigation.html"]
+
["test_reserved.html"]
skip-if = [
"debug", # bug 1263213
diff --git a/docshell/test/navigation/test_replaceState_during_fragment_navigation.html b/docshell/test/navigation/test_replaceState_during_fragment_navigation.html
@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+ <title>Test that replaceState from within fragment navigation does not cause a crash</title>
+ <meta charset="utf-8">
+ <script src="/tests/SimpleTest/SimpleTest.js"></script>
+ <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=2002159">Mozilla Bug 2002159</a>
+<p id="display"></p>
+<div id="content" style="display: none"></div>
+<pre id="test">
+<a id="fragment-link" href="#content" onblur="triggerReplace()"></a>
+</pre>
+<script>
+ SimpleTest.waitForExplicitFinish();
+ window.onload = async () => {
+ let link = document.getElementById("fragment-link");
+ link.focus();
+ link.click();
+ };
+
+ function triggerReplace() {
+ history.replaceState(null, null, "#0");
+
+ ok(true, "replaceState from within fragment navigation does not cause a crash");
+ SimpleTest.finish();
+ }
+</script>
+</body>
+</html>
