Bug 1365637 place WE sidebars into the photon sidebar dropdown, r?mattw,Gijs draft
authorShane Caraveo <scaraveo@mozilla.com>
Mon, 29 May 2017 15:20:48 -0700
changeset 586183 4752ac1287f384fcae6f1bffd7bc174c4769705b
parent 586049 34ac1a5d6576d6775491c8a882710a1520551da6
child 630913 8fcbed60d48891d3b16b59aabc8cdfb1d612d5e0
push id61322
push usermixedpuppy@gmail.com
push dateMon, 29 May 2017 22:21:08 +0000
reviewersmattw, Gijs
bugs1365637
milestone55.0a1
Bug 1365637 place WE sidebars into the photon sidebar dropdown, r?mattw,Gijs MozReview-Commit-ID: 3ZM9mXjEbWh
browser/base/content/browser.xul
browser/components/extensions/ext-sidebarAction.js
browser/components/extensions/test/browser/browser_ext_sidebarAction_windows.js
browser/themes/shared/sidebar.inc.css
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -316,16 +316,18 @@
       <toolbarbutton id="sidebar-switcher-tabs"
                      label="&syncedTabs.sidebar.label;"
                      class="subviewbutton subviewbutton-iconic"
                      observes="viewTabsSidebar"
                      oncommand="SidebarUI.show('viewTabsSidebar');">
          <observes element="viewTabsSidebar" attribute="checked"/>
        </toolbarbutton>
       <toolbarseparator/>
+      <vbox id="sidebar-extensions"></vbox>
+      <toolbarseparator/>
       <toolbarbutton id="sidebar-reverse-position"
                      class="subviewbutton"
                      oncommand="SidebarUI.reversePosition()"/>
       <toolbarseparator/>
       <toolbarbutton label="&sidebarMenuClose.label;"
                      class="subviewbutton"
                      oncommand="SidebarUI.hide()"/>
     </panel>
--- a/browser/components/extensions/ext-sidebarAction.js
+++ b/browser/components/extensions/ext-sidebarAction.js
@@ -37,16 +37,17 @@ this.sidebarAction = class extends Exten
 
     let options = extension.manifest.sidebar_action;
 
     // Add the extension to the sidebar menu.  The sidebar widget will copy
     // from that when it is viewed, so we shouldn't need to update that.
     let widgetId = makeWidgetId(extension.id);
     this.id = `${widgetId}-sidebar-action`;
     this.menuId = `menu_${this.id}`;
+    this.buttonId = `button_${this.id}`;
 
     // We default browser_style to true because this is a new API and
     // we therefore don't need to worry about breaking existing add-ons.
     this.browserStyle = options.browser_style || options.browser_style === null;
 
     this.defaults = {
       enabled: true,
       title: options.default_title || extension.name,
@@ -85,16 +86,20 @@ this.sidebarAction = class extends Exten
       let {document, SidebarUI} = window;
       if (SidebarUI.currentID === this.id) {
         SidebarUI.hide();
       }
       let menu = document.getElementById(this.menuId);
       if (menu) {
         menu.remove();
       }
+      let button = document.getElementById(this.buttonId);
+      if (button) {
+        button.remove();
+      }
       let broadcaster = document.getElementById(this.id);
       if (broadcaster) {
         broadcaster.remove();
       }
     }
     windowTracker.removeOpenListener(this.windowOpenListener);
   }
 
@@ -147,27 +152,35 @@ this.sidebarAction = class extends Exten
     broadcaster.setAttribute("autoCheck", "false");
     broadcaster.setAttribute("type", "checkbox");
     broadcaster.setAttribute("group", "sidebar");
     broadcaster.setAttribute("label", details.title);
     broadcaster.setAttribute("sidebarurl", this.sidebarUrl(details.panel));
 
     // oncommand gets attached to menuitem, so we use the observes attribute to
     // get the command id we pass to SidebarUI.
-    broadcaster.setAttribute("oncommand", "SidebarUI.toggle(this.getAttribute('observes'))");
+    broadcaster.setAttribute("oncommand", "SidebarUI.show(this.getAttribute('observes'))");
 
+    // Insert a menuitem for View->Show Sidebars.
     let menuitem = document.createElementNS(XUL_NS, "menuitem");
     menuitem.setAttribute("id", this.menuId);
     menuitem.setAttribute("observes", this.id);
     menuitem.setAttribute("class", "menuitem-iconic webextension-menuitem");
+    this.setMenuIcon(menuitem, details);
 
