Bug 1337545 - Use keydown event for keycodes in commands draft
authorDoug Thayer <dothayer@mozilla.com>
Mon, 01 May 2017 13:49:18 -0700
changeset 570981 c6f7d0d43d5dad87a2a229da5509d011a6da6069
parent 569501 abe5868346c7abb5b0bdf76f29bc3d9f839461f5
child 626636 5836df5e6bf094178a0f8e64100cf717b922d07f
push id56648
push userbmo:dothayer@mozilla.com
push dateMon, 01 May 2017 20:52:19 +0000
bugs1337545
milestone55.0a1
Bug 1337545 - Use keydown event for keycodes in commands When you create a <key> element, which is what the commands API does under the hood to allow keyboard shortcuts, the event type that the handler listens to defaults to keypress. For printable characters, the keyCode property of a keypress event is 0, causing keycode bindings for these printable characters to not work. We could use key for these properties, but the shift key interacts with the key property in ways that make it simpler to simply fix the keycode usage for our purposes. MozReview-Commit-ID: HdWdN7cMfuq
browser/components/extensions/ext-commands.js
browser/components/extensions/test/browser/browser_ext_commands_onCommand.js
--- a/browser/components/extensions/ext-commands.js
+++ b/browser/components/extensions/ext-commands.js
@@ -162,21 +162,22 @@ this.commands = class extends ExtensionA
     let parts = shortcut.split("+");
 
     // The key is always the last element.
     let chromeKey = parts.pop();
 
     // The modifiers are the remaining elements.
     keyElement.setAttribute("modifiers", this.getModifiersAttribute(parts));
 
-    if (/^[A-Z0-9]$/.test(chromeKey)) {
+    if (/^[A-Z]$/.test(chromeKey)) {
       // We use the key attribute for all single digits and characters.
       keyElement.setAttribute("key", chromeKey);
     } else {
       keyElement.setAttribute("keycode", this.getKeycodeAttribute(chromeKey));
+      keyElement.setAttribute("event", "keydown");
     }
 
     return keyElement;
   }
 
   /**
    * Determines the corresponding XUL keycode from the given chrome key.
    *
@@ -186,16 +187,19 @@ this.commands = class extends ExtensionA
    *    ---------------------------------------
    *    "PageUP"  |  "VK_PAGE_UP"
    *    "Delete"  |  "VK_DELETE"
    *
    * @param {string} chromeKey The chrome key (e.g. "PageUp", "Space", ...)
    * @returns {string} The constructed value for the Key's 'keycode' attribute.
    */
   getKeycodeAttribute(chromeKey) {
+    if (/[0-9]/.test(chromeKey)) {
+      return `VK_${chromeKey}`;
+    }
     return `VK${chromeKey.replace(/([A-Z])/g, "_$&").toUpperCase()}`;
   }
 
   /**
    * Determines the corresponding XUL modifiers from the chrome modifiers.
    *
    * For example:
    *
--- a/browser/components/extensions/test/browser/browser_ext_commands_onCommand.js
+++ b/browser/components/extensions/test/browser/browser_ext_commands_onCommand.js
@@ -65,16 +65,25 @@ add_task(function* test_user_defined_com
       name: "toggle-ctrl-shift-left",
       shortcut: "Ctrl+Shift+Left",
       key: "VK_LEFT",
       modifiers: {
         accelKey: true,
         shiftKey: true,
       },
     },
+    {
+      name: "toggle-ctrl-shift-1",
+      shortcut: "Ctrl+Shift+1",
+      key: "1",
+      modifiers: {
+        accelKey: true,
+        shiftKey: true,
+      },
+    },
     // Alt+Shift Shortcuts
     {
       name: "toggle-alt-shift-1",
       shortcut: "Alt+Shift+1",
       key: "1",
       modifiers: {
         altKey: true,
         shiftKey: true,
@@ -132,16 +141,41 @@ add_task(function* test_user_defined_com
       name: "spaces-in-shortcut-name",
       shortcut: "  Alt + Shift + 2  ",
       key: "2",
       modifiers: {
         altKey: true,
         shiftKey: true,
       },
     },
+    {
+      name: "toggle-ctrl-space",
+      shortcut: "Ctrl+Space",
+      key: "VK_SPACE",
+      modifiers: {
+        accelKey: true,
+      },
+    },
+    {
+      name: "toggle-ctrl-comma",
+      shortcut: "Ctrl+Comma",
+      key: "VK_COMMA",
+      modifiers: {
+        accelKey: true,
+      },
+    },
+    {
+      name: "toggle-ctrl-period",
+      shortcut: "Ctrl+Period",
+      key: "VK_PERIOD",
+      modifiers: {
+        accelKey: true,
+      },
+    },
+
   ];
 
   // Create a window before the extension is loaded.
   let win1 = yield BrowserTestUtils.openNewBrowserWindow();
   yield BrowserTestUtils.loadURI(win1.gBrowser.selectedBrowser, "about:robots");
   yield BrowserTestUtils.browserLoaded(win1.gBrowser.selectedBrowser);
 
   let commands = {};