Bug 1546984 - Add scroll restoration to about:addons r=mstriemer a=lizzard
authorRob Wu <rob@robwu.nl>
Fri, 13 Sep 2019 16:27:52 +0000
changeset 555070 d45c3cd5b904bf59827b0d92949ceddf5eb6658a
parent 555069 a32faba2e5e601fe9cf0f702a8b8be18444ff8ea
child 555071 a7f28a80e29544e2546633b87bfd8501f9f3459a
push id2165
push userffxbld-merge
push dateMon, 14 Oct 2019 16:30:58 +0000
treeherdermozilla-release@0eae18af659f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstriemer, lizzard
bugs1546984
milestone70.0
Bug 1546984 - Add scroll restoration to about:addons r=mstriemer a=lizzard - Attach scroll offsets to history entries in about:addons. - For other views, start at the top of the page, regardless of the scroll position of the (likely unrelated) previous view. Differential Revision: https://phabricator.services.mozilla.com/D44826
toolkit/mozapps/extensions/content/aboutaddons.js
toolkit/mozapps/extensions/content/extensions.js
toolkit/mozapps/extensions/test/browser/browser.ini
toolkit/mozapps/extensions/test/browser/browser_html_scroll_restoration.js
--- a/toolkit/mozapps/extensions/content/aboutaddons.js
+++ b/toolkit/mozapps/extensions/content/aboutaddons.js
@@ -1391,16 +1391,21 @@ class AddonDetails extends HTMLElement {
           }
           break;
         case "preferences":
           if (getOptionsType(this.addon) == "inline") {
             this.inlineOptions.ensureBrowserCreated();
           }
           break;
       }