-    this.setMenuIcon(menuitem, details);
+    // Insert a toolbarbutton for the sidebar dropdown selector.
+    let toolbarbutton = document.createElementNS(XUL_NS, "toolbarbutton");
+    toolbarbutton.setAttribute("id", this.buttonId);
+    toolbarbutton.setAttribute("observes", this.id);
+    toolbarbutton.setAttribute("class", "subviewbutton subviewbutton-iconic webextension-menuitem");
+    this.setMenuIcon(toolbarbutton, details);
 
     document.getElementById("mainBroadcasterSet").appendChild(broadcaster);
     document.getElementById("viewSidebarMenu").appendChild(menuitem);
+    document.getElementById("sidebar-extensions").appendChild(toolbarbutton);
 
     return menuitem;
   }
 
   setMenuIcon(menuitem, details) {
     let getIcon = size => IconDetails.escapeUrl(
       IconDetails.getPreferredIcon(details.icon, this.extension, size).icon);
 
@@ -202,16 +215,19 @@ this.sidebarAction = class extends Exten
     let url = this.sidebarUrl(tabData.panel);
     let urlChanged = url !== broadcaster.getAttribute("sidebarurl");
     if (urlChanged) {
       broadcaster.setAttribute("sidebarurl", url);
     }
 
     this.setMenuIcon(menu, tabData);
 
+    let button = document.getElementById(this.buttonId);
+    this.setMenuIcon(button, tabData);
+
     // Update the sidebar if this extension is the current sidebar.
     if (SidebarUI.currentID === this.id) {
       SidebarUI.title = title;
       if (SidebarUI.isOpen && urlChanged) {
         SidebarUI.show(this.id);
       }
     }
   }
--- a/browser/components/extensions/test/browser/browser_ext_sidebarAction_windows.js
+++ b/browser/components/extensions/test/browser/browser_ext_sidebarAction_windows.js
@@ -33,31 +33,33 @@ let extData = {
 add_task(async function sidebar_windows() {
   let extension = ExtensionTestUtils.loadExtension(extData);
   await extension.startup();
   // Test sidebar is opened on install
   await extension.awaitMessage("sidebar");
   ok(!document.getElementById("sidebar-box").hidden, "sidebar box is visible in first window");
   // Check that the menuitem has our image styling.
   let elements = document.getElementsByClassName("webextension-menuitem");
-  is(elements.length, 1, "have one menuitem");
+  // ui is in flux, at time of writing we potentially have 3 menuitems, later
+  // it may be two or one, just make sure one is there.
+  ok(elements.length > 0, "have a menuitem");
   let style = elements[0].getAttribute("style");
   ok(style.includes("webextension-menuitem-image"), "this menu has style");
 
   let secondSidebar = extension.awaitMessage("sidebar");
 
   // SidebarUI relies on window.opener being set, which is normal behavior when
   // using menu or key commands to open a new browser window.
   let win = await BrowserTestUtils.openNewBrowserWindow({opener: window});
 
   await secondSidebar;
   ok(!win.document.getElementById("sidebar-box").hidden, "sidebar box is visible in second window");
   // Check that the menuitem has our image styling.
   elements = win.document.getElementsByClassName("webextension-menuitem");
-  is(elements.length, 1, "have one menuitem");
+  ok(elements.length > 0, "have a menuitem");
   style = elements[0].getAttribute("style");
   ok(style.includes("webextension-menuitem-image"), "this menu has style");
 
   await extension.unload();
   await BrowserTestUtils.closeWindow(win);
 
   // Move toolbar button back to customization.
   CustomizableUI.removeWidgetFromArea("sidebar-button", CustomizableUI.AREA_NAVBAR);
--- a/browser/themes/shared/sidebar.inc.css
+++ b/browser/themes/shared/sidebar.inc.css
@@ -84,16 +84,20 @@
 #sidebar-close:hover {
   background: var(--header-background-color-hover);
 }
 
 #sidebarMenu-popup .subviewbutton {
   min-width: 190px;
 }
 
+#sidebar-extensions:empty + toolbarseparator {
+  display: none;
+}
+
 %ifndef XP_MACOSX
 /* Allow room for the checkbox drawn as a background image at the start of the toolbarbutton */
 #sidebarMenu-popup .subviewbutton-iconic > .toolbarbutton-icon {
   padding-inline-start: 16px;
 }
 #sidebarMenu-popup .subviewbutton-iconic > .toolbarbutton-text {
   padding-inline-start: 0;
 }