Bug 1520119 - Allow shortcut removal via button r=mstriemer

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

--HG--
extra : moz-landing-system : lando

toolkit/mozapps/extensions/content/aboutaddons.html

@@ -353,6 +353,7 @@
       <div class="shortcut-row">
         <label class="shortcut-label"></label>
         <input class="shortcut-input" data-l10n-id="shortcuts-input" type="text" readonly>
+        <button class="shortcut-remove-button ghost-button"></button>
       </div>
     </template>
 

toolkit/mozapps/extensions/content/shortcuts.css

@@ -33,7 +33,6 @@
 
 .shortcut-row {
   display: flex;
-  justify-content: space-between;
   align-items: center;
   margin-top: 10px;
 }
@@ -42,6 +41,23 @@
   display: none;
 }
 
+.shortcut-label {
+  flex-grow: 1;
+}
+
+.shortcut-remove-button {
+  background-image: url("chrome://global/skin/icons/delete.svg");
+  background-position: center;
+  background-repeat: no-repeat;
+  -moz-context-properties: fill;
+  fill: currentColor;
+  min-width: 32px;
+}
+
+.shortcut-input[shortcut=""] + .shortcut-remove-button {
+  visibility: hidden;
+}
+
 .expand-row {
   display: flex;
   justify-content: center;

toolkit/mozapps/extensions/content/shortcuts.js

@@ -461,6 +461,15 @@ XPCOMUtils.defineLazyModuleGetters(this, {
     }
   }
 
+  function onShortcutRemove(e) {
+    let removeButton = e.target;
+    let input = removeButton.parentNode.querySelector(".shortcut-input");
+    if (input.getAttribute("shortcut")) {
+      input.value = "";
+      assignShortcutToInput(input, "");
+    }
+  }
+
   function assignShortcutToInput(input, shortcutString) {
     let addonId = input.closest(".card").getAttribute("addon-id");
     let extension = extensionForAddonId(addonId);
@@ -582,6 +591,9 @@ XPCOMUtils.defineLazyModuleGetters(this, {
           input.addEventListener("blur", inputBlurred);
           input.addEventListener("focus", onFocus);
 
+          let removeButton = row.querySelector(".shortcut-remove-button");
+          removeButton.addEventListener("click", onShortcutRemove);
+
           if (willHideCommands && i == limit) {
             firstHiddenInput = input;
           }

toolkit/mozapps/extensions/test/browser/browser_manage_shortcuts_remove.js

@@ -29,6 +29,23 @@ async function waitForShortcutSet(input, expected) {
     `Shortcut should be set to ${JSON.stringify(expected)}`
   );
   ok(doc.activeElement != input, "The input is no longer focused");
+  checkHasRemoveButton(input, expected !== "");
+}
+
+function removeButtonForInput(input) {
+  let removeButton = input.parentNode.querySelector(".shortcut-remove-button");
+  ok(removeButton, "has remove button");
+  return removeButton;
+}
+
+function checkHasRemoveButton(input, expected) {
+  let removeButton = removeButtonForInput(input);
+  let visibility = input.ownerGlobal.getComputedStyle(removeButton).visibility;
+  if (expected) {
+    is(visibility, "visible", "Remove button should be visible");
+  } else {
+    is(visibility, "hidden", "Remove button should be hidden");
+  }
 }
 
 add_task(async function test_remove_shortcut() {
@@ -58,6 +75,8 @@ add_task(async function test_remove_shortcut() {
 
   let input = getShortcutByName(doc, extension, "commandOne");
 
+  checkHasRemoveButton(input, true);
+
   // First: Verify that Shift-Del is not valid, but doesn't do anything.
   input.focus();
   EventUtils.synthesizeKey("KEY_Delete", { shiftKey: true });
@@ -71,6 +90,7 @@ add_task(async function test_remove_shortcut() {
   } else {
     is(errorId, "shortcuts-modifier-other", "Shift-Del isn't a valid shortcut");
   }
+  checkHasRemoveButton(input, true);
 
   // Now, verify that the original shortcut still works.
   EventUtils.synthesizeKey("KEY_Escape");
@@ -105,6 +125,7 @@ add_task(async function test_remove_shortcut() {
   // Set a shortcut where the default was not set.
   let inputEmpty = getShortcutByName(doc, extension, "commandEmpty");
   is(inputEmpty.getAttribute("shortcut"), "", "Empty shortcut by default");
+  checkHasRemoveButton(input, false);
   inputEmpty.focus();
   EventUtils.synthesizeKey("3", { altKey: true, shiftKey: true });
   await waitForShortcutSet(inputEmpty, "Alt+Shift+3");
@@ -137,6 +158,15 @@ add_task(async function test_remove_shortcut() {
     "commandEmpty should be disabled again by Backspace"
   );
 
+  // Check that the remove button works as expected.
+  let inputTwo = getShortcutByName(doc, extension, "commandTwo");
+  is(inputTwo.getAttribute("shortcut"), "Shift+Alt+2", "initial shortcut");
+  checkHasRemoveButton(inputTwo, true);
+  removeButtonForInput(inputTwo).click();
+  is(inputTwo.getAttribute("shortcut"), "", "cleared shortcut");
+  checkHasRemoveButton(inputTwo, false);
+  ok(doc.activeElement != inputTwo, "input of removed shortcut is not focused");
+
   await closeShortcutsView(doc);
 
   await extension.unload();