Bug 1300458 - Devtools key shortcuts fix shift+cmd shortcuts on OSX. r=ochameau, a=ritu
authorJulian Descottes <jdescottes@mozilla.com>
Thu, 08 Sep 2016 09:54:58 +0200
changeset 350154 fe5e0299f26d8b493f325f7fe2fe474a69e41f8c
parent 350153 befed59939f765b9bb263572d7c16e096612986b
child 350155 5a6b175148fc5b68e32fe15b1fa327dabad1df80
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau, ritu
bugs1300458
milestone50.0a2
Bug 1300458 - Devtools key shortcuts fix shift+cmd shortcuts on OSX. r=ochameau, a=ritu MozReview-Commit-ID: B1OZFsCBI2c
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
@@ -185,21 +185,25 @@ 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;
     }
     // For character keys, we match if the final character is the expected one.
     // But for digits we also accept indirect match to please azerty keyboard,
     // which requires Shift to be pressed to get digits.
     return event.key.toLowerCase() == shortcut.key ||
--- a/devtools/client/shared/test/browser_key_shortcuts.js
+++ b/devtools/client/shared/test/browser_key_shortcuts.js
@@ -14,16 +14,17 @@ add_task(function* () {
   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) {
@@ -335,16 +336,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();