Bug 1482610 - Part 2 - Move the remaining attributes and remove the sidebar broadcasters. r=jaws,mixedpuppy
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Sun, 19 Aug 2018 19:54:02 +0100
changeset 432384 be8aca082faf9aef30236657c9c486750e831a21
parent 432383 8ceb8813cc1d7beb81e044d411c0bac6babea9da
child 432385 5ed210553fd0f580bd9369201596acda13d62690
push id106720
push userpaolo.mozmail@amadzone.org
push dateMon, 20 Aug 2018 15:48:29 +0000
treeherdermozilla-inbound@be8aca082faf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjaws, mixedpuppy
bugs1482610
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1482610 - Part 2 - Move the remaining attributes and remove the sidebar broadcasters. r=jaws,mixedpuppy This also fixes unintended behavior for which clicking the selected item in the sidebar selector would hide the sidebar. Differential Revision: https://phabricator.services.mozilla.com/D3145
browser/base/content/browser-menubar.inc
browser/base/content/browser-sets.inc
browser/base/content/browser-sidebar.js
browser/base/content/browser.xul
browser/base/content/test/sidebar/browser.ini
browser/base/content/test/sidebar/browser_sidebar_keys.js
browser/components/customizableui/content/panelUI.inc.xul
browser/components/extensions/parent/ext-sidebarAction.js
browser/components/extensions/test/browser/browser_ext_sidebarAction_context.js
--- a/browser/base/content/browser-menubar.inc
+++ b/browser/base/content/browser-menubar.inc
@@ -194,26 +194,29 @@
                               command="cmd_CustomizeToolbars"/>
                   </menupopup>
                 </menu>
                 <menu id="viewSidebarMenuMenu"
                       label="&viewSidebarMenu.label;"
                       accesskey="&viewSidebarMenu.accesskey;">
                   <menupopup id="viewSidebarMenu">
                     <menuitem id="menu_bookmarksSidebar"
+                              type="checkbox"
                               key="viewBookmarksSidebarKb"
-                              observes="viewBookmarksSidebar"
+                              oncommand="SidebarUI.toggle('viewBookmarksSidebar');"
                               label="&bookmarksButton.label;"/>
                     <menuitem id="menu_historySidebar"
+                              type="checkbox"
                               key="key_gotoHistory"
-                              observes="viewHistorySidebar"
+                              oncommand="SidebarUI.toggle('viewHistorySidebar');"
                               label="&historyButton.label;"/>
                     <menuitem id="menu_tabsSidebar"
+                              type="checkbox"
                               class="sync-ui-item"
-                              observes="viewTabsSidebar"
+                              oncommand="SidebarUI.toggle('viewTabsSidebar');"
                               label="&syncedTabs.sidebar.label;"/>
                   </menupopup>
                 </menu>
                 <menuseparator/>
                 <menu id="viewFullZoomMenu" label="&fullZoom.label;"
                       accesskey="&fullZoom.accesskey;"
                       onpopupshowing="FullZoom.updateMenu();">
                   <menupopup>
--- a/browser/base/content/browser-sets.inc
+++ b/browser/base/content/browser-sets.inc
@@ -117,42 +117,31 @@
              label="&zoomWindow.label;"
              oncommand="zoomWindow();" />
 #endif
   </commandset>
 
 #include ../../components/places/content/placesCommands.inc.xul
 
   <broadcasterset id="mainBroadcasterSet">
-    <broadcaster id="viewBookmarksSidebar"
-                 type="checkbox" group="sidebar"
-                 oncommand="SidebarUI.toggle('viewBookmarksSidebar');"/>
-
-    <broadcaster id="viewHistorySidebar"
-                 type="checkbox" group="sidebar"
-                 oncommand="SidebarUI.toggle('viewHistorySidebar');"/>
-
     <broadcaster id="isImage"/>
     <broadcaster id="canViewSource"/>
     <broadcaster id="isFrameImage"/>
 
     <!-- Sync broadcasters -->
     <!-- A broadcaster of a number of attributes suitable for "sync now" UI -
         A 'syncstatus' attribute is set while actively syncing, and the label
         attribute which changes from "sync now" to "syncing" etc. -->
     <broadcaster id="sync-status" onmouseover="gSync.refreshSyncButtonsTooltip();"/>
     <!-- broadcasters of the "hidden" attribute to reflect setup state for
          menus -->
     <broadcaster id="sync-setup-state" hidden="true"/>
     <broadcaster id="sync-unverified-state" hidden="true"/>
     <broadcaster id="sync-syncnow-state" hidden="true"/>
     <broadcaster id="sync-reauth-state" hidden="true"/>
