commit ff7376054bcd428d5783ce0c53d707720ba9e6de
parent a4b46b07d1b02ffa08a4ec1d8d4ca0fbb72c6f58
Author: Andreas Farre <farre@mozilla.com>
Date: Wed, 17 Dec 2025 15:39:27 +0000
Bug 2005471 - Abort history traversal to identical entry. r=smaug
Differential Revision: https://phabricator.services.mozilla.com/D276815
Diffstat:
5 files changed, 106 insertions(+), 11 deletions(-)
diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp
@@ -1244,7 +1244,7 @@ void CanonicalBrowsingContext::SessionHistoryCommit(
if (LOAD_TYPE_HAS_FLAGS(aLoadType,
nsIWebNavigation::LOAD_FLAGS_REPLACE_HISTORY)) {
// Replace the current entry with the new entry.
- int32_t index = shistory->GetIndexForReplace();
+ int32_t index = shistory->GetTargetIndexForHistoryOperation();
// If we're trying to replace an inexistant shistory entry then we
// should append instead.
@@ -1613,9 +1613,7 @@ Maybe<int32_t> CanonicalBrowsingContext::HistoryGo(
return Nothing();
}
- CheckedInt<int32_t> index = shistory->GetRequestedIndex() >= 0
- ? shistory->GetRequestedIndex()
- : shistory->Index();
+ CheckedInt<int32_t> index = shistory->GetTargetIndexForHistoryOperation();
MOZ_LOG(gSHLog, LogLevel::Debug,
("HistoryGo(%d->%d) epoch %" PRIu64 "/id %" PRIu64, aOffset,
(index + aOffset).value(), aHistoryEpoch,
@@ -1716,10 +1714,12 @@ void CanonicalBrowsingContext::NavigationTraverse(
return true;
});
+ // Step 12.2
if (!targetEntry) {
return aResolver(NS_ERROR_DOM_INVALID_STATE_ERR);
}
+ // Step 12.3
if (targetEntry == mActiveEntry) {
return aResolver(NS_OK);
}
@@ -1736,8 +1736,16 @@ void CanonicalBrowsingContext::NavigationTraverse(
}
int32_t offset = targetIndex - activeIndex;
- MOZ_LOG_FMT(gNavigationAPILog, LogLevel::Debug, "Performing traversal by {}",
- offset);
+
+ int32_t requestedIndex = shistory->GetTargetIndexForHistoryOperation();
+ // Step 12.3
+ if (requestedIndex == targetIndex) {
+ return aResolver(NS_OK);
+ }
+
+ // Reset the requested index since this is not a relative traversal, and the
+ // offset is overriding any currently ongoing history traversals.
+ shistory->InternalSetRequestedIndex(-1);
HistoryGo(offset, aHistoryEpoch, false, aUserActivation, aCheckForCancelation,
aContentId, std::move(aResolver));
diff --git a/docshell/shistory/nsSHistory.cpp b/docshell/shistory/nsSHistory.cpp
@@ -791,7 +791,7 @@ nsSHistory::AddToRootSessionHistory(bool aCloneChildren, nsISHEntry* aOSHE,
// Replace current entry in session history; If the requested index is
// valid, it indicates the loading was triggered by a history load, and
// we should replace the entry at requested index instead.
- int32_t index = GetIndexForReplace();
+ int32_t index = GetTargetIndexForHistoryOperation();
// Replace the current entry with the new entry
if (index >= 0) {
diff --git a/docshell/shistory/nsSHistory.h b/docshell/shistory/nsSHistory.h
@@ -202,10 +202,11 @@ class nsSHistory : public mozilla::LinkedListElement<nsSHistory>,
}
}
- int32_t GetIndexForReplace() {
- // Replace current entry in session history; If the requested index is
- // valid, it indicates the loading was triggered by a history load, and
- // we should replace the entry at requested index instead.
+ int32_t GetTargetIndexForHistoryOperation() {
+ // When performing a session history operation, such as replace or
+ // navigation by key that can happen during ongoing history traversals; If
+ // the requested index is valid, it indicates the loading was triggered by a
+ // history load, and we should target the entry at requested index instead.
return mRequestedIndex == -1 ? mIndex : mRequestedIndex;
}
diff --git a/testing/web-platform/tests/navigation-api/navigate-event/navigate-history-traversal-during-onnavigate.html b/testing/web-platform/tests/navigation-api/navigate-event/navigate-history-traversal-during-onnavigate.html
@@ -0,0 +1,52 @@
+<!doctype html>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src=/resources/testdriver.js></script>
+<script src=/resources/testdriver-vendor.js></script>
+<script>
+promise_test(async t => {
+ let w = window.open("/common/blank.html");
+ await new Promise(resolve => w.onload = resolve);
+
+ await test_driver.bless("navigate", async function() {
+ w.navigation.navigate(new URL('resources/navigate-history-traversal-during-onnavigate-helper.html?count=1', location.href).href);
+ });
+
+ await new Promise(resolve => onmessage = resolve);
+ w.postMessage("start", "*");
+ await new Promise(resolve => onmessage = resolve);
+
+ await new Promise(resolve => w.onhashchange = resolve);
+
+ w.onpopstate = () => assert_unreached("No more popstate events");
+
+ assert_equals(w.navigation.currentEntry.index, 1);
+ assert_equals(new URL(w.navigation.currentEntry.url).pathname, "/navigation-api/navigate-event/resources/navigate-history-traversal-during-onnavigate-helper.html");
+ assert_equals(new URL(w.navigation.currentEntry.url).hash, "");
+ w.close();
+}, "Test that navigation.back() silently aborts during onnavigate");
+
+promise_test(async t => {
+ let w = window.open("/common/blank.html");
+ await new Promise(resolve => w.onload = resolve);
+
+ await test_driver.bless("navigate", async function() {
+ w.navigation.navigate(new URL('resources/navigate-history-traversal-during-onnavigate-helper.html?count=3', location.href).href);
+ });
+
+ await new Promise(resolve => onmessage = resolve);
+ w.postMessage("start", "*");
+ await new Promise(resolve => onmessage = resolve);
+
+ await new Promise(resolve => w.onhashchange = resolve);
+ assert_equals(w.navigation.currentEntry.index, 3);
+ assert_equals(new URL(w.navigation.currentEntry.url).pathname, "/navigation-api/navigate-event/resources/navigate-history-traversal-during-onnavigate-helper.html");
+ assert_equals(new URL(w.navigation.currentEntry.url).hash, "#1");
+
+ await new Promise(resolve => w.onhashchange = resolve);
+ assert_equals(w.navigation.currentEntry.index, 1);
+ assert_equals(new URL(w.navigation.currentEntry.url).pathname, "/navigation-api/navigate-event/resources/navigate-history-traversal-during-onnavigate-helper.html");
+ assert_equals(new URL(w.navigation.currentEntry.url).hash, "");
+ w.close();
+}, "Test that navigation.traverseTo() works during onnavigate");
+</script>
diff --git a/testing/web-platform/tests/navigation-api/navigate-event/resources/navigate-history-traversal-during-onnavigate-helper.html b/testing/web-platform/tests/navigation-api/navigate-event/resources/navigate-history-traversal-during-onnavigate-helper.html
@@ -0,0 +1,34 @@
+<!doctype html>
+<head>
+<script>
+onload = () => {
+ opener.postMessage("load", "*");
+};
+
+onmessage = () => {
+ runTest();
+}
+
+async function runTest() {
+ const params = new URLSearchParams(location.search);
+ const count = params.get("count") ?? 1;
+ for (let frag = 0; frag < count; ++frag) {
+ await navigation.navigate(`#${frag}`).finished;
+ }
+
+ const key = navigation.entries()[navigation.currentEntry.index - count].key;
+
+ navigation.onnavigate = async () => {
+ if (count === 1) {
+ navigation.back();
+ } else {
+ navigation.traverseTo(key);
+ }
+ }
+
+ opener.postMessage("done", "*");
+
+ history.back();
+}
+</script>
+</head>
