From f1ed7fa59ebfb770d78d2f34f3960a03905077c2 Mon Sep 17 00:00:00 2001
From: Peter Van der Beken <peterv@propagandism.org>
Date: Wed, 2 Feb 2022 13:22:30 +0000
Subject: [PATCH] Bug 1750174 - Auto-refreshing pages reposition themselves to
 the top upon refresh. r=smaug

Differential Revision: https://phabricator.services.mozilla.com/D137383
---
 docshell/base/BrowsingContext.cpp            | 12 +++-
 docshell/base/BrowsingContext.h              |  2 +
 docshell/base/CanonicalBrowsingContext.cpp   |  5 ++
 docshell/base/CanonicalBrowsingContext.h     |  8 ++-
 docshell/test/mochitest/file_bug1742865.sjs  | 13 +++-
 docshell/test/mochitest/test_bug1742865.html | 72 ++++++++++++++------
 6 files changed, 84 insertions(+), 28 deletions(-)

diff --git a/docshell/base/BrowsingContext.cpp b/docshell/base/BrowsingContext.cpp
index b6f80119de73..fc442dd60f04 100644
--- a/docshell/base/BrowsingContext.cpp
+++ b/docshell/base/BrowsingContext.cpp
@@ -3499,13 +3499,21 @@ bool BrowsingContext::IsPopupAllowed() {
 /* static */
 bool BrowsingContext::ShouldAddEntryForRefresh(
     nsIURI* aCurrentURI, const SessionHistoryInfo& aInfo) {
-  if (aInfo.GetPostData()) {
+  return ShouldAddEntryForRefresh(aCurrentURI, aInfo.GetURI(),
+                                  aInfo.GetPostData());
+}
+
+/* static */
+bool BrowsingContext::ShouldAddEntryForRefresh(nsIURI* aCurrentURI,
+                                               nsIURI* aNewURI,
+                                               bool aHasPostData) {
+  if (aHasPostData) {
     return true;
   }
 
   bool equalsURI = false;
   if (aCurrentURI) {
-    aCurrentURI->Equals(aInfo.GetURI(), &equalsURI);
+    aCurrentURI->Equals(aNewURI, &equalsURI);
   }
   return !equalsURI;
 }
diff --git a/docshell/base/BrowsingContext.h b/docshell/base/BrowsingContext.h
index 6fbf4a05d05e..ef047d445116 100644
--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@@ -908,6 +908,8 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
 
   static bool ShouldAddEntryForRefresh(nsIURI* aCurrentURI,
                                        const SessionHistoryInfo& aInfo);
+  static bool ShouldAddEntryForRefresh(nsIURI* aCurrentURI, nsIURI* aNewURI,
+                                       bool aHasPostData);
 
  private:
   void Attach(bool aFromIPC, ContentParent* aOriginProcess);
diff --git a/docshell/base/CanonicalBrowsingContext.cpp b/docshell/base/CanonicalBrowsingContext.cpp
index 5c4a99f397f6..9d98d6efdcf0 100644
--- a/docshell/base/CanonicalBrowsingContext.cpp
+++ b/docshell/base/CanonicalBrowsingContext.cpp
@@ -535,6 +535,11 @@ CanonicalBrowsingContext::CreateLoadingSessionHistoryEntryForLoad(
       return nullptr;
     }
     Unused << SetHistoryEntryCount(entry->BCHistoryLength());
+  } else if (aLoadState->LoadType() == LOAD_REFRESH &&
+             !ShouldAddEntryForRefresh(aLoadState->URI(),
+                                       aLoadState->PostDataStream()) &&
+             mActiveEntry) {
+    entry = mActiveEntry;
   } else {
     entry = new SessionHistoryEntry(aLoadState, aChannel);
     if (IsTop()) {
diff --git a/docshell/base/CanonicalBrowsingContext.h b/docshell/base/CanonicalBrowsingContext.h
index 3522ced00788..dbda72f598e3 100644
--- a/docshell/base/CanonicalBrowsingContext.h
+++ b/docshell/base/CanonicalBrowsingContext.h
@@ -464,9 +464,13 @@ class CanonicalBrowsingContext final : public BrowsingContext {
   void RemovePendingDiscard();
 
   bool ShouldAddEntryForRefresh(const SessionHistoryEntry* aEntry) {
+    return ShouldAddEntryForRefresh(aEntry->Info().GetURI(),
+                                    aEntry->Info().GetPostData());
+  }
+  bool ShouldAddEntryForRefresh(nsIURI* aNewURI, bool aHasPostData) {
     nsCOMPtr<nsIURI> currentURI = GetCurrentURI();
-    return BrowsingContext::ShouldAddEntryForRefresh(currentURI,
-                                                     aEntry->Info());
+    return BrowsingContext::ShouldAddEntryForRefresh(currentURI, aNewURI,
+                                                     aHasPostData);
   }
 
   // XXX(farre): Store a ContentParent pointer here rather than mProcessId?
diff --git a/docshell/test/mochitest/file_bug1742865.sjs b/docshell/test/mochitest/file_bug1742865.sjs
index 514427bfd83e..950c30ecd289 100644
--- a/docshell/test/mochitest/file_bug1742865.sjs
+++ b/docshell/test/mochitest/file_bug1742865.sjs
@@ -13,10 +13,10 @@ function handleRequest(request, response) {
   // index == 0 First load, returns first meta refresh
   // index == 1 Second load, caused by first meta refresh, returns second meta refresh
   // index == 2 Third load, caused by second meta refresh, doesn't return a meta refresh
+  let query = new URLSearchParams(request.queryString);
   if (index < 2) {
-    let query = new URLSearchParams(request.queryString);
     refresh = query.get("seconds");
-    if (query.get("crossorigin") == "true") {
+    if (query.get("crossOrigin") == "true") {
       const hosts = ["example.org", "example.com"];
 
       let url = `${request.scheme}://${hosts[index]}${request.path}?${request.queryString}`;
@@ -24,6 +24,10 @@ function handleRequest(request, response) {
     }
     refresh = `<meta http-equiv="Refresh" content="${refresh}">`;
   }
+  // We want to scroll for the first load, and check that the meta refreshes keep the same
+  // scroll position.
+  let scroll = index == 0 ? `scrollTo(0, ${query.get("scrollTo")});` : "";
+
   setState("index", String(index + 1));
 
   response.write(
@@ -35,10 +39,12 @@ function handleRequest(request, response) {
   ${refresh}
   <script>
     window.addEventListener("pageshow", () => {
+      ${scroll}
       window.top.opener.postMessage({
         commandType: "pageShow",
         commandData: {
           inputValue: document.getElementById("input").value,
+          scrollPosition: window.scrollY,
         },
       }, "*");
     });
@@ -62,6 +68,9 @@ function handleRequest(request, response) {
 </head>
 <body>
 <input type="text" id="input" value="initial"></input>
+<div style='height: 9000px;'></div>
+<p>
+</p>
 </body>
 </html>`
   );
diff --git a/docshell/test/mochitest/test_bug1742865.html b/docshell/test/mochitest/test_bug1742865.html
index cf459605ba67..c8f9a4eca31d 100644
--- a/docshell/test/mochitest/test_bug1742865.html
+++ b/docshell/test/mochitest/test_bug1742865.html
@@ -16,8 +16,32 @@
 
     const SJS = new URL("file_bug1742865.sjs", location.href);
     const SJS_OUTER = new URL("file_bug1742865_outer.sjs", location.href);
+    const SCROLL = 500;
+
+    let tolerance;
+    function setup() {
+      return SpecialPowers.spawn(window.top, [], () => {
+        return SpecialPowers.getDOMWindowUtils(content.window).getResolution();
+      }).then(resolution => {
+        // Allow a half pixel difference if the top document's resolution is lower
+        // than 1.0 because the scroll position is aligned with screen pixels
+        // instead of CSS pixels.
+        tolerance = resolution < 1.0 ? 0.5 : 0.0;
+      });
+    }
+
+    function checkScrollPosition(scrollPosition, shouldKeepScrollPosition) {
+      isfuzzy(scrollPosition, shouldKeepScrollPosition ? SCROLL : 0, tolerance,
+              `Scroll position ${shouldKeepScrollPosition ? "should" : "shouldn't"} be maintained for meta refresh`);
+    }
+
+    function openWindowAndCheckRefresh(url, params, shouldAddToHistory, shouldKeepScrollPosition) {
+      info(`Running test for ${JSON.stringify(params)}`);
+
+      url = new URL(url);
+      Object.entries(params).forEach(([k, v]) => { url.searchParams.append(k, v) });
+      url.searchParams.append("scrollTo", SCROLL);
 
-    function openWindowAndCheckRefresh(url, shouldAddToHistory) {
       let resetURL = new URL(SJS);
       resetURL.search = "?reset";
       return fetch(resetURL).then(() => {
@@ -41,14 +65,18 @@
 
             is(commandType, "pageShow", "Unknown command type");
 
-            let { inputValue } = commandData;
+            let { inputValue, scrollPosition } = commandData;
 
             switch (++count) {
               // file_bug1742865.sjs causes 3 loads:
               //  * first load, returns first meta refresh
               //  * second load, caused by first meta refresh, returns second meta refresh
               //  * third load, caused by second meta refresh, doesn't return a meta refresh
+              case 2:
+                checkScrollPosition(scrollPosition, shouldKeepScrollPosition);
+                break;
               case 3:
+                checkScrollPosition(scrollPosition, shouldKeepScrollPosition);
                 win.postMessage("changeInputValue", "*");
                 break;
               case 4:
@@ -56,6 +84,7 @@
                 break;
               case 5:
                 is(inputValue, "1234", "Entries for auto-refresh should be attached to session history");
+                checkScrollPosition(scrollPosition, shouldKeepScrollPosition);
                 removeEventListener("message", listener);
                 win.close();
                 resolve();
@@ -67,36 +96,35 @@
       });
     }
 
-    function doTest(seconds, crossOrigin, shouldAddToHistory) {
-      let url = new URL(SJS);
-      url.searchParams.append("seconds", seconds);
-      url.searchParams.append("crossorigin", crossOrigin);
+    function doTest(seconds, crossOrigin, shouldAddToHistory, shouldKeepScrollPosition) {
+      let params = {
+        seconds,
+        crossOrigin,
+      };
 
-      let urlOuter = new URL(SJS_OUTER);
-      urlOuter.searchParams.append("seconds", seconds);
-      urlOuter.searchParams.append("crossorigin", crossOrigin);
-
-      return openWindowAndCheckRefresh(url, shouldAddToHistory).then(() =>
-        openWindowAndCheckRefresh(urlOuter, shouldAddToHistory)
+      return openWindowAndCheckRefresh(SJS, params, shouldAddToHistory, shouldKeepScrollPosition).then(() =>
+        openWindowAndCheckRefresh(SJS_OUTER, params, shouldAddToHistory, shouldKeepScrollPosition)
       );
     }
 
-    function runTest() {
+    async function runTest() {
       const FAST = Math.min(1, REFRESH_REDIRECT_TIMER);
       const SLOW = REFRESH_REDIRECT_TIMER + 1;
       let tests = [
-        // [ time, crossOrigin, shouldAddToHistory ]
-        [ FAST, false, false ],
-        [ FAST, true, false ],
-        [ SLOW, false, false ],
-        [ SLOW, true, true ],
+        // [ time, crossOrigin, shouldAddToHistory, shouldKeepScrollPosition ]
+        [ FAST, false, false, true ],
+        [ FAST, true, false, false ],
+        [ SLOW, false, false, true ],
+        [ SLOW, true, true, false ],
       ];
 
-      let test = Promise.resolve();
-      for (let [ time, crossOrigin, shouldAddToHistory ] of tests) {
-        test = test.then(() => doTest(time, crossOrigin, shouldAddToHistory));
+      await setup();
+
+      for (let [ time, crossOrigin, shouldAddToHistory, shouldKeepScrollPosition ] of tests) {
+        await doTest(time, crossOrigin, shouldAddToHistory, shouldKeepScrollPosition);
       }
-      test.then(() => SimpleTest.finish());
+
+      SimpleTest.finish();
     }
   </script>
 </head>