Bug 1300458 - devtools key shortcuts fix shift+cmd shortcuts on OSX;r=ochameau draft
authorJulian Descottes <jdescottes@mozilla.com>
Tue, 06 Sep 2016 11:43:11 +0200
changeset 410236 07c240f080dce7e4fe6d20275d26ffefb81d705d
parent 409960 22565d35ed35cc2b0a3e11472fc38dd02f2db19a
child 410237 32ba5059c1f04e4d5df167b3a72104dfd47b5c4f
push id28687
push userjdescottes@mozilla.com
push dateTue, 06 Sep 2016 12:34:24 +0000
reviewersochameau
bugs1300458
milestone51.0a1
Bug 1300458 - devtools key shortcuts fix shift+cmd shortcuts on OSX;r=ochameau MozReview-Commit-ID: Aq6cFkF86Qa
devtools/client/shared/key-shortcuts.js
devtools/client/shared/test/browser_key_shortcuts.js
--- a/devtools/client/shared/key-shortcuts.js
+++ b/devtools/client/shared/key-shortcuts.js
@@ -187,22 +187,27 @@ KeyShortcuts.prototype = {
       return false;
     }
     if (shortcut.ctrl != event.ctrlKey) {
       return false;
     }
     if (shortcut.alt != event.altKey) {
       return false;
     }
-    // Shift is a special modifier, it may implicitely be required if the
-    // expected key is a special character accessible via shift.
-    if (shortcut.shift != event.shiftKey && event.key &&
-        event.key.match(/[a-zA-Z]/)) {
-      return false;
+    if (shortcut.shift != event.shiftKey) {
+      // Shift is a special modifier, it may implicitely be required if the expected key
+      // is a special character accessible via shift.
+      let isAlphabetical = event.key && event.key.match(/[a-zA-Z]/);
+      // OSX: distinguish cmd+[key] from cmd+shift+[key] shortcuts (Bug 1300458)
+      let cmdShortcut = shortcut.meta && !shortcut.alt && !shortcut.ctrl;
+      if (isAlphabetical || cmdShortcut) {
+        return false;
+      }
     }
+
     if (shortcut.keyCode) {
       return event.keyCode == shortcut.keyCode;
     } else if (event.key in ElectronKeysMapping) {
       return ElectronKeysMapping[event.key] === shortcut.key;
     }
 
     // get the key from the keyCode if key is not provided.
     let key = event.key || String.fromCharCode(event.keyCode);
--- a/devtools/client/shared/test/browser_key_shortcuts.js
+++ b/devtools/client/shared/test/browser_key_shortcuts.js
@@ -2,29 +2,31 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 var isOSX = Services.appinfo.OS === "Darwin";
 
 add_task(function* () {
   let shortcuts = new KeyShortcuts({
     window
   });
+
   yield testSimple(shortcuts);
   yield testNonLetterCharacter(shortcuts);
   yield testPlusCharacter(shortcuts);
   yield testFunctionKey(shortcuts);
   yield testMixup(shortcuts);
   yield testLooseDigits(shortcuts);
   yield testExactModifiers(shortcuts);
   yield testLooseShiftModifier(shortcuts);
   yield testStrictLetterShiftModifier(shortcuts);
   yield testAltModifier(shortcuts);
   yield testCommandOrControlModifier(shortcuts);
   yield testCtrlModifier(shortcuts);
   yield testInvalidShortcutString(shortcuts);
+  yield testCmdShiftShortcut(shortcuts);
   shortcuts.destroy();
 
   yield testTarget();
 });
 
 // Test helper to listen to the next key press for a given key,
 // returning a promise to help using Tasks.
 function once(shortcuts, key, listener) {
@@ -347,16 +349,50 @@ function testCtrlModifier(shortcuts) {
   EventUtils.synthesizeKey(
     "VK_F1",
     { ctrlKey: true },
     window);
   yield onKey;
   yield onKeyAlias;
 }
 
+function testCmdShiftShortcut(shortcuts) {
+  if (!isOSX) {
+    // This test is OSX only (Bug 1300458).
+    return;
+  }
+
+  let onCmdKey = once(shortcuts, "CmdOrCtrl+[", (key, event) => {
+    is(event.key, "[");
+    ok(!event.altKey);
+    ok(!event.ctrlKey);
+    ok(event.metaKey);
+    ok(!event.shiftKey);
+  });
+  let onCmdShiftKey = once(shortcuts, "CmdOrCtrl+Shift+[", (key, event) => {
+    is(event.key, "[");
+    ok(!event.altKey);
+    ok(!event.ctrlKey);
+    ok(event.metaKey);
+    ok(event.shiftKey);
+  });
+
+  EventUtils.synthesizeKey(
+    "[",
+    { metaKey: true, shiftKey: true },
+    window);
+  EventUtils.synthesizeKey(
+    "[",
+    { metaKey: true },
+    window);
+
+  yield onCmdKey;
+  yield onCmdShiftKey;
+}
+
 function testTarget() {
   info("Test KeyShortcuts with target argument");
 
   let target = document.createElementNS("http://www.w3.org/1999/xhtml",
     "input");
   document.documentElement.appendChild(target);
   target.focus();