commit e1c5f13b938980c97d5bfedd54369464df7c4051
parent 0b832d6a4b1b2c5231376456f1f616a99f503d7e
Author: Adam Vandolder <avandolder@mozilla.com>
Date: Sun, 5 Oct 2025 17:32:37 +0000
Bug 1992503 - When recreating the contiguous entries list, check for parent entry same-documentness instead of equality. r=dom-core,sessionstore-reviewers,smaug,sfoster
Differential Revision: https://phabricator.services.mozilla.com/D267471
Diffstat:
2 files changed, 81 insertions(+), 34 deletions(-)
diff --git a/browser/components/sessionstore/test/browser_navigation_api_restore.js b/browser/components/sessionstore/test/browser_navigation_api_restore.js
@@ -91,6 +91,37 @@ add_task(async function test_frame_fragment_navigations() {
gBrowser.removeTab(tab);
});
+add_task(async function test_mixed_fragment_navigations() {
+ let tab = BrowserTestUtils.addTab(gBrowser, EMPTY_FRAME_URL);
+ let browser = tab.linkedBrowser;
+ await promiseBrowserLoaded(browser);
+
+ let hashes = ["#frame1", "#frame2", "#frame3"];
+
+ await SpecialPowers.spawn(browser, [], async () => {
+ content.frames[0].history.pushState(null, "", "#frame1");
+ content.frames[0].history.pushState(null, "", "#frame2");
+ content.history.pushState(null, "", "#top");
+ content.frames[0].history.pushState(null, "", "#frame3");
+ });
+
+ let expectedUrls = [EMPTY_URL, ...hashes.map(hash => `${EMPTY_URL}${hash}`)];
+ await checkNavigationEntries(browser, expectedUrls, true);
+
+ await TabStateFlusher.flush(browser);
+ let { entries } = JSON.parse(ss.getTabState(tab));
+ is(entries.length, 5, "Got expected number of history entries");
+
+ await SessionStoreTestUtils.closeTab(tab);
+ tab = ss.undoCloseTab(window, 0);
+ await promiseTabRestored(tab);
+ browser = tab.linkedBrowser;
+
+ await checkNavigationEntries(browser, expectedUrls, true);
+
+ gBrowser.removeTab(tab);
+});
+
add_task(async function test_toplevel_navigations() {
let tab = BrowserTestUtils.addTab(gBrowser, EMPTY_URL);
let browser = tab.linkedBrowser;
diff --git a/docshell/shistory/nsSHistory.cpp b/docshell/shistory/nsSHistory.cpp
@@ -2407,71 +2407,87 @@ nsresult nsSHistory::LoadEntry(int32_t aIndex, long aLoadType,
return NS_OK;
}
-// Walk the subtree from aSubtreeRoot, finding the next entry in the list
-// of inclusive ancestors, returning either the final entry in the list
-// or the last ancestor that appears in the subtree.
-static nsISHEntry* FindLowestAncestor(Span<nsISHEntry*> aInclusiveAncestors,
- nsISHEntry* aSubtreeRoot) {
- if (!aSubtreeRoot || aInclusiveAncestors.IsEmpty()) {
+namespace {
+
+// Walk the subtree downwards from aSubtreeRoot, finding the next entry in the
+// list of ancestors, returning only if we are able to find the last ancestor -
+// the parent we are looking for.
+// aAncestors is ordered starting from the root down to the parent entry.
+SessionHistoryEntry* FindParent(Span<SessionHistoryEntry*> aAncestors,
+ SessionHistoryEntry* aSubtreeRoot) {
+ if (!aSubtreeRoot || aAncestors.IsEmpty() ||
+ !aAncestors[0]->Info().SharesDocumentWith(aSubtreeRoot->Info())) {
return nullptr;
}
- if (aInclusiveAncestors[0]->GetID() != aSubtreeRoot->GetID()) {
- return nullptr;
+ if (aAncestors.Length() == 1) {
+ return aSubtreeRoot;
}
for (int32_t i = 0, childCount = aSubtreeRoot->GetChildCount();
- aInclusiveAncestors.Length() > 1 && i < childCount; i++) {
+ i < childCount; i++) {
nsCOMPtr<nsISHEntry> child;
aSubtreeRoot->GetChildAt(i, getter_AddRefs(child));
- if (auto* lowestFoundAncestor =
- FindLowestAncestor(aInclusiveAncestors.From(1), child)) {
- return lowestFoundAncestor;
+ if (auto* foundParent =
+ FindParent(aAncestors.From(1),
+ static_cast<SessionHistoryEntry*>(child.get()))) {
+ return foundParent;
}
}
- return aInclusiveAncestors[0];
+ return nullptr;
}
+class SessionHistoryEntryIDComparator {
+ public:
+ static bool Equals(const RefPtr<SessionHistoryEntry>& aLhs,
+ const RefPtr<SessionHistoryEntry>& aRhs) {
+ return aLhs->GetID() == aRhs->GetID();
+ }
+};
+
+} // namespace
+
mozilla::dom::SessionHistoryEntry* nsSHistory::FindAdjacentContiguousEntryFor(
mozilla::dom::SessionHistoryEntry* aEntry, int32_t aSearchDirection) {
MOZ_ASSERT(aSearchDirection == 1 || aSearchDirection == -1);
- RefPtr<BrowsingContext> bc = GetBrowsingContext();
- if (!bc) {
- return {};
- }
-
- nsCOMPtr<nsISHEntry> parent = aEntry;
- nsTArray<nsISHEntry*> inclusiveAncestors;
- while (parent) {
- inclusiveAncestors.AppendElement(parent);
- parent = parent->GetParent();
+ // Construct the list of ancestors of aEntry, starting with the root entry
+ // down to it's immediate parent.
+ nsCOMPtr<nsISHEntry> ancestor = aEntry->GetParent();
+ nsTArray<SessionHistoryEntry*> ancestors;
+ while (ancestor) {
+ ancestors.AppendElement(static_cast<SessionHistoryEntry*>(ancestor.get()));
+ ancestor = ancestor->GetParent();
}
- inclusiveAncestors.Reverse();
+ ancestors.Reverse();
- nsCOMPtr<nsISHEntry> entry = inclusiveAncestors[0];
+ nsCOMPtr<nsISHEntry> rootEntry = ancestors.IsEmpty() ? aEntry : ancestors[0];
nsCOMPtr<nsISHEntry> nextEntry;
- nsISHEntry* lowestFoundAncestor = nullptr;
- for (int32_t i = GetIndexOfEntry(entry) + aSearchDirection;
+ SessionHistoryEntry* foundParent = nullptr;
+ for (int32_t i = GetIndexOfEntry(rootEntry) + aSearchDirection;
i >= 0 && i < Length(); i += aSearchDirection) {
GetEntryAtIndex(i, getter_AddRefs(nextEntry));
- if ((lowestFoundAncestor =
- FindLowestAncestor(inclusiveAncestors, nextEntry)) != aEntry) {
+ foundParent = FindParent(
+ ancestors, static_cast<SessionHistoryEntry*>(nextEntry.get()));
+ if (!foundParent || !foundParent->Children().Contains(
+ aEntry, SessionHistoryEntryIDComparator())) {
break;
}
- entry = nextEntry;
+ rootEntry = nextEntry;
}
- if (!nextEntry || entry == nextEntry ||
- lowestFoundAncestor != nsCOMPtr(aEntry->GetParent())) {
+ if (!nextEntry || rootEntry == nextEntry ||
+ (nsCOMPtr(aEntry->GetParent()) && !foundParent)) {
// If we were unable to find a tree that doesn't contain aEntry, or if
- // aEntry is for a subframe and we were unable to find it's direct parent
+ // aEntry is for a subframe and we were unable to find it's parent
// within the previous tree, then aEntry must be the first or last entry
// within it's contiguous entry list.
return {};
}
nsISHEntry* foundEntry = nullptr;
+
+ RefPtr bc = GetBrowsingContext();
bool differenceFound = ForEachDifferingEntry(
- entry, nextEntry, bc,
+ rootEntry, nextEntry, bc,
[&foundEntry](nsISHEntry* differingEntry,
[[maybe_unused]] BrowsingContext* parent) {
// Only one differing entry should be found.
