Bug 1360282 - TO FOLD INTO PREVIOUS PATCH;r=Gijs draft
authorBrian Grinstead <bgrinstead@mozilla.com>
Mon, 12 Jun 2017 17:25:46 -0700
changeset 592950 ff8277dbba348a71b56348c0f4417a68f0ee7408
parent 592949 d2dac15a931f2275d69699c072c388649f99d250
child 632972 e6b35cb5631ec72928861dff82f44bf12c8f2b8d
push id63547
push userbgrinstead@mozilla.com
push dateTue, 13 Jun 2017 00:26:20 +0000
reviewersGijs
bugs1360282
milestone56.0a1
Bug 1360282 - TO FOLD INTO PREVIOUS PATCH;r=Gijs MozReview-Commit-ID: D4m0fOz1qyf
browser/base/content/browser-sidebar.js
browser/components/customizableui/CustomizableWidgets.jsm
--- a/browser/base/content/browser-sidebar.js
+++ b/browser/base/content/browser-sidebar.js
@@ -23,16 +23,18 @@ var SidebarUI = {
   // Avoid getting the browser element from init() to avoid triggering the
   // <browser> constructor during startup if the sidebar is hidden.
   get browser() {
     if (this._browser)
       return this._browser;
     return this._browser = document.getElementById("sidebar");
   },
   POSITION_START_PREF: "sidebar.position_start",
+  DEFAULT_SIDEBAR_ID: "viewBookmarksSidebar",
+  lastOpenedId: null,
 
   _box: null,
   // The constructor of this label accesses the browser element due to the
   // control="sidebar" attribute, so avoid getting this label during startup.
   get _title() {
     if (this.__title)
       return this.__title;
     return this.__title = document.getElementById("sidebar-title");
@@ -253,25 +255,41 @@ var SidebarUI = {
   get title() {
     return this._title.value;
   },
 
   set title(value) {
     this._title.value = value;
   },
 
+  getBroadcasterById(id) {
+    let sidebarBroadcaster = document.getElementById(id);
+    if (sidebarBroadcaster && sidebarBroadcaster.localName == "broadcaster") {
+      return sidebarBroadcaster;
+    }
+    return null;
+  },
+
   /**
    * Toggle the visibility of the sidebar. If the sidebar is hidden or is open
    * with a different commandID, then the sidebar will be opened using the
    * specified commandID. Otherwise the sidebar will be hidden.
    *
    * @param {string} commandID ID of the xul:broadcaster element to use.
    * @return {Promise}
    */
-  toggle(commandID = this.currentID) {
+  toggle(commandID) {
+    // 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.
+    commandID = commandID || this.lastOpenedId;
+    if (!commandID || !this.getBroadcasterById(commandID)) {
+      commandID = this.DEFAULT_SIDEBAR_ID;
+    }
+
     if (this.isOpen && commandID == this.currentID) {
       this.hide();
       return Promise.resolve();
     }
     return this.show(commandID);
   },
 
   /**
@@ -291,18 +309,18 @@ var SidebarUI = {
   /**
    * Implementation for show. Also used internally for sidebars that are shown
    * when a window is opened and we don't want to ping telemetry.
    *
    * @param {string} commandID ID of the xul:broadcaster element to use.
    */
   _show(commandID) {
     return new Promise((resolve, reject) => {
-      let sidebarBroadcaster = document.getElementById(commandID);
-      if (!sidebarBroadcaster || sidebarBroadcaster.localName != "broadcaster") {
+      let sidebarBroadcaster = this.getBroadcasterById(commandID);
+      if (!sidebarBroadcaster) {
         reject(new Error("Invalid sidebar broadcaster specified: " + commandID));
         return;
       }
 
       if (this.isOpen && commandID != this.currentID) {
         BrowserUITelemetry.countSidebarEvent(this.currentID, "hide");
       }
 
--- a/browser/components/customizableui/CustomizableWidgets.jsm
+++ b/browser/components/customizableui/CustomizableWidgets.jsm
@@ -594,17 +594,17 @@ const CustomizableWidgets = [
       let win = aEvent.target.ownerGlobal;
       win.BrowserOpenFileWindow();
     }
   }, {
     id: "sidebar-button",
     tooltiptext: "sidebar-button.tooltiptext2",
     onCommand(aEvent) {
       let win = aEvent.target.ownerGlobal;
-      win.SidebarUI.toggle(win.SidebarUI.lastOpenedId || "viewBookmarksSidebar");
+      win.SidebarUI.toggle();
     },
     onCreated(aNode) {
       // Add an observer so the button is checked while the sidebar is open
       let doc = aNode.ownerDocument;
       let obnode = doc.createElementNS(kNSXUL, "observes");
       obnode.setAttribute("element", "sidebar-box");
       obnode.setAttribute("attribute", "checked");
       aNode.appendChild(obnode);