Bug 1522757 - Remove addons with no shortcut from extension shortcut page r=mstriemer,Gijs,flod
authortrishul <trishul.goel@gmail.com>
Thu, 07 Feb 2019 23:04:39 +0000
changeset 457650 8607666f33d88c2ab99cb20319224c4a54b85f34
parent 457649 81aac3b38db3c93bf3fef050ecdf389d9747bab6
child 457651 550f4337bc9bf9c61f203cc728d9e8b6f3db4723
push id35516
push userrmaries@mozilla.com
push dateFri, 08 Feb 2019 04:23:26 +0000
treeherdermozilla-central@d599d1a73a3a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmstriemer, Gijs, flod
bugs1522757
milestone67.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 1522757 - Remove addons with no shortcut from extension shortcut page r=mstriemer,Gijs,flod Remove addons with no shortcut from extension shortcut page Differential Revision: https://phabricator.services.mozilla.com/D17861
toolkit/locales/en-US/toolkit/about/aboutAddons.ftl
toolkit/mozapps/extensions/content/shortcuts.html
toolkit/mozapps/extensions/content/shortcuts.js
toolkit/mozapps/extensions/test/browser/browser_manage_shortcuts.js
--- a/toolkit/locales/en-US/toolkit/about/aboutAddons.ftl
+++ b/toolkit/locales/en-US/toolkit/about/aboutAddons.ftl
@@ -295,18 +295,18 @@ extensions-updates-update-selected =
     .label = Install Updates
     .tooltiptext = Install available updates in this list
 
 ## Extension shortcut management
 
 manage-extensions-shortcuts =
     .label = Manage Extension Shortcuts
     .accesskey = S
-shortcuts-empty-message = There are no shortcuts for this extension.
 shortcuts-no-addons = You don’t have any extensions enabled.
+shortcuts-no-commands = The following extensions do not have shortcuts:
 shortcuts-input =
   .placeholder = Type a shortcut
 
 shortcuts-browserAction = Activate extension
 shortcuts-pageAction = Activate page action
 shortcuts-sidebarAction = Toggle the sidebar
 
 shortcuts-modifier-mac = Include Ctrl, Alt, or ⌘
--- a/toolkit/mozapps/extensions/content/shortcuts.html
+++ b/toolkit/mozapps/extensions/content/shortcuts.html
@@ -39,19 +39,20 @@
       </template>
 
       <template id="expand-row-template">
         <div class="expand-row">
           <button class="expand-button"></button>
         </div>
       </template>
 
-      <template id="shortcuts-empty-template">
-        <div class="shortcuts-empty-label" data-l10n-id="shortcuts-empty-message"></div>
+      <template id="shortcuts-no-addons">
+        <div data-l10n-id="shortcuts-no-addons"></div>
       </template>
 
-      <template id="shortcuts-no-addons">
-        <div data-l10n-id="shortcuts-no-addons"></div>
+      <template id="shortcuts-no-commands-template">
+        <div data-l10n-id="shortcuts-no-commands"></div>
+        <ul class="shortcuts-no-commands-list"></ul>
       </template>
 
     </div>
   </body>
 </html>
