From 8808ec1a118e569d1246b50fbb0c9ad02b8e533d Mon Sep 17 00:00:00 2001 From: Jonathan Sudiaman Date: Tue, 15 Oct 2024 13:14:26 +0000 Subject: [PATCH] Bug 1908019 - Store sidebar UI state in a pref that acts as a fallback r=sidebar-reviewers,sessionstore-reviewers,sfoster Refactor sidebar state persistence logic outside of SessionStore and into SidebarController and SidebarManager. Expose an API for session store to update state. If session store data is not available, use the backup state instead. Works for both "Never remember history" and "Use custom settings for history". Differential Revision: https://phabricator.services.mozilla.com/D225220 --- browser/app/profile/firefox.js | 4 + .../sessionstore/SessionStore.sys.mjs | 43 +--------- .../test/marionette/test_restore_sidebar.py | 41 ++++++++++ .../components/sidebar/SidebarManager.sys.mjs | 36 +++++++++ browser/components/sidebar/browser-sidebar.js | 81 +++++++++++++------ 5 files changed, 140 insertions(+), 65 deletions(-) diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index f78066bb586a..5083bd1d0e4c 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1991,6 +1991,10 @@ pref("sidebar.animation.duration-ms", 200); pref("sidebar.main.tools", "aichat,syncedtabs,history"); pref("sidebar.verticalTabs", false); pref("sidebar.visibility", "always-show"); +// Sidebar UI state is stored per-window via session restore. Use this pref +// as a backup to restore the sidebar UI state when a user has PPB mode on +// or has history cleared on browser close. +pref("sidebar.backupState", "{}"); pref("browser.ml.chat.enabled", false); pref("browser.ml.chat.hideLocalhost", true); diff --git a/browser/components/sessionstore/SessionStore.sys.mjs b/browser/components/sessionstore/SessionStore.sys.mjs index a46aa6794b74..33bcb407424f 100644 --- a/browser/components/sessionstore/SessionStore.sys.mjs +++ b/browser/components/sessionstore/SessionStore.sys.mjs @@ -4557,24 +4557,7 @@ var SessionStoreInternal = { delete winData.hidden; } - let sidebarBox = aWindow.document.getElementById("sidebar-box"); - let command = sidebarBox.getAttribute("sidebarcommand"); - winData.sidebar = {}; - if (sidebarBox.style.width) { - winData.sidebar.width = sidebarBox.style.width; - } - if (command && sidebarBox.getAttribute("checked") == "true") { - winData.sidebar.command = command; - } else if (winData.sidebar?.command) { - delete winData.sidebar.command; - } - - if (aWindow.SidebarController.revampComponentsLoaded) { - winData.sidebar = Object.assign(winData.sidebar || {}, { - expanded: aWindow.SidebarController.sidebarMain.expanded, - hidden: aWindow.SidebarController.sidebarContainer.hidden, - }); - } + winData.sidebar = aWindow.SidebarController.getUIState(); let workspaceID = aWindow.getWorkspaceID(); if (workspaceID) { @@ -5632,30 +5615,8 @@ var SessionStoreInternal = { * Object containing command (sidebarcommand/category) and styles */ restoreSidebar(aWindow, aSidebar, isPopup) { - if (!aSidebar) { - return; - } if (!isPopup) { - let sidebarBox = aWindow.document.getElementById("sidebar-box"); - // Always restore sidebar width - if (aSidebar.width) { - sidebarBox.style.width = aSidebar.width; - } - if ( - aSidebar.command && - (sidebarBox.getAttribute("sidebarcommand") != aSidebar.command || - !sidebarBox.getAttribute("checked")) - ) { - aWindow.SidebarController.showInitially(aSidebar.command); - } - } - if (aWindow.SidebarController.sidebarRevampEnabled) { - const { SidebarController } = aWindow; - SidebarController.promiseInitialized.then(() => { - SidebarController.toggleExpanded(aSidebar.expanded); - SidebarController.sidebarContainer.hidden = aSidebar.hidden; - SidebarController.updateToolbarButton(); - }); + aWindow.SidebarController.setUIState(aSidebar); } }, diff --git a/browser/components/sessionstore/test/marionette/test_restore_sidebar.py b/browser/components/sessionstore/test/marionette/test_restore_sidebar.py index fa015a8ef817..1bcb2c8419e2 100644 --- a/browser/components/sessionstore/test/marionette/test_restore_sidebar.py +++ b/browser/components/sessionstore/test/marionette/test_restore_sidebar.py @@ -246,3 +246,44 @@ class TestSessionRestore(SessionStoreTestCase): ), "Sidebar visibility state has been restored.", ) + + def test_restore_sidebar_open_from_backup_pref(self): + self.marionette.execute_script( + """ + Services.prefs.setBoolPref("sidebar.revamp", true); + Services.prefs.setBoolPref("browser.privatebrowsing.autostart", true); + """ + ) + self.marionette.restart() + self.marionette.set_context("chrome") + + # Open the history panel. + self.marionette.execute_async_script( + """ + let resolve = arguments[0]; + let window = BrowserWindowTracker.getTopWindow(); + window.SidebarController.show("viewHistorySidebar").then(resolve); + """ + ) + + # Restart the browser. + self.marionette.restart() + self.marionette.execute_async_script( + """ + let resolve = arguments[0]; + let { BrowserInitState } = ChromeUtils.importESModule("resource:///modules/BrowserGlue.sys.mjs"); + BrowserInitState.startupIdleTaskPromise.then(resolve); + """ + ) + + # Check to see if the history panel was restored. + self.assertEqual( + self.marionette.execute_script( + """ + let window = BrowserWindowTracker.getTopWindow(); + return window.SidebarController.currentID; + """ + ), + "viewHistorySidebar", + "Correct sidebar category has been restored.", + ) diff --git a/browser/components/sidebar/SidebarManager.sys.mjs b/browser/components/sidebar/SidebarManager.sys.mjs index d7d2a356b746..017896b92120 100644 --- a/browser/components/sidebar/SidebarManager.sys.mjs +++ b/browser/components/sidebar/SidebarManager.sys.mjs @@ -5,6 +5,8 @@ import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs"; import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs"; +const BACKUP_STATE_PREF = "sidebar.backupState"; + const lazy = {}; ChromeUtils.defineESModuleGetters(lazy, { ExperimentAPI: "resource://nimbus/ExperimentAPI.sys.mjs", @@ -12,6 +14,11 @@ ChromeUtils.defineESModuleGetters(lazy, { PrefUtils: "resource://normandy/lib/PrefUtils.sys.mjs", }); XPCOMUtils.defineLazyPreferenceGetter(lazy, "sidebarNimbus", "sidebar.nimbus"); +XPCOMUtils.defineLazyPreferenceGetter( + lazy, + "sidebarBackupState", + BACKUP_STATE_PREF +); export const SidebarManager = { /** @@ -60,6 +67,35 @@ export const SidebarManager = { ); }); }, + + /** + * Provide a system-level "backup" state to be stored for those using "Never + * remember history" or "Clear history when browser closes". + * + * If it doesn't exist or isn't parsable, return `null`. + * + * @returns {object} + */ + getBackupState() { + try { + return JSON.parse(lazy.sidebarBackupState); + } catch (e) { + Services.prefs.clearUserPref(BACKUP_STATE_PREF); + return null; + } + }, + + /** + * Set the backup state. + * + * @param {object} state + */ + setBackupState(state) { + if (!state) { + return; + } + Services.prefs.setStringPref(BACKUP_STATE_PREF, JSON.stringify(state)); + }, }; // Initialize on first import diff --git a/browser/components/sidebar/browser-sidebar.js b/browser/components/sidebar/browser-sidebar.js index 19598ed0b46a..7b2c50b9435f 100644 --- a/browser/components/sidebar/browser-sidebar.js +++ b/browser/components/sidebar/browser-sidebar.js @@ -408,6 +408,14 @@ var SidebarController = { this._tabstripOrientationObserverAdded = true; } + requestIdleCallback(() => { + if (!this._uiState) { + // UI state has not been set by SessionStore. Use backup state for now. + const backupState = this.SidebarManager.getBackupState(); + this.setUIState(backupState); + } + }); + this._initDeferred.resolve(); }, @@ -420,8 +428,10 @@ var SidebarController = { let enumerator = Services.wm.getEnumerator("navigator:browser"); if (!enumerator.hasMoreElements()) { let xulStore = Services.xulStore; - xulStore.persist(this._title, "value"); + + const currentState = this.getUIState(); + this.SidebarManager.setBackupState(currentState); } Services.obs.removeObserver(this, "intl:app-locales-changed"); @@ -446,6 +456,50 @@ var SidebarController = { this.browser.removeEventListener("resize", this._browserResizeObserver); }, + getUIState() { + const state = { width: this._box.style.width, command: this.currentID }; + if (this.sidebarRevampEnabled) { + state.expanded = this.sidebarMain.expanded; + state.hidden = this.sidebarContainer.hidden; + } + return state; + }, + + /** + * Update and store the UI state of the sidebar for this window. + * + * @param {object} state + * @param {string} state.width + * Panel width of the sidebar. + * @param {string} state.command + * Panel ID that is currently open. + * @param {boolean} state.expanded + * Whether the sidebar launcher is expanded. (Revamp only) + * @param {boolean} state.hidden + * Whether the sidebar is hidden. (Revamp only) + */ + async setUIState(state) { + if (!state) { + return; + } + this._uiState = state; + if (state.width) { + this._box.style.width = state.width; + } + if (state.command && this.currentID != state.command && !this.isOpen) { + await this.showInitially(state.command); + } + if (this.sidebarRevampEnabled) { + // The `sidebar-main` component is lazy-loaded in the `init()` method. + // Wait this out to ensure that it is connected to the DOM before making + // any changes. + await this.promiseInitialized; + this.toggleExpanded(state.expanded); + this.sidebarContainer.hidden = state.hidden; + this.updateToolbarButton(); + } + }, + /** * The handler for Services.obs.addObserver. */ @@ -692,29 +746,8 @@ var SidebarController = { this._box.setAttribute("sidebarcommand", commandID); } - // Adopt `expanded` and `hidden` states only if the opener was also using - // revamped sidebar. - if (this.sidebarRevampEnabled && sourceController.revampComponentsLoaded) { - this.promiseInitialized.then(() => { - this.sidebarContainer.hidden = sourceController.sidebarContainer.hidden; - this.toggleExpanded(sourceController.sidebarMain.expanded); - }); - } - - if (sourceController._box.hidden) { - // just hidden means we have adopted the hidden state. - return true; - } - - // dynamically generated sidebars will fail this check, but we still - // consider it adopted. - if (!this.sidebars.has(commandID)) { - return true; - } - - this._box.style.width = sourceController._box.style.width; - this.showInitially(commandID); - + const sourceControllerState = sourceController.getUIState(); + this.setUIState(sourceControllerState); return true; },