-    <broadcaster id="viewTabsSidebar"
-                 type="checkbox" group="sidebar"
-                 oncommand="SidebarUI.toggle('viewTabsSidebar');"/>
     <broadcaster id="workOfflineMenuitemState"/>
     <broadcaster id="devtoolsMenuBroadcaster_RecordExecution"
                  label="&devtoolsRecordExecution.label;"
                  command="Tools:RecordExecution"/>
     <broadcaster id="devtoolsMenuBroadcaster_SaveRecording"
                  label="&devtoolsSaveRecording.label;"
                  command="Tools:SaveRecording"/>
     <broadcaster id="devtoolsMenuBroadcaster_ReplayExecution"
@@ -281,37 +270,37 @@
     <key id="addBookmarkAsKb" key="&bookmarkThisPageCmd.commandkey;" command="Browser:AddBookmarkAs" modifiers="accel"/>
 # Accel+Shift+A-F are reserved on GTK
 #ifndef MOZ_WIDGET_GTK
     <key id="bookmarkAllTabsKb" key="&bookmarkThisPageCmd.commandkey;" oncommand="PlacesCommandHook.bookmarkPages(PlacesCommandHook.uniqueCurrentPages);" modifiers="accel,shift"/>
     <key id="manBookmarkKb" key="&bookmarksCmd.commandkey;" command="Browser:ShowAllBookmarks" modifiers="accel,shift"/>
 #else
     <key id="manBookmarkKb" key="&bookmarksGtkCmd.commandkey;" command="Browser:ShowAllBookmarks" modifiers="accel,shift"/>
 #endif
-    <key id="viewBookmarksSidebarKb" key="&bookmarksCmd.commandkey;" command="viewBookmarksSidebar" modifiers="accel"/>
+    <key id="viewBookmarksSidebarKb" key="&bookmarksCmd.commandkey;" oncommand="SidebarUI.toggle('viewBookmarksSidebar');" modifiers="accel"/>
 #ifdef XP_WIN
 # Cmd+I is conventially mapped to Info on MacOS X, thus it should not be
 # overridden for other purposes there.
-    <key id="viewBookmarksSidebarWinKb" key="&bookmarksWinCmd.commandkey;" command="viewBookmarksSidebar" modifiers="accel"/>
+    <key id="viewBookmarksSidebarWinKb" key="&bookmarksWinCmd.commandkey;" oncommand="SidebarUI.toggle('viewBookmarksSidebar');" modifiers="accel"/>
 #endif
 
     <key id="key_stop" keycode="VK_ESCAPE" command="Browser:Stop"/>
 
 #ifdef XP_MACOSX
     <key id="key_stop_mac" modifiers="accel" key="&stopCmd.macCommandKey;" command="Browser:Stop"/>
 #endif
 
     <key id="key_gotoHistory"
          key="&historySidebarCmd.commandKey;"
 #ifdef XP_MACOSX
          modifiers="accel,shift"
 #else
          modifiers="accel"
 #endif