+
+      // When a details view is rendered again, the default details view is
+      // unconditionally shown. So if any other tab is selected, do not save
+      // the current scroll offset, but start at the top of the page instead.
+      ScrollOffsets.canRestore = this.deck.selectedViewName === "details";
     }
   }
 
   onInstalled() {
     let policy = WebExtensionPolicy.getByID(this.addon.id);
     let extension = policy && policy.extension;
     if (extension && extension.startupReason === "ADDON_UPGRADE") {
       // Ensure the options browser is recreated when a new version starts.
@@ -3163,16 +3168,50 @@ let mainEl = null;
  *                     element must define a current-view property.
  * @returns {string} The current view name.
  */
 function getTelemetryViewName(el) {
   return el.closest("[current-view]").getAttribute("current-view");
 }
 
 /**
+ * Helper for saving and restoring the scroll offsets when a previously loaded
+ * view is accessed again.
+ */
+var ScrollOffsets = {
+  _key: null,
+  _offsets: new Map(),
+  canRestore: true,
+
+  setView(historyEntryId) {
+    this._key = historyEntryId;
+    this.canRestore = true;
+  },
+
+  getPosition() {
+    if (!this.canRestore) {
+      return { top: 0, left: 0 };
+    }
+    let { scrollTop: top, scrollLeft: left } = document.documentElement;
+    return { top, left };
+  },
+
+  save() {
+    if (this._key) {
+      this._offsets.set(this._key, this.getPosition());
+    }
+  },
+
+  restore() {
+    let { top = 0, left = 0 } = this._offsets.get(this._key) || {};
+    window.scrollTo({ top, left, behavior: "auto" });
+  },
+};
+
+/**
  * Called from extensions.js once, when about:addons is loading.
  */
 function initialize(opts) {
   mainEl = document.getElementById("main");
   loadViewFn = opts.loadViewFn;
   replaceWithDefaultViewFn = opts.replaceWithDefaultViewFn;
   setCategoryFn = opts.setCategoryFn;
   AddonCardListenerHandler.startup();
@@ -3188,17 +3227,17 @@ function initialize(opts) {
   );
 }
 
 /**
  * Called from extensions.js to load a view. The view's render method should
  * resolve once the view has been updated to conform with other about:addons
  * views.
  */
-async function show(type, param, { isKeyboardNavigation }) {
+async function show(type, param, { isKeyboardNavigation, historyEntryId }) {
   let container = document.createElement("div");
   container.setAttribute("current-view", type);
   if (type == "list") {
     await new ListView({ param, root: container }).render();
   } else if (type == "detail") {
     await new DetailView({
       isKeyboardNavigation,
       param,
@@ -3209,15 +3248,31 @@ async function show(type, param, { isKey
     let elem = discoverView.render();
     await document.l10n.translateFragment(elem);
     container.append(elem);
   } else if (type == "updates") {
     await new UpdatesView({ param, root: container }).render();
   } else {
     throw new Error(`Unknown view type: ${type}`);
   }
+
+  ScrollOffsets.save();
+  ScrollOffsets.setView(historyEntryId);
   mainEl.textContent = "";
   mainEl.appendChild(container);
+
+  // Most content has been rendered at this point. The only exception are
+  // recommendations in the discovery pane and extension/theme list, because
+  // they rely on remote data. If loaded before, then these may be rendered
+  // within one tick, so wait a frame before restoring scroll offsets.
+  return new Promise(resolve => {
+    window.requestAnimationFrame(() => {
+      ScrollOffsets.restore();
+      resolve();
+    });
+  });
 }
 
 function hide() {
+  ScrollOffsets.save();
+  ScrollOffsets.setView(null);
   mainEl.textContent = "";
 }
--- a/toolkit/mozapps/extensions/content/extensions.js
+++ b/toolkit/mozapps/extensions/content/extensions.js
@@ -649,16 +649,21 @@ var gEventManager = {
   },
 };
 
 var gViewController = {
   viewPort: null,
   currentViewId: "",
   currentViewObj: null,
   currentViewRequest: 0,
+  // All historyEntryId values must be unique within one session, because the
+  // IDs are used to map history entries to page state. It is not possible to
+  // see whether a historyEntryId was used in history entries before this page
+  // was loaded, so start counting from a random value to avoid collisions.
+  nextHistoryEntryId: Math.floor(Math.random() * 2 ** 32),
   viewObjects: {},
   viewChangeCallback: null,
   initialViewSelected: false,
   lastHistoryIndex: -1,
   backButton: null,
 
   initialize() {
     this.viewPort = document.getElementById("view-port");
@@ -762,16 +767,17 @@ var gViewController = {
     }
 
     let isKeyboardNavigation =
       sourceEvent &&
       sourceEvent.mozInputSource === MouseEvent.MOZ_SOURCE_KEYBOARD;
     var state = {
       view: aViewId,
       previousView: this.currentViewId,
+      historyEntryId: ++this.nextHistoryEntryId,
       isKeyboardNavigation,
     };
     if (!isRefresh) {
       gHistory.pushState(state);
       this.lastHistoryIndex = gHistory.index;
     }
     this.loadViewInternal(aViewId, this.currentViewId, state);
   },
@@ -781,25 +787,27 @@ var gViewController = {
   replaceView(aViewId) {
     if (aViewId == this.currentViewId) {
       return;
     }
 
     var state = {
       view: aViewId,
       previousView: null,
+      historyEntryId: ++this.nextHistoryEntryId,
     };
     gHistory.replaceState(state);
     this.loadViewInternal(aViewId, null, state);
   },
 
   loadInitialView(aViewId) {
     var state = {
       view: aViewId,
       previousView: null,
+      historyEntryId: ++this.nextHistoryEntryId,
     };
     gHistory.replaceState(state);
 
     this.loadViewInternal(aViewId, null, state);
     notifyInitialized();
   },
 
   get displayedView() {
--- a/toolkit/mozapps/extensions/test/browser/browser.ini
+++ b/toolkit/mozapps/extensions/test/browser/browser.ini
@@ -76,16 +76,17 @@ skip-if = true # Bug 1449071 - Frequent 
 [browser_html_message_bar.js]
 [browser_html_named_deck.js]
 [browser_html_options_ui.js]
 [browser_html_options_ui_in_tab.js]
 [browser_html_plugins.js]
 skip-if = (os == 'win' && processor == 'aarch64') # aarch64 has no plugin support, bug 1525174 and 1547495
 [browser_html_recent_updates.js]
 [browser_html_recommendations.js]
+[browser_html_scroll_restoration.js]
 [browser_html_updates.js]
 [browser_html_warning_messages.js]
 [browser_installssl.js]
 skip-if = verify
 [browser_interaction_telemetry.js]
 [browser_manage_shortcuts.js]
 [browser_manage_shortcuts_hidden.js]
 [browser_pluginprefs.js]
new file mode 100644
--- /dev/null
+++ b/toolkit/mozapps/extensions/test/browser/browser_html_scroll_restoration.js
@@ -0,0 +1,226 @@
+/* eslint max-len: ["error", 80] */
+"use strict";
+
+const { AddonTestUtils } = ChromeUtils.import(
+  "resource://testing-common/AddonTestUtils.jsm"
+);
+
+AddonTestUtils.initMochitest(this);
+const server = AddonTestUtils.createHttpServer();
+const TEST_API_URL = `http://localhost:${server.identity.primaryPort}/discoapi`;
+
+const EXT_ID_EXTENSION = "extension@example.com";
+const EXT_ID_THEME = "theme@example.com";
+
+let requestCount = 0;
+server.registerPathHandler("/discoapi", (request, response) => {
+  // This test is expected to load the results only once, and then cache the
+  // results.
+  is(++requestCount, 1, "Expect only one discoapi request");
+
+  let results = {
+    results: [
+      {
+        addon: {
+          authors: [{ name: "Some author" }],
+          current_version: {
+            files: [{ platform: "all", url: "data:," }],
+          },
+          url: "data:,",
+          guid: "recommendation@example.com",
+          type: "extension",
+        },
+      },
+    ],
+  };
+  response.write(JSON.stringify(results));
+});
+
+add_task(async function setup() {
+  await SpecialPowers.pushPrefEnv({
+    set: [["extensions.getAddons.discovery.api_url", TEST_API_URL]],
+  });
+
+  let mockProvider = new MockProvider();
+  mockProvider.createAddons([
+    {
+      id: EXT_ID_EXTENSION,
+      name: "Mock 1",
+      type: "extension",
+      userPermissions: {
+        origins: ["<all_urls>"],
+        permissions: ["tabs"],
+      },
+    },
+    {
+      id: EXT_ID_THEME,
+      name: "Mock 2",
+      type: "theme",
+    },
+  ]);
+});
+
+async function switchToView(win, type, param = "") {
+  let loaded = waitForViewLoad(win);
+  win.managerWindow.gViewController.loadView(`addons://${type}/${param}`);
+  await loaded;
+  await waitForStableLayout(win);
+}
+
+// delta = -1 = go back.
+// delta = +1 = go forwards.
+async function historyGo(win, delta, expectedViewType) {
+  let loaded = waitForViewLoad(win);
+  win.managerWindow.history.go(delta);
+  await loaded;
+  is(
+    win.managerWindow.gViewController.currentViewId,
+    expectedViewType,
+    "Expected view after history navigation"
+  );
+  await waitForStableLayout(win);
+}
+
+async function waitForStableLayout(win) {
+  // In the test, it is important that the layout is fully stable before we
+  // consider the view loaded, because those affect the offset calculations.
+  await TestUtils.waitForCondition(
+    () => isLayoutStable(win),
+    "Waiting for layout to stabilize"
+  );
+}
+
+function isLayoutStable(win) {
+  // <message-bar> elements may affect the layout of a page, and therefore we
+  // should check whether its embedded style sheet has finished loading.
+  for (let bar of win.document.querySelectorAll("message-bar")) {
+    // Check for the existence of a CSS property from message-bar.css.
+    if (!win.getComputedStyle(bar).getPropertyValue("--message-bar-icon-url")) {
+      return false;
+    }
+  }
+  return true;
+}
+
+function getAddonCard(win, addonId) {
+  return win.document.querySelector(`addon-card[addon-id="${addonId}"]`);
+}
+
+function getScrollOffset(win) {
+  let { scrollTop: top, scrollLeft: left } = win.document.documentElement;
+  return { top, left };
+}
+
+// Scroll an element into view. The purpose of this is to simulate a real-world
+// scenario where the user has moved part of the UI is in the viewport.
+function scrollTopLeftIntoView(elem) {
+  elem.scrollIntoView({ block: "start", inline: "start" });
+  // Sanity check: In this test, a large padding has been added to the top and
+  // left of the document. So when an element has been scrolled into view, the
+  // top and left offsets must be non-zero.
+  assertNonZeroScrollOffsets(getScrollOffset(elem.ownerGlobal));
+}
+
+function assertNonZeroScrollOffsets(offsets) {
+  ok(offsets.left, "Should have scrolled to the right");
+  ok(offsets.top, "Should have scrolled down");
+}
+
+function checkScrollOffset(win, expected, msg = "") {
+  let actual = getScrollOffset(win);
+  is(actual.top, expected.top, `Top scroll offset - ${msg}`);
+  is(actual.left, expected.left, `Left scroll offset - ${msg}`);
+}
+
+add_task(async function test_scroll_restoration() {
+  let win = await loadInitialView("discover");
+
+  // Wait until the recommendations have been loaded. These are cached after
+  // the first load, so we only need to wait once, at the start of the test.
+  await win.document.querySelector("recommended-addon-list").cardsReady;
+
+  // Force scrollbar to appear, by adding enough space at the top and left.
+  win.document.body.style.paddingTop = "200vh";
+  win.document.body.style.paddingLeft = "100vw";
+  win.document.body.style.width = "200vw";
+
+  checkScrollOffset(win, { top: 0, left: 0 }, "initial page load");
+
+  scrollTopLeftIntoView(win.document.querySelector("recommended-addon-card"));
+  let discoOffsets = getScrollOffset(win);
+  assertNonZeroScrollOffsets(discoOffsets);
+
+  // Switch from disco pane to extension list
+
+  await switchToView(win, "list", "extension");
+  checkScrollOffset(win, { top: 0, left: 0 }, "initial extension list");
+
+  scrollTopLeftIntoView(getAddonCard(win, EXT_ID_EXTENSION));
+  let extListOffsets = getScrollOffset(win);
+  assertNonZeroScrollOffsets(extListOffsets);
+
+  // Switch from extension list to details view.
+
+  let loaded = waitForViewLoad(win);
+  getAddonCard(win, EXT_ID_EXTENSION).click();
+  await loaded;
+
+  checkScrollOffset(win, { top: 0, left: 0 }, "initial details view");
+  scrollTopLeftIntoView(getAddonCard(win, EXT_ID_EXTENSION));
+  let detailsOffsets = getScrollOffset(win);
+  assertNonZeroScrollOffsets(detailsOffsets);
+
+  // Switch from details view back to extension list.
+
+  await historyGo(win, -1, "addons://list/extension");
+  checkScrollOffset(win, extListOffsets, "back to extension list");
+
+  // Now scroll to the bottom-right corner, so we can check whether the scroll
+  // offset is correctly restored when the extension view is loaded, even when
+  // the recommendations are loaded after the initial render.
+  ok(
+    win.document.querySelector("recommended-addon-card"),
+    "Recommendations have already been loaded"
+  );
+  win.document.body.scrollIntoView({ block: "end", inline: "end" });
+  extListOffsets = getScrollOffset(win);
+  assertNonZeroScrollOffsets(extListOffsets);
+
+  // Switch back from the extension list to the details view.
+
+  await historyGo(win, +1, `addons://detail/${EXT_ID_EXTENSION}`);
+  checkScrollOffset(win, detailsOffsets, "details view with default tab");
+
+  // Switch from the default details tab to the permissions tab.
+  // (this does not change the history).
+  win.document.querySelector("named-deck-button[name='permissions']").click();
+
+  // Switch back from the details view to the extension list.
+
+  await historyGo(win, -1, "addons://list/extension");
+  checkScrollOffset(win, extListOffsets, "bottom-right of extension list");
+  ok(
+    win.document.querySelector("recommended-addon-card"),
+    "Recommendations should have been loaded again"
+  );
+
+  // Switch back from extension list to the details view.
+
+  await historyGo(win, +1, `addons://detail/${EXT_ID_EXTENSION}`);
+  // Scroll offsets are not remembered for the details view, because at the
+  // time of leaving the details view, the non-default tab was selected.
+  checkScrollOffset(win, { top: 0, left: 0 }, "details view, non-default tab");
+
+  // Switch back from the details view to the disco pane.
+
+  await historyGo(win, -2, "addons://discover/");
+  checkScrollOffset(win, discoOffsets, "after switching back to disco pane");
+
+  // Switch from disco pane to theme list.
+
+  // Verifies that the extension list and theme lists are independent.
+  await switchToView(win, "list", "theme");
+  checkScrollOffset(win, { top: 0, left: 0 }, "initial theme list");
+
+  await closeView(win);
+});