--- a/toolkit/mozapps/extensions/content/shortcuts.js
+++ b/toolkit/mozapps/extensions/content/shortcuts.js
@@ -23,19 +23,19 @@ let templatesLoaded = false;
 const templates = {};
 
 function loadTemplates() {
   if (templatesLoaded) return;
   templatesLoaded = true;
 
   templates.card = document.getElementById("card-template");
   templates.row = document.getElementById("shortcut-row-template");
-  templates.empty = document.getElementById("shortcuts-empty-template");
   templates.noAddons = document.getElementById("shortcuts-no-addons");
   templates.expandRow = document.getElementById("expand-row-template");
+  templates.noShortcutAddons = document.getElementById("shortcuts-no-commands-template");
 }
 
 function extensionForAddonId(id) {
   let policy = WebExtensionPolicy.getByID(id);
   return policy && policy.extension;
 }
 
 let builtInNames = new Map([
@@ -256,32 +256,46 @@ function onShortcutChange(e) {
       setError(input, "shortcuts-invalid");
       break;
     case ShortcutUtils.INVALID_KEY:
       setError(input, "shortcuts-letter");
       break;
   }
 }
 
+function renderNoShortcutAddons(addons) {
+  let fragment = document.importNode(templates.noShortcutAddons.content, true);
+  let list = fragment.querySelector(".shortcuts-no-commands-list");
+  for (let addon of addons) {
+    let addonItem = document.createElement("li");
+    addonItem.textContent = addon.name;
+    addonItem.setAttribute("addon-id", addon.id);
+    list.appendChild(addonItem);
+  }
+
+  return fragment;
+}
+
 async function renderAddons(addons) {
   let frag = document.createDocumentFragment();
+  let noShortcutAddons = [];
   for (let addon of addons) {
     let extension = extensionForAddonId(addon.id);
 
     // Skip this extension if it isn't a webextension.
     if (!extension) continue;
 
-    let card = document.importNode(
-      templates.card.content, true).firstElementChild;
-    let icon = AddonManager.getPreferredIconURL(addon, 24, window);
-    card.setAttribute("addon-id", addon.id);
-    card.querySelector(".addon-icon").src = icon || FALLBACK_ICON;
-    card.querySelector(".addon-name").textContent = addon.name;
+    if (extension.shortcuts) {
+      let card = document.importNode(
+        templates.card.content, true).firstElementChild;
+      let icon = AddonManager.getPreferredIconURL(addon, 24, window);
+      card.setAttribute("addon-id", addon.id);
+      card.querySelector(".addon-icon").src = icon || FALLBACK_ICON;
+      card.querySelector(".addon-name").textContent = addon.name;
 
-    if (extension.shortcuts) {
       let commands = await extension.shortcuts.allCommands();
 
       // Sort the commands so the ones with shortcuts are at the top.
       commands.sort((a, b) => {
         // Boolean compare the shortcuts to see if they're both set or unset.
         if (!a.shortcut == !b.shortcut)
           return 0;
         if (a.shortcut)
@@ -348,22 +362,27 @@ async function renderAddons(addons) {
             // If this as a keyboard event then focus the next input.
             if (event.mozInputSource == MouseEvent.MOZ_SOURCE_KEYBOARD) {
               firstHiddenInput.focus();
             }
           }
         });
         card.appendChild(row);
       }
+
+      frag.appendChild(card);
     } else {
-      card.appendChild(document.importNode(templates.empty.content, true));
+      noShortcutAddons.push({ id: addon.id, name: addon.name });
     }
+  }
 
-    frag.appendChild(card);
+  if (noShortcutAddons.length > 0) {
+    frag.appendChild(renderNoShortcutAddons(noShortcutAddons));
   }
+
   return frag;
 }
 
 async function render() {
   loadTemplates();
   let allAddons = await AddonManager.getAddonsByTypes(["extension"]);
   let addons = allAddons
     .filter(addon => !addon.isSystem && addon.isActive)
--- a/toolkit/mozapps/extensions/test/browser/browser_manage_shortcuts.js
+++ b/toolkit/mozapps/extensions/test/browser/browser_manage_shortcuts.js
@@ -52,16 +52,34 @@ add_task(async function testUpdatingComm
       browser.test.sendMessage("ready");
     },
     useAddonManager: "temporary",
   });
 
   await extension.startup();
   await extension.awaitMessage("ready");
 
+  let extension2 = ExtensionTestUtils.loadExtension({
+    manifest: {
+      browser_specific_settings: {
+        gecko: {
+          id: "addons@noShorcut",
+        },
+      },
+      name: "no shortcut addon",
+    },
+    background() {
+      browser.test.sendMessage("ready");
+    },
+    useAddonManager: "temporary",
+  });
+
+  await extension2.startup();
+  await extension2.awaitMessage("ready");
+
   async function checkShortcut(name, key, modifiers) {
     EventUtils.synthesizeKey(key, modifiers);
     let message = await extension.awaitMessage("oncommand");
     is(message, name, `Expected onCommand listener to fire with the correct name: ${name}`);
   }
 
   // Check that the original shortcuts work.
   await checkShortcut("commandOne", "4", {shiftKey: true, altKey: true});
@@ -130,18 +148,32 @@ add_task(async function testUpdatingComm
     } else {
       is(label.textContent, value, "The textContent is set");
     }
   }
   checkLabel("commandOne", "commandOne");
   checkLabel("commandTwo", "Command Two!");
   checkLabel("_execute_browser_action", "shortcuts-browserAction");
 
+  // Check there is only 1 shortcut card.
+  let shortcutAddonsList = doc.querySelectorAll(".shortcut");
+  is(shortcutAddonsList.length, 1, "There is only 1 addon card with shortcut");
+
+  // Check there is unordered list of shortcut-less addons.
+  let noShortcutAddonsList = doc.querySelector(".shortcuts-no-commands-list");
+  ok(noShortcutAddonsList, "There is an unordered list of addons without shortcuts");
+
+  // Check there is shortcut-less addon in the list.
+  let addon = noShortcutAddonsList.querySelector(`[addon-id="addons@noShorcut"]`);
+  ok(addon, "There is addon without shortcut in unordered list");
+  is(addon.textContent, "no shortcut addon", "The add-on's name is set in the list");
+
   await closeView();
   await extension.unload();
+  await extension2.unload();
 });
 
 async function startExtensionWithCommands(numCommands) {
   let commands = {};
 
   for (let i = 0; i < numCommands; i++) {
     commands[`command-${i}`] = {};
   }