-         command="viewHistorySidebar"/>
+         oncommand="SidebarUI.toggle('viewHistorySidebar');"/>
 
     <key id="key_fullZoomReduce"  key="&fullZoomReduceCmd.commandkey;"   command="cmd_fullZoomReduce"  modifiers="accel"/>
     <key                          key="&fullZoomReduceCmd.commandkey2;"  command="cmd_fullZoomReduce"  modifiers="accel"/>
     <key id="key_fullZoomEnlarge" key="&fullZoomEnlargeCmd.commandkey;"  command="cmd_fullZoomEnlarge" modifiers="accel"/>
     <key                          key="&fullZoomEnlargeCmd.commandkey2;" command="cmd_fullZoomEnlarge" modifiers="accel"/>
     <key                          key="&fullZoomEnlargeCmd.commandkey3;" command="cmd_fullZoomEnlarge" modifiers="accel"/>
     <key id="key_fullZoomReset"   key="&fullZoomResetCmd.commandkey;"    command="cmd_fullZoomReset"   modifiers="accel"/>
     <key                          key="&fullZoomResetCmd.commandkey2;"   command="cmd_fullZoomReset"   modifiers="accel"/>
--- a/browser/base/content/browser-sidebar.js
+++ b/browser/base/content/browser-sidebar.js
@@ -10,26 +10,32 @@ var SidebarUI = {
     if (this._sidebars) {
       return this._sidebars;
     }
     return this._sidebars = new Map([
       ["viewBookmarksSidebar", {
         title: document.getElementById("sidebar-switcher-bookmarks")
                        .getAttribute("label"),
         url: "chrome://browser/content/places/bookmarksSidebar.xul",
+        menuId: "menu_bookmarksSidebar",
+        buttonId: "sidebar-switcher-bookmarks",
       }],
       ["viewHistorySidebar", {
         title: document.getElementById("sidebar-switcher-history")
                        .getAttribute("label"),
         url: "chrome://browser/content/places/historySidebar.xul",
+        menuId: "menu_historySidebar",
+        buttonId: "sidebar-switcher-history",
       }],
       ["viewTabsSidebar", {
         title: document.getElementById("sidebar-switcher-tabs")
                        .getAttribute("label"),
         url: "chrome://browser/content/syncedtabs/sidebar.xhtml",
+        menuId: "menu_tabsSidebar",
+        buttonId: "sidebar-switcher-tabs",
       }],
     ]);
   },
 
   // 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)
