From cf52c09668057a392ae17ff6834356134bec7436 Mon Sep 17 00:00:00 2001
From: Mark Striemer <mstriemer@mozilla.com>
Date: Wed, 30 Jan 2019 19:08:04 +0000
Subject: [PATCH] Bug 1521826 - Part 1: Sort commands by a shortcut being set
 r=Gijs

Commands that have a shortcut will be listed first in the extension shortcut UI.

Differential Revision: https://phabricator.services.mozilla.com/D17877

--HG--
extra : moz-landing-system : lando
---
 toolkit/mozapps/extensions/content/shortcuts.js           | 8 ++++++++
 .../extensions/test/browser/browser_manage_shortcuts.js   | 7 +++++++
 2 files changed, 15 insertions(+)

diff --git a/toolkit/mozapps/extensions/content/shortcuts.js b/toolkit/mozapps/extensions/content/shortcuts.js
index c2248a401bf9..1db847fb2f7d 100644
--- a/toolkit/mozapps/extensions/content/shortcuts.js
+++ b/toolkit/mozapps/extensions/content/shortcuts.js
@@ -273,6 +273,14 @@ async function renderAddons(addons) {
 
     if (extension.shortcuts) {
       let commands = await extension.shortcuts.allCommands();
+      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)
+          return -1;
+        return 1;
+      });
 
       for (let command of commands) {
         let row = document.importNode(templates.row.content, true);
diff --git a/toolkit/mozapps/extensions/test/browser/browser_manage_shortcuts.js b/toolkit/mozapps/extensions/test/browser/browser_manage_shortcuts.js
index 860d0f3606ee..e992520fbfcf 100644
--- a/toolkit/mozapps/extensions/test/browser/browser_manage_shortcuts.js
+++ b/toolkit/mozapps/extensions/test/browser/browser_manage_shortcuts.js
@@ -8,6 +8,7 @@ PromiseTestUtils.whitelistRejectionsGlobally(/Message manager disconnected/);
 
 add_task(async function testUpdatingCommands() {
   let commands = {
+    commandZero: {},
     commandOne: {
       suggested_key: {default: "Shift+Alt+4"},
     },
@@ -64,6 +65,12 @@ add_task(async function testUpdatingCommands() {
   let inputs = card.querySelectorAll(".shortcut-input");
   is(inputs.length, Object.keys(commands).length, "There is an input for each command");
 
+  let nameOrder = Array.from(inputs).map(input => input.getAttribute("name"));
+  Assert.deepEqual(
+    nameOrder,
+    ["commandOne", "commandTwo", "_execute_browser_action", "commandZero"],
+    "commandZero should be last since it is unset");
+
   for (let input of inputs) {
     // Change the shortcut.
     input.focus();