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

@@ -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;
 

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);
 });

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,