@@ -230,17 +236,17 @@ var SidebarUI = {
 
     if (sourceUI._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 (!document.getElementById(commandID)) {
+    if (!this.sidebars.has(commandID)) {
       return true;
     }
 
     this._box.setAttribute("width", sourceUI._box.boxObject.width);
     this.showInitially(commandID);
 
     return true;
   },
@@ -270,17 +276,17 @@ var SidebarUI = {
 
     // If we're not adopting settings from a parent window, set them now.
     let wasOpen = this._box.getAttribute("checked");
     if (!wasOpen) {
       return;
     }
 
     let commandID = this._box.getAttribute("sidebarcommand");
-    if (commandID && document.getElementById(commandID)) {
+    if (commandID && this.sidebars.has(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)
       // We use setAttribute rather than removeAttribute so it persists
       // correctly.
@@ -318,160 +324,138 @@ var SidebarUI = {
   /**
    * True if the sidebar is currently open.
    */
   get isOpen() {
     return !this._box.hidden;
   },
 
   /**
-   * The ID of the current sidebar (ie, the ID of the broadcaster being used).
+   * The ID of the current sidebar.
    */
   get currentID() {
     return this.isOpen ? this._box.getAttribute("sidebarcommand") : "";
   },
 
   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.
+   * @param  {string}  commandID     ID of the sidebar.
    * @param  {DOMNode} [triggerNode] Node, usually a button, that triggered the
    *                                 visibility toggling of the sidebar.
    * @return {Promise}
    */
   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 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)) {
+    if (!commandID || !this.sidebars.has(commandID)) {
       commandID = this.DEFAULT_SIDEBAR_ID;
     }
 
     if (this.isOpen && commandID == this.currentID) {
       this.hide(triggerNode);
       return Promise.resolve();
     }
     return this.show(commandID, triggerNode);
   },
 
-  _loadSidebarExtension(sidebarBroadcaster) {
-    let extensionId = sidebarBroadcaster.getAttribute("extensionId");
+  _loadSidebarExtension(commandID) {
+    let sidebar = this.sidebars.get(commandID);
+    let {extensionId} = sidebar;
     if (extensionId) {
-      let extensionUrl = sidebarBroadcaster.getAttribute("panel");
-      let browserStyle = sidebarBroadcaster.getAttribute("browserStyle");
-      SidebarUI.browser.contentWindow.loadPanel(extensionId, extensionUrl, browserStyle);
+      SidebarUI.browser.contentWindow.loadPanel(extensionId, sidebar.panel,
+                                                sidebar.browserStyle);
     }
   },
 
   /**
-   * Show the sidebar, using the parameters from the specified broadcaster.
-   * @see SidebarUI note.
+   * Show the sidebar.
    *
    * This wraps the internal method, including a ping to telemetry.
    *
-   * @param {string}  commandID     ID of the xul:broadcaster element to use.
+   * @param {string}  commandID     ID of the sidebar to use.
    * @param {DOMNode} [triggerNode] Node, usually a button, that triggered the
    *                                showing of the sidebar.
    */
   show(commandID, triggerNode) {
-    return this._show(commandID).then((sidebarBroadcaster) => {
-      this._loadSidebarExtension(sidebarBroadcaster);
+    return this._show(commandID).then(() => {
+      this._loadSidebarExtension(commandID);
 
       if (triggerNode) {
         updateToggleControlLabel(triggerNode);
       }
 
       this._fireFocusedEvent();
     });
   },
 
   /**
    * Show the sidebar, without firing the focused event or logging telemetry.
    * This is intended to be used when the sidebar is opened automatically
    * when a window opens (not triggered by user interaction).
    *
-   * @param {string} commandID ID of the xul:broadcaster element to use.
+   * @param {string} commandID ID of the sidebar.
    */
   showInitially(commandID) {
-    return this._show(commandID).then((sidebarBroadcaster) => {
-      this._loadSidebarExtension(sidebarBroadcaster);
+    return this._show(commandID).then(() => {
+      this._loadSidebarExtension(commandID);
     });
   },
 
   /**
    * 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.
+   * @param {string} commandID ID of the sidebar.
    */
   _show(commandID) {
-    return new Promise((resolve, reject) => {
-      let sidebarBroadcaster = this.getBroadcasterById(commandID);
-      if (!sidebarBroadcaster) {
-        reject(new Error("Invalid sidebar broadcaster specified: " + commandID));
-        return;
-      }
-
-      let broadcasters = document.querySelectorAll("broadcaster[group=sidebar]");
-      for (let broadcaster of broadcasters) {
-        if (broadcaster != sidebarBroadcaster) {
-          broadcaster.removeAttribute("checked");
-        } else {
-          sidebarBroadcaster.setAttribute("checked", "true");
-        }
-      }
+    return new Promise(resolve => {
+      this.selectMenuItem(commandID);
 
       this._box.hidden = this._splitter.hidden = false;
       this.setPosition();
 
       this.hideSwitcherPanel();
 
       this._box.setAttribute("checked", "true");
-      this._box.setAttribute("sidebarcommand", sidebarBroadcaster.id);
-      this.lastOpenedId = sidebarBroadcaster.id;
+      this._box.setAttribute("sidebarcommand", commandID);
+      this.lastOpenedId = commandID;
 
       let {url, title} = this.sidebars.get(commandID);
       this.title = title;
       this.browser.setAttribute("src", url); // kick off async load
 
       if (this.browser.contentDocument.location.href != url) {
         this.browser.addEventListener("load", event => {
           // We're handling the 'load' event before it bubbles up to the usual
           // (non-capturing) event handlers. Let it bubble up before resolving.
           setTimeout(() => {
-            resolve(sidebarBroadcaster);
+            resolve();
 
             // Now that the currentId is updated, fire a show event.
             this._fireShowEvent();
           }, 0);
         }, {capture: true, once: true});
       } else {
-        resolve(sidebarBroadcaster);
+        resolve();
 
         // Now that the currentId is updated, fire a show event.
         this._fireShowEvent();
       }
 
       let selBrowser = gBrowser.selectedBrowser;
       selBrowser.messageManager.sendAsyncMessage("Sidebar:VisibilityChange",
         {commandID, isOpen: true}
@@ -488,43 +472,56 @@ var SidebarUI = {
   hide(triggerNode) {
     if (!this.isOpen) {
       return;
     }
 
     this.hideSwitcherPanel();
 
     let commandID = this._box.getAttribute("sidebarcommand");
-    let sidebarBroadcaster = document.getElementById(commandID);
-
-    if (sidebarBroadcaster.getAttribute("checked") != "true") {
-      return;
-    }
+    this.selectMenuItem("");
 
     // Replace the document currently displayed in the sidebar with about:blank
     // so that we can free memory by unloading the page. We need to explicitly
     // create a new content viewer because the old one doesn't get destroyed
     // until about:blank has loaded (which does not happen as long as the
     // element is hidden).
     this.browser.setAttribute("src", "about:blank");
     this.browser.docShell.createAboutBlankContentViewer(null);
 
-    sidebarBroadcaster.removeAttribute("checked");
     this._box.removeAttribute("checked");
     this._box.hidden = this._splitter.hidden = true;
 
     let selBrowser = gBrowser.selectedBrowser;
     selBrowser.focus();
     selBrowser.messageManager.sendAsyncMessage("Sidebar:VisibilityChange",
       {commandID, isOpen: false}
     );
     if (triggerNode) {
       updateToggleControlLabel(triggerNode);
     }
   },
+
+  /**
+   * Sets the checked state only on the menu items of the specified sidebar, or
+   * none if the argument is an empty string.
+   */
+  selectMenuItem(commandID) {
+    for (let [id, {menuId, buttonId}] of this.sidebars) {
+      let menu = document.getElementById(menuId);
+      let button = document.getElementById(buttonId);
+      if (id == commandID) {
+        menu.setAttribute("checked", "true");
+        button.setAttribute("checked", "true");
+      } else {
+        menu.removeAttribute("checked");
+        button.removeAttribute("checked");
+      }
+    }
+  },
 };
 
 // Add getters related to the position here, since we will want them
 // available for both startDelayedLoad and init.
 XPCOMUtils.defineLazyPreferenceGetter(SidebarUI, "_positionStart",
   SidebarUI.POSITION_START_PREF, true, SidebarUI.setPosition.bind(SidebarUI));
 XPCOMUtils.defineLazyGetter(SidebarUI, "isRTL", () => {
   return getComputedStyle(document.documentElement).direction == "rtl";
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -316,38 +316,32 @@ xmlns="http://www.w3.org/1999/xhtml"
            class="cui-widget-panel"
            role="group"
            type="arrow"
            hidden="true"
            flip="slide"
            orient="vertical"
            position="bottomcenter topleft">
       <toolbarbutton id="sidebar-switcher-bookmarks"
+                     type="checkbox"
                      label="&bookmarksButton.label;"
                      class="subviewbutton subviewbutton-iconic"
                      key="viewBookmarksSidebarKb"
-                     observes="viewBookmarksSidebar"
-                     oncommand="SidebarUI.show('viewBookmarksSidebar');">
-        <observes element="viewBookmarksSidebar" attribute="checked"/>
-      </toolbarbutton>
+                     oncommand="SidebarUI.show('viewBookmarksSidebar');"/>
       <toolbarbutton id="sidebar-switcher-history"
+                     type="checkbox"
                      label="&historyButton.label;"
                      class="subviewbutton subviewbutton-iconic"
                      key="key_gotoHistory"
-                     observes="viewHistorySidebar"
-                     oncommand="SidebarUI.show('viewHistorySidebar');">
-        <observes element="viewHistorySidebar" attribute="checked"/>
-      </toolbarbutton>
+                     oncommand="SidebarUI.show('viewHistorySidebar');"/>
       <toolbarbutton id="sidebar-switcher-tabs"
+                     type="checkbox"
                      label="&syncedTabs.sidebar.label;"
                      class="subviewbutton subviewbutton-iconic sync-ui-item"
-                     observes="viewTabsSidebar"
-                     oncommand="SidebarUI.show('viewTabsSidebar');">
-        <observes element="viewTabsSidebar" attribute="checked"/>
-      </toolbarbutton>
+                     oncommand="SidebarUI.show('viewTabsSidebar');"/>
       <toolbarseparator/>
       <!-- Extension toolbarbuttons go here. -->
       <toolbarseparator id="sidebar-extensions-separator"/>
       <toolbarbutton id="sidebar-reverse-position"
                      class="subviewbutton"
                      oncommand="SidebarUI.reversePosition()"/>
       <toolbarseparator/>
       <toolbarbutton label="&sidebarMenuClose.label;"
--- a/browser/base/content/test/sidebar/browser.ini
+++ b/browser/base/content/test/sidebar/browser.ini
@@ -1,5 +1,6 @@
 [DEFAULT]
 
 [browser_sidebar_adopt.js]
+[browser_sidebar_keys.js]
 [browser_sidebar_move.js]
 [browser_sidebar_switcher.js]
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/sidebar/browser_sidebar_keys.js
@@ -0,0 +1,24 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+async function testSidebarKeyToggle(key, options, expectedSidebarId) {
+  let promiseShown = BrowserTestUtils.waitForEvent(window, "SidebarShown");
+  EventUtils.synthesizeKey(key, options);
+  await promiseShown;
+  Assert.equal(document.getElementById("sidebar-box")
+                       .getAttribute("sidebarcommand"), expectedSidebarId);
+  EventUtils.synthesizeKey(key, options);
+  Assert.ok(!SidebarUI.isOpen);
+}
+
+add_task(async function test_sidebar_keys() {
+  registerCleanupFunction(() => SidebarUI.hide());
+
+  await testSidebarKeyToggle("b", { accelKey: true }, "viewBookmarksSidebar");
+  if (AppConstants.platform == "win") {
+    await testSidebarKeyToggle("i", { accelKey: true }, "viewBookmarksSidebar");
+  }
+
+  let options = { accelKey: true, shiftKey: AppConstants.platform == "macosx" };
+  await testSidebarKeyToggle("h", options, "viewHistorySidebar");
+});
--- a/browser/components/customizableui/content/panelUI.inc.xul
+++ b/browser/components/customizableui/content/panelUI.inc.xul
@@ -373,19 +373,17 @@
     </panelview>
     <panelview id="PanelUI-history" flex="1">
       <vbox class="panel-subview-body">
         <toolbarbutton id="appMenuViewHistorySidebar"
                        label="&appMenuHistory.viewSidebar.label;"
                        type="checkbox"
                        class="subviewbutton subviewbutton-iconic"
                        key="key_gotoHistory"
-                       oncommand="SidebarUI.toggle('viewHistorySidebar'); PanelUI.hide();">
-          <observes element="viewHistorySidebar" attribute="checked"/>
-        </toolbarbutton>
+                       oncommand="SidebarUI.toggle('viewHistorySidebar');"/>
         <toolbarbutton id="appMenuClearRecentHistory"
                        label="&appMenuHistory.clearRecent.label;"
                        class="subviewbutton subviewbutton-iconic"
                        command="Tools:Sanitize"/>
         <toolbarseparator/>
         <toolbarbutton id="appMenuRecentlyClosedTabs"
                        label="&historyUndoMenu.label;"
                        class="subviewbutton subviewbutton-iconic subviewbutton-nav"
@@ -420,18 +418,18 @@
                descriptionheightworkaround="true">
       <vbox class="panel-subview-body">
         <!-- this widget has 3 boxes in the body, but only 1 is ever visible -->
         <!-- When Sync is ready to sync -->
         <vbox id="PanelUI-remotetabs-main" observes="sync-syncnow-state">
           <vbox id="PanelUI-remotetabs-buttons">
             <toolbarbutton id="PanelUI-remotetabs-view-sidebar"
                            class="subviewbutton subviewbutton-iconic"
-                           observes="viewTabsSidebar"
-                           label="&appMenuRemoteTabs.sidebar.label;"/>
+                           label="&appMenuRemoteTabs.sidebar.label;"
+                           oncommand="SidebarUI.toggle('viewTabsSidebar');"/>
             <toolbarbutton id="PanelUI-remotetabs-view-managedevices"
                            class="subviewbutton subviewbutton-iconic"
                            label="&appMenuRemoteTabs.managedevices.label;"
                            oncommand="gSync.openDevicesManagementPage('syncedtabs-menupanel');"/>
             <toolbarbutton id="PanelUI-remotetabs-syncnow"
                            observes="sync-status"
                            class="subviewbutton subviewbutton-iconic"
                            oncommand="gSync.doSync();"
--- a/browser/components/extensions/parent/ext-sidebarAction.js
+++ b/browser/components/extensions/parent/ext-sidebarAction.js
@@ -111,20 +111,16 @@ this.sidebarAction = class extends Exten
       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();
-      }
       let header = document.getElementById("sidebar-switcher-target");
       header.removeEventListener("SidebarShown", this.updateHeader);
       SidebarUI.sidebars.delete(this.id);
     }
     windowTracker.removeOpenListener(this.windowOpenListener);
     windowTracker.removeCloseListener(this.windowCloseListener);
   }
 
@@ -140,60 +136,52 @@ this.sidebarAction = class extends Exten
           SidebarUI.lastOpenedId == this.id) {
         SidebarUI.show(this.id);
       }
     }
   }
 
   createMenuItem(window, details) {
     let {document, SidebarUI} = window;
+    let keyId = `ext-key-id-${this.id}`;
 
     SidebarUI.sidebars.set(this.id, {
       title: details.title,
       url: sidebarURL,
+      menuId: this.menuId,
+      buttonId: this.buttonId,
+      // The following properties are specific to extensions
+      extensionId: this.extension.id,
+      panel: details.panel,
+      browserStyle: this.browserStyle,
     });
 
-    // Use of the broadcaster allows browser-sidebar.js to properly manage the
-    // checkmarks in the menus.
-    let broadcaster = document.createXULElement("broadcaster");
-    broadcaster.setAttribute("id", this.id);
-    broadcaster.setAttribute("type", "checkbox");
-    broadcaster.setAttribute("group", "sidebar");
-    broadcaster.setAttribute("panel", details.panel);
-    if (this.browserStyle) {
-      broadcaster.setAttribute("browserStyle", "true");
-    }
-    broadcaster.setAttribute("extensionId", this.extension.id);
-    let id = `ext-key-id-${this.id}`;
-    broadcaster.setAttribute("key", id);
-
-    // 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'))");
-
     let header = document.getElementById("sidebar-switcher-target");
     header.addEventListener("SidebarShown", this.updateHeader);
 
     // Insert a menuitem for View->Show Sidebars.
     let menuitem = document.createXULElement("menuitem");
     menuitem.setAttribute("id", this.menuId);
+    menuitem.setAttribute("type", "checkbox");
     menuitem.setAttribute("label", details.title);
-    menuitem.setAttribute("observes", this.id);
+    menuitem.setAttribute("oncommand", `SidebarUI.toggle("${this.id}");`);
     menuitem.setAttribute("class", "menuitem-iconic webextension-menuitem");
+    menuitem.setAttribute("key", keyId);
     this.setMenuIcon(menuitem, details);
 
     // Insert a toolbarbutton for the sidebar dropdown selector.
     let toolbarbutton = document.createXULElement("toolbarbutton");
     toolbarbutton.setAttribute("id", this.buttonId);
+    toolbarbutton.setAttribute("type", "checkbox");
     toolbarbutton.setAttribute("label", details.title);
-    toolbarbutton.setAttribute("observes", this.id);
+    toolbarbutton.setAttribute("oncommand", `SidebarUI.show("${this.id}");`);
     toolbarbutton.setAttribute("class", "subviewbutton subviewbutton-iconic webextension-menuitem");
+    toolbarbutton.setAttribute("key", keyId);
     this.setMenuIcon(toolbarbutton, details);
 
-    document.getElementById("mainBroadcasterSet").appendChild(broadcaster);
     document.getElementById("viewSidebarMenu").appendChild(menuitem);
     let separator = document.getElementById("sidebar-extensions-separator");
     separator.parentNode.insertBefore(toolbarbutton, separator);
     SidebarUI.updateShortcut({button: toolbarbutton});
 
     return menuitem;
   }
 
@@ -203,36 +191,34 @@ this.sidebarAction = class extends Exten
 
     menuitem.setAttribute("style", `
       --webextension-menuitem-image: url("${getIcon(16)}");
       --webextension-menuitem-image-2x: url("${getIcon(32)}");
     `);
   }
 
   /**
-   * Update the broadcaster and menuitem `node` with the tab context data
-   * in `tabData`.
+   * Update the menu items with the tab context data in `tabData`.
    *
    * @param {ChromeWindow} window
    *        Browser chrome window.
    * @param {object} tabData
    *        Tab specific sidebar configuration.
    */
   updateButton(window, tabData) {
     let {document, SidebarUI} = window;
     let title = tabData.title || this.extension.name;
     let menu = document.getElementById(this.menuId);
     if (!menu) {
       menu = this.createMenuItem(window, tabData);
     }
 
-    let broadcaster = document.getElementById(this.id);
-    let urlChanged = tabData.panel !== broadcaster.getAttribute("panel");
+    let urlChanged = tabData.panel !== SidebarUI.sidebars.get(this.id).panel;
     if (urlChanged) {
-      broadcaster.setAttribute("panel", tabData.panel);
+      SidebarUI.sidebars.get(this.id).panel = tabData.panel;
     }
 
     menu.setAttribute("label", title);
     this.setMenuIcon(menu, tabData);
 
     let button = document.getElementById(this.buttonId);
     button.setAttribute("label", title);
     this.setMenuIcon(button, tabData);
@@ -244,28 +230,28 @@ this.sidebarAction = class extends Exten
       this.setMenuIcon(header, tabData);
       if (SidebarUI.isOpen && urlChanged) {
         SidebarUI.show(this.id);
       }
     }
   }
 
   /**
-   * Update the broadcaster and menuitem for a given window.
+   * Update the menu items for a given window.
    *
    * @param {ChromeWindow} window
    *        Browser chrome window.
    */
   updateWindow(window) {
     let nativeTab = window.gBrowser.selectedTab;
     this.updateButton(window, this.tabContext.get(nativeTab));
   }
 
   /**
-   * Update the broadcaster and menuitem when the extension changes the icon,
+   * Update the menu items when the extension changes the icon,
    * title, url, etc. If it only changes a parameter for a single tab, `target`
    * will be that tab. If it only changes a parameter for a single window,
    * `target` will be that window. Otherwise `target` will be null.
    *
    * @param {XULElement|ChromeWindow|null} target
    *        Browser tab or browser chrome window, may be null.
    */
   updateOnChange(target) {
--- a/browser/components/extensions/test/browser/browser_ext_sidebarAction_context.js
+++ b/browser/components/extensions/test/browser/browser_ext_sidebarAction_context.js
@@ -83,19 +83,16 @@ async function runTests(options) {
 
   let sidebarActionId;
   function checkDetails(details, windowId) {
     let {document} = Services.wm.getOuterWindowWithId(windowId);
     if (!sidebarActionId) {
       sidebarActionId = `${makeWidgetId(extension.id)}-sidebar-action`;
     }
 
-    let command = document.getElementById(sidebarActionId);
-    ok(command, "command exists");
-
     let menuId = `menu_${sidebarActionId}`;
     let menu = document.getElementById(menuId);
     ok(menu, "menu exists");
 
     let title = details.title || options.manifest.name;
 
     is(getListStyleImage(menu), details.icon, "icon URL is correct");
     is(menu.getAttribute("label"), title, "image label is correct");