Bug 1421811 - Part 3: Update shortcut in sidebar on update r=Gijs,mixedpuppy

MozReview-Commit-ID: 4y02mCqwacg

--HG--
extra : rebase_source : c0a0334d46037793faf3c6797903df70f8de0e51

browser/base/content/browser-sidebar.js

@@ -119,6 +119,25 @@ var SidebarUI = {
     this._switcherTarget.classList.add("active");
   },
 
+  updateShortcut({button, key}) {
+    // If the shortcuts haven't been rendered yet then it will be set correctly
+    // on the first render so there's nothing to do now.
+    if (!this._addedShortcuts) {
+      return;
+    }
+    if (key) {
+      let keyId = key.getAttribute("id");
+      button = this._switcherPanel.querySelector(`[key="${keyId}"]`);
+    } else if (button) {
+      let keyId = button.getAttribute("key");
+      key = document.getElementById(keyId);
+    }
+    if (!button || !key) {
+      return;
+    }
+    button.setAttribute("shortcut", ShortcutUtils.prettifyShortcut(key));
+  },
+
   _addedShortcuts: false,
   _ensureShortcutsShown() {
     if (this._addedShortcuts) {
@@ -126,12 +145,7 @@ var SidebarUI = {
     }
     this._addedShortcuts = true;
     for (let button of this._switcherPanel.querySelectorAll("toolbarbutton[key]")) {
-      let keyId = button.getAttribute("key");
-      let key = document.getElementById(keyId);
-      if (!key) {
-        continue;
-      }
-      button.setAttribute("shortcut", ShortcutUtils.prettifyShortcut(key));
+      this.updateShortcut({button});
     }
   },
 

browser/components/extensions/ext-commands.js

@@ -13,6 +13,10 @@ ChromeUtils.defineModuleGetter(this, "ExtensionSettingsStore",
 
 var XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
+const EXECUTE_PAGE_ACTION = "_execute_page_action";
+const EXECUTE_BROWSER_ACTION = "_execute_browser_action";
+const EXECUTE_SIDEBAR_ACTION = "_execute_sidebar_action";
+
 function normalizeShortcut(shortcut) {
   return shortcut ? shortcut.replace(/\s+/g, "") : null;
 }
@@ -154,13 +158,20 @@ this.commands = class extends ExtensionAPI {
     if (this.keysetsMap.has(window)) {
       this.keysetsMap.get(window).remove();
     }
+    let sidebarKey;
     commands.forEach((command, name) => {
       if (command.shortcut) {
         let keyElement = this.buildKey(doc, name, command.shortcut);
         keyset.appendChild(keyElement);
+        if (name == EXECUTE_SIDEBAR_ACTION) {
+          sidebarKey = keyElement;
+        }
       }
     });
     doc.documentElement.appendChild(keyset);
+    if (sidebarKey) {
+      window.SidebarUI.updateShortcut({key: sidebarKey});
+    }
     this.keysetsMap.set(window, keyset);
   }
 
@@ -187,11 +198,11 @@ this.commands = class extends ExtensionAPI {
     // therefore the listeners for these elements will be garbage collected.
     keyElement.addEventListener("command", (event) => {
       let action;
-      if (name == "_execute_page_action") {
+      if (name == EXECUTE_PAGE_ACTION) {
         action = pageActionFor(this.extension);
-      } else if (name == "_execute_browser_action") {
+      } else if (name == EXECUTE_BROWSER_ACTION) {
         action = browserActionFor(this.extension);
-      } else if (name == "_execute_sidebar_action") {
+      } else if (name == EXECUTE_SIDEBAR_ACTION) {
         action = sidebarActionFor(this.extension);
       } else {
         this.extension.tabManager
@@ -229,7 +240,7 @@ this.commands = class extends ExtensionAPI {
 
     // The modifiers are the remaining elements.
     keyElement.setAttribute("modifiers", this.getModifiersAttribute(parts));
-    if (name == "_execute_sidebar_action") {
+    if (name == EXECUTE_SIDEBAR_ACTION) {
       let id = `ext-key-id-${this.id}-sidebar-action`;
       keyElement.setAttribute("id", id);
     }

browser/components/extensions/ext-sidebarAction.js

@@ -151,7 +151,7 @@ this.sidebarAction = class extends ExtensionAPI {
   }
 
   createMenuItem(window, details) {
-    let {document} = window;
+    let {document, SidebarUI} = window;
 
     // Use of the broadcaster allows browser-sidebar.js to properly manage the
     // checkmarks in the menus.
@@ -190,6 +190,7 @@ this.sidebarAction = class extends ExtensionAPI {
     document.getElementById("viewSidebarMenu").appendChild(menuitem);
     let separator = document.getElementById("sidebar-extensions-separator");
     separator.parentNode.insertBefore(toolbarbutton, separator);
+    SidebarUI.updateShortcut({button: toolbarbutton});
 
     return menuitem;
   }

browser/components/extensions/test/browser/browser_ext_commands_update.js

@@ -236,3 +236,72 @@ add_task(async function test_update_defined_command() {
   // Shortcut is unchanged since it was previously updated.
   checkKey(extension.id, "P", "alt shift");
 });
+
+add_task(async function updateSidebarCommand() {
+  let extension = ExtensionTestUtils.loadExtension({
+    useAddonManager: "temporary",
+    manifest: {
+      commands: {
+        _execute_sidebar_action: {
+          suggested_key: {
+            default: "Ctrl+Shift+E",
+          },
+        },
+      },
+      sidebar_action: {
+        default_panel: "sidebar.html",
+      },
+    },
+    background() {
+      browser.test.onMessage.addListener(async (msg, data) => {
+        if (msg == "updateShortcut") {
+          await browser.commands.update(data);
+          return browser.test.sendMessage("done");
+        }
+        throw new Error("Unknown message");
+      });
+    },
+    files: {
+      "sidebar.html": `
+        <!DOCTYPE html>
+        <html>
+        <head><meta charset="utf-8"/>
+        <script src="sidebar.js"></script>
+        </head>
+        <body>
+        A Test Sidebar
+        </body></html>
+      `,
+
+      "sidebar.js": function() {
+        window.onload = () => {
+          browser.test.sendMessage("sidebar");
+        };
+      },
+    },
+  });
+  await extension.startup();
+  await extension.awaitMessage("sidebar");
+
+  // Show and hide the switcher panel to generate the initial shortcuts.
+  let switcherShown = promisePopupShown(SidebarUI._switcherPanel);
+  SidebarUI.showSwitcherPanel();
+  await switcherShown;
+  let switcherHidden = promisePopupHidden(SidebarUI._switcherPanel);
+  SidebarUI.hideSwitcherPanel();
+  await switcherHidden;
+
+  let buttonId = `button_${makeWidgetId(extension.id)}-sidebar-action`;
+  let button = document.getElementById(buttonId);
+  let shortcut = button.getAttribute("shortcut");
+  ok(shortcut.endsWith("E"), "The button has the shortcut set");
+
+  extension.sendMessage(
+    "updateShortcut", {name: "_execute_sidebar_action", shortcut: "Ctrl+Shift+M"});
+  await extension.awaitMessage("done");
+
+  shortcut = button.getAttribute("shortcut");
+  ok(shortcut.endsWith("M"), "The button shortcut has been updated");
+
+  await extension.unload();
+});

browser/components/extensions/test/browser/browser_ext_sidebarAction.js

@@ -6,13 +6,6 @@ requestLongerTimeout(2);
 
 let extData = {
   manifest: {
-    commands: {
-      _execute_sidebar_action: {
-        suggested_key: {
-          default: "Ctrl+Shift+I",
-        },
-      },
-    },
     sidebar_action: {
       default_panel: "sidebar.html",
     },
@@ -57,6 +50,17 @@ let extData = {
   },
 };
 
+function getExtData(manifestUpdates = {}) {
+  return {
+    ...extData,
+    manifest: {
+      ...extData.manifest,
+      ...manifestUpdates,
+    },
+  };
+}
+
+
 async function sendMessage(ext, msg, data = undefined) {
   ext.sendMessage({msg, data});
   await ext.awaitMessage("done");
@@ -64,7 +68,7 @@ async function sendMessage(ext, msg, data = undefined) {
 
 add_task(async function sidebar_initial_install() {
   ok(document.getElementById("sidebar-box").hidden, "sidebar box is not visible");
-  let extension = ExtensionTestUtils.loadExtension(extData);
+  let extension = ExtensionTestUtils.loadExtension(getExtData());
   await extension.startup();
   // Test sidebar is opened on install
   await extension.awaitMessage("sidebar");
@@ -77,14 +81,14 @@ add_task(async function sidebar_initial_install() {
 
 
 add_task(async function sidebar_two_sidebar_addons() {
-  let extension2 = ExtensionTestUtils.loadExtension(extData);
+  let extension2 = ExtensionTestUtils.loadExtension(getExtData());
   await extension2.startup();
   // Test sidebar is opened on install
   await extension2.awaitMessage("sidebar");
   ok(!document.getElementById("sidebar-box").hidden, "sidebar box is visible");
 
   // Test second sidebar install opens new sidebar
-  let extension3 = ExtensionTestUtils.loadExtension(extData);
+  let extension3 = ExtensionTestUtils.loadExtension(getExtData());
   await extension3.startup();
   // Test sidebar is opened on install
   await extension3.awaitMessage("sidebar");
@@ -98,7 +102,7 @@ add_task(async function sidebar_two_sidebar_addons() {
 });
 
 add_task(async function sidebar_empty_panel() {
-  let extension = ExtensionTestUtils.loadExtension(extData);
+  let extension = ExtensionTestUtils.loadExtension(getExtData());
   await extension.startup();
   // Test sidebar is opened on install
   await extension.awaitMessage("sidebar");
@@ -109,7 +113,7 @@ add_task(async function sidebar_empty_panel() {
 
 add_task(async function sidebar_isOpen() {
   info("Load extension1");
-  let extension1 = ExtensionTestUtils.loadExtension(extData);
+  let extension1 = ExtensionTestUtils.loadExtension(getExtData());
   await extension1.startup();
 
   info("Test extension1's sidebar is opened on install");
@@ -117,14 +121,8 @@ add_task(async function sidebar_isOpen() {
   await sendMessage(extension1, "isOpen", {result: true});
   let sidebar1ID = SidebarUI.currentID;
 
-  // Test that the key is set for the extension.
-  let button = document.getElementById(`button_${makeWidgetId(extension1.id)}-sidebar-action`);
-  ok(button.hasAttribute("key"), "The menu item has a key specified");
-  let key = document.getElementById(button.getAttribute("key"));
-  ok(key, "The key attribute finds the related key element");
-
   info("Load extension2");
-  let extension2 = ExtensionTestUtils.loadExtension(extData);
+  let extension2 = ExtensionTestUtils.loadExtension(getExtData());
   await extension2.startup();
 
   info("Test extension2's sidebar is opened on install");
@@ -168,3 +166,61 @@ add_task(async function sidebar_isOpen() {
   await extension1.unload();
   await extension2.unload();
 });
+
+add_task(async function testShortcuts() {
+  function verifyShortcut(id, commandKey) {
+    // We're just testing the command key since the modifiers have different
+    // icons on different platforms.
+    let button = document.getElementById(`button_${makeWidgetId(id)}-sidebar-action`);
+    ok(button.hasAttribute("key"), "The menu item has a key specified");
+    let key = document.getElementById(button.getAttribute("key"));
+    ok(key, "The key attribute finds the related key element");
+    ok(button.getAttribute("shortcut").endsWith(commandKey),
+       "The shortcut has the right key");
+  }
+
+  let extension1 = ExtensionTestUtils.loadExtension(
+    getExtData({
+      commands: {
+        _execute_sidebar_action: {
+          suggested_key: {
+            default: "Ctrl+Shift+I",
+          },
+        },
+      },
+    }));
+  let extension2 = ExtensionTestUtils.loadExtension(
+    getExtData({
+      commands: {
+        _execute_sidebar_action: {
+          suggested_key: {
+            default: "Ctrl+Shift+E",
+          },
+        },
+      },
+    }));
+
+  await extension1.startup();
+  await extension1.awaitMessage("sidebar");
+
+  // Open and close the switcher panel to trigger shortcut content rendering.
+  let switcherPanelShown = promisePopupShown(SidebarUI._switcherPanel);
+  SidebarUI.showSwitcherPanel();
+  await switcherPanelShown;
+  let switcherPanelHidden = promisePopupHidden(SidebarUI._switcherPanel);
+  SidebarUI.hideSwitcherPanel();
+  await switcherPanelHidden;
+
+  // Test that the key is set for the extension after the shortcuts are rendered.
+  verifyShortcut(extension1.id, "I");
+
+  await extension2.startup();
+  await extension2.awaitMessage("sidebar");
+
+  // Once the switcher panel has been opened new shortcuts should be added
+  // automatically.
+  verifyShortcut(extension2.id, "E");
+
+  await extension1.unload();
+  await extension2.unload();
+});