tor-browser

commit 4e44144ea28bb5b3323d4fe655eaac91f571bd4b
parent e6973e59747dd61e43696c15add39a07093cfa0d
Author: Simon Farre <sfarre@mozilla.com>
Date:   Tue, 18 Nov 2025 16:15:34 +0000

Bug 1999663 - Fix restoration of scroll position r=dom-core,smaug

This patch updates the scroll position data of the "local" (i.e. not
SHIP/in parent process) navigation API entries at the right time and
removes restoring scroll position from mActiveEntry of nsDocShell and
instead uses the target history entry, i.e. the one we're navigating
towards and use that scroll that. This is technically spec compliant
since the active entry is supposed to be the target entry at that
moment.

SHIP still works as intended and is left unchanged by this patch.

Differential Revision: https://phabricator.services.mozilla.com/D272446

Diffstat:
M
docshell/base/nsDocShell.cpp
 | 
14
++++++--------
M
docshell/base/nsDocShell.h
 | 
3
++-
M
dom/events/NavigateEvent.cpp
 | 
18
+++++++++++++++---
M
dom/navigation/Navigation.cpp
 | 
11
+++++++++++

4 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
@@ -5020,17 +5020,15 @@ nsresult nsDocShell::SetCurScrollPosEx(int32_t aCurHorizontalPos,
   return NS_OK;
 }
 
-void nsDocShell::RestoreScrollPosFromActiveSHE() {
+void nsDocShell::RestoreScrollPositionFromTargetSessionHistoryInfo(
+    SessionHistoryInfo* aTarget) {
+  MOZ_DIAGNOSTIC_ASSERT(mozilla::SessionHistoryInParent());
   nscoord bx = 0;
   nscoord by = 0;
-  if ((mozilla::SessionHistoryInParent() ? !!mActiveEntry : !!mOSHE)) {
-    if (mozilla::SessionHistoryInParent()) {
-      mActiveEntry->GetScrollPosition(&bx, &by);
-    } else {
-      mOSHE->GetScrollPosition(&bx, &by);
-    }
-    SetCurScrollPosEx(bx, by);
+  if (aTarget) {
+    aTarget->GetScrollPosition(&bx, &by);
   }
+  SetCurScrollPosEx(bx, by);
 }
 
 void nsDocShell::SetScrollbarPreference(mozilla::ScrollbarPreference aPref) {
diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h
@@ -1052,7 +1052,8 @@ class nsDocShell final : public nsDocLoader,
                              int32_t aCurVerticalPos);
   nsPoint GetCurScrollPos();
 
-  void RestoreScrollPosFromActiveSHE();
+  void RestoreScrollPositionFromTargetSessionHistoryInfo(
+      mozilla::dom::SessionHistoryInfo* aTarget);
 
   already_AddRefed<mozilla::dom::ChildSHistory> GetRootSessionHistory();
 
diff --git a/dom/events/NavigateEvent.cpp b/dom/events/NavigateEvent.cpp
@@ -13,6 +13,7 @@
 #include "mozilla/dom/ElementBinding.h"
 #include "mozilla/dom/NavigateEventBinding.h"
 #include "mozilla/dom/Navigation.h"
+#include "mozilla/dom/NavigationHistoryEntry.h"
 #include "mozilla/dom/SessionHistoryEntry.h"
 #include "nsDocShell.h"
 #include "nsFocusManager.h"
@@ -468,7 +469,8 @@ static void ScrollToBeginningOfDocument(Document& aDocument) {
 
 // https://html.spec.whatwg.org/#restore-scroll-position-data
 static void RestoreScrollPositionData(Document* aDocument,
-                                      const uint32_t& aLastScrollGeneration) {
+                                      const uint32_t& aLastScrollGeneration,
+                                      SessionHistoryInfo* aHistoryEntry) {
   // 1. Let document be entry's document.
   // 2. If document's has been scrolled by the user is true, then the user agent
   // should return.
@@ -485,7 +487,7 @@ static void RestoreScrollPositionData(Document* aDocument,
   // restore the scroll positions of entry's document's restorable scrollable
   // regions. The user agent may continue to attempt to do so periodically,
   // until document's has been scrolled by the user becomes true.
-  docShell->RestoreScrollPosFromActiveSHE();
+  docShell->RestoreScrollPositionFromTargetSessionHistoryInfo(aHistoryEntry);
 }
 
 // https://html.spec.whatwg.org/#process-scroll-behavior
@@ -500,7 +502,17 @@ void NavigateEvent::ProcessScrollBehavior() {
   if (mNavigationType == NavigationType::Traverse ||
       mNavigationType == NavigationType::Reload) {
     RefPtr<Document> document = GetAssociatedDocument();
-    RestoreScrollPositionData(document, mLastScrollGeneration);
+    // SHIP changes the active entry in
+    // `nsDocShell::HandleSameDocumentNavigation`, which breaks with Navigation
+    // API spec steps as it's too late, and at this point, the actual "active
+    // session history entry" will become the target session history entry
+    // provided here, which is why we're using this instead of
+    // nsDocShell::mActiveEntry
+    RestoreScrollPositionData(
+        document, mLastScrollGeneration,
+        mDestination->GetEntry()
+            ? mDestination->GetEntry()->SessionHistoryInfo()
+            : nullptr);
     return;
   }
 
diff --git a/dom/navigation/Navigation.cpp b/dom/navigation/Navigation.cpp
@@ -1278,6 +1278,17 @@ struct NavigationWaitForAllScope final : public nsISupports,
 
     // 9. If event's interception state is not "none":
     if (mEvent->InterceptionState() != NavigateEvent::InterceptionState::None) {
+      // The copy of the active session history info might be stale at this
+      // point, so make sure to update that. This is not a spec step, but a side
+      // effect of SHIP owning the session history entries making Navigation API
+      // keep copies for its purposes. Should navigation get aborted at this
+      // point, all we've done is eagerly stored scroll positions.
+      if (RefPtr current = mNavigation->GetCurrentEntry()) {
+        nsPoint scrollPos = docShell->GetCurScrollPos();
+        current->SessionHistoryInfo()->SetScrollPosition(scrollPos.x,
+                                                         scrollPos.y);
+      }
+
       // 5. Set event's interception state to "committed".
       // See https://github.com/whatwg/html/issues/11830 for this change.
       mEvent->SetInterceptionState(NavigateEvent::InterceptionState::Committed);