From 069f2c2997b0acd869b58f557f6ac78550104f8e Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Wed, 23 Aug 2017 14:06:26 +0100 Subject: [PATCH] Bug 1391280 - store last sidebar command irrespective of whether sidebar was open, r=bgrins,mixedpuppy MozReview-Commit-ID: HBfdW5vEZaD --HG-- extra : rebase_source : 5058884698a2a2144b921a36abbe7c8c3a1fda52 --- browser/base/content/browser-sidebar.js | 44 +++++++++++++------ .../test/browser_sidebar_toggle.js | 26 +++++++---- .../components/sessionstore/SessionStore.jsm | 10 +++-- 3 files changed, 54 insertions(+), 26 deletions(-) diff --git a/browser/base/content/browser-sidebar.js b/browser/base/content/browser-sidebar.js index 1903c1d39c9b..d22529d6fb73 100644 --- a/browser/base/content/browser-sidebar.js +++ b/browser/base/content/browser-sidebar.js @@ -64,18 +64,24 @@ var SidebarUI = { }, uninit() { - let enumerator = Services.wm.getEnumerator(null); + // If this is the last browser window, persist various values that should be + // remembered for after a restart / reopening a browser window. + let enumerator = Services.wm.getEnumerator("navigator:browser"); enumerator.getNext(); if (!enumerator.hasMoreElements()) { document.persist("sidebar-box", "sidebarcommand"); let xulStore = Cc["@mozilla.org/xul/xulstore;1"].getService(Ci.nsIXULStore); - if (this._box.hasAttribute("positionend")) { document.persist("sidebar-box", "positionend"); } else { xulStore.removeValue(document.documentURI, "sidebar-box", "positionend"); } + if (this._box.hasAttribute("checked")) { + document.persist("sidebar-box", "checked"); + } else { + xulStore.removeValue(document.documentURI, "sidebar-box", "checked"); + } document.persist("sidebar-box", "width"); document.persist("sidebar-title", "value"); @@ -180,13 +186,19 @@ var SidebarUI = { // no source UI or no _box means we also can't adopt the state. return false; } + + // Set sidebar command even if hidden, so that we keep the same sidebar + // even if it's currently closed. + let commandID = sourceUI._box.getAttribute("sidebarcommand"); + if (commandID) { + this._box.setAttribute("sidebarcommand", commandID); + } + if (sourceUI._box.hidden) { // just hidden means we have adopted the hidden state. return true; } - let commandID = sourceUI._box.getAttribute("sidebarcommand"); - // dynamically generated sidebars will fail this check, but we still // consider it adopted. if (!document.getElementById(commandID)) { @@ -223,18 +235,22 @@ var SidebarUI = { } // If we're not adopting settings from a parent window, set them now. - let commandID = this._box.getAttribute("sidebarcommand"); - if (!commandID) { + let wasOpen = this._box.getAttribute("checked"); + if (!wasOpen) { return; } - if (document.getElementById(commandID)) { + let commandID = this._box.getAttribute("sidebarcommand"); + if (commandID && document.getElementById(commandID)) { this.showInitially(commandID); } else { + this._box.removeAttribute("checked"); // Remove the |sidebarcommand| attribute, because the element it // refers to no longer exists, so we should assume this sidebar // panel has been uninstalled. (249883) - this._box.removeAttribute("sidebarcommand"); + // We use setAttribute rather than removeAttribute so it persists + // correctly. + this._box.setAttribute("sidebarcommand", ""); // On a startup in which the startup cache was invalidated (e.g. app update) // extensions will not be started prior to delayedLoad, thus the // sidebarcommand element will not exist yet. Store the commandID so @@ -274,10 +290,9 @@ var SidebarUI = { /** * The ID of the current sidebar (ie, the ID of the broadcaster being used). - * This can be set even if the sidebar is hidden. */ get currentID() { - return this._box.getAttribute("sidebarcommand"); + return this.isOpen ? this._box.getAttribute("sidebarcommand") : ""; }, get title() { @@ -308,8 +323,12 @@ var SidebarUI = { */ toggle(commandID = this.lastOpenedId, triggerNode) { // First priority for a default value is this.lastOpenedId which is set during show() - // and not reset in hide(), unlike currentID. If show() hasn't been called or the command - // doesn't exist anymore, then fallback to a default sidebar. + // and not reset in hide(), unlike currentID. If show() hasn't been called and we don't + // have a persisted command either, or the command doesn't exist anymore, then + // fallback to a default sidebar. + if (!commandID) { + commandID = this._box.getAttribute("sidebarcommand"); + } if (!commandID || !this.getBroadcasterById(commandID)) { commandID = this.DEFAULT_SIDEBAR_ID; } @@ -456,7 +475,6 @@ var SidebarUI = { this.browser.docShell.createAboutBlankContentViewer(null); sidebarBroadcaster.removeAttribute("checked"); - this._box.setAttribute("sidebarcommand", ""); this._box.removeAttribute("checked"); this._box.hidden = this._splitter.hidden = true; diff --git a/browser/components/customizableui/test/browser_sidebar_toggle.js b/browser/components/customizableui/test/browser_sidebar_toggle.js index ce53a37250e9..18136102eab3 100644 --- a/browser/components/customizableui/test/browser_sidebar_toggle.js +++ b/browser/components/customizableui/test/browser_sidebar_toggle.js @@ -13,19 +13,19 @@ registerCleanupFunction(async function() { } }); -var showSidebar = async function() { - let button = document.getElementById("sidebar-button"); - let sidebarFocusedPromise = BrowserTestUtils.waitForEvent(document, "SidebarFocused"); - EventUtils.synthesizeMouseAtCenter(button, {}); +var showSidebar = async function(win = window) { + let button = win.document.getElementById("sidebar-button"); + let sidebarFocusedPromise = BrowserTestUtils.waitForEvent(win.document, "SidebarFocused"); + EventUtils.synthesizeMouseAtCenter(button, {}, win); await sidebarFocusedPromise; - ok(SidebarUI.isOpen, "Sidebar is opened"); + ok(win.SidebarUI.isOpen, "Sidebar is opened"); ok(button.hasAttribute("checked"), "Toolbar button is checked"); }; -var hideSidebar = async function() { - let button = document.getElementById("sidebar-button"); - EventUtils.synthesizeMouseAtCenter(button, {}); - ok(!SidebarUI.isOpen, "Sidebar is closed"); +var hideSidebar = async function(win = window) { + let button = win.document.getElementById("sidebar-button"); + EventUtils.synthesizeMouseAtCenter(button, {}, win); + ok(!win.SidebarUI.isOpen, "Sidebar is closed"); ok(!button.hasAttribute("checked"), "Toolbar button isn't checked"); }; @@ -40,4 +40,12 @@ add_task(async function() { await hideSidebar(); await showSidebar(); is(SidebarUI.currentID, "viewHistorySidebar", "Selected sidebar remembered"); + + await hideSidebar(); + let otherWin = await BrowserTestUtils.openNewBrowserWindow({opener: window}); + await showSidebar(otherWin); + is(otherWin.SidebarUI.currentID, "viewHistorySidebar", "Selected sidebar remembered across windows"); + await hideSidebar(otherWin); + + await BrowserTestUtils.closeWindow(otherWin); }); diff --git a/browser/components/sessionstore/SessionStore.jsm b/browser/components/sessionstore/SessionStore.jsm index 5f2582a5b44e..82637db7a94e 100644 --- a/browser/components/sessionstore/SessionStore.jsm +++ b/browser/components/sessionstore/SessionStore.jsm @@ -3075,8 +3075,9 @@ var SessionStoreInternal = { else if (winData.hidden) delete winData.hidden; - var sidebar = aWindow.document.getElementById("sidebar-box").getAttribute("sidebarcommand"); - if (sidebar) + let sidebarBox = aWindow.document.getElementById("sidebar-box"); + let sidebar = sidebarBox.getAttribute("sidebarcommand"); + if (sidebar && sidebarBox.getAttribute("checked") == "true") winData.sidebar = sidebar; else if (winData.sidebar) delete winData.sidebar; @@ -4089,8 +4090,9 @@ var SessionStoreInternal = { break; } } - var sidebar = aWindow.document.getElementById("sidebar-box"); - if (sidebar.getAttribute("sidebarcommand") != aSidebar) { + let sidebarBox = aWindow.document.getElementById("sidebar-box"); + if (aSidebar && (sidebarBox.getAttribute("sidebarcommand") != aSidebar || + !sidebarBox.getAttribute("checked"))) { aWindow.SidebarUI.showInitially(aSidebar); } // since resizing/moving a window brings it to the foreground,