Bug 1268441 - Make Shift modifier only strict for letter characters. r=bgrins

devtools/client/shared/key-shortcuts.js

@@ -153,9 +153,9 @@ KeyShortcuts.prototype = {
       return false;
     }
     // Shift is a special modifier, it may implicitely be required if the
-    // expected key is a character and is only accessible via shift.
-    if (shortcut.shift != event.shiftKey &&
-        (shortcut.keyCode || (event.key.toLowerCase() !== shortcut.key))) {
+    // 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.keyCode) {

devtools/client/shared/test/browser_key_shortcuts.js

@@ -11,6 +11,7 @@ add_task(function* () {
   yield testMixup(shortcuts);
   yield testExactModifiers(shortcuts);
   yield testLooseShiftModifier(shortcuts);
+  yield testStrictLetterShiftModifier(shortcuts);
   yield testAltModifier(shortcuts);
   yield testCommandOrControlModifier(shortcuts);
   yield testCtrlModifier(shortcuts);
@@ -129,19 +130,67 @@ function testExactModifiers(shortcuts) {
 }
 
 // Some keys are only accessible via shift and listener should also be called
-// even if the key didn't explicitely requested Shift modifier
+// even if the key didn't explicitely requested Shift modifier.
+// For example, `%` on french keyboards is only accessible via Shift.
+// Same thing for `@` on US keybords.
 function testLooseShiftModifier(shortcuts) {
   info("Test Loose shift modifier");
-  let onKey = once(shortcuts, "Alt+A", (key, event) => {
+  let onKey = once(shortcuts, "%", (key, event) => {
+    is(event.key, "%");
+    ok(!event.altKey);
+    ok(!event.ctrlKey);
+    ok(!event.metaKey);
+    ok(event.shiftKey);
+  });
+  EventUtils.synthesizeKey(
+    "%",
+    { accelKey: false, altKey: false, ctrlKey: false, shiftKey: true},
+    window);
+  yield onKey;
+
+  onKey = once(shortcuts, "@", (key, event) => {
+    is(event.key, "@");
+    ok(!event.altKey);
+    ok(!event.ctrlKey);
+    ok(!event.metaKey);
+    ok(event.shiftKey);
+  });
+  EventUtils.synthesizeKey(
+    "@",
+    { accelKey: false, altKey: false, ctrlKey: false, shiftKey: true},
+    window);
+  yield onKey;
+}
+
+// But Shift modifier is strict on all letter characters (a to Z)
+function testStrictLetterShiftModifier(shortcuts) {
+  info("Test strict shift modifier on letters");
+  let hitFirst = false;
+  let onKey = once(shortcuts, "a", (key, event) => {
     is(event.key, "a");
-    ok(event.altKey);
+    ok(!event.altKey);
+    ok(!event.ctrlKey);
+    ok(!event.metaKey);
+    ok(!event.shiftKey);
+    hitFirst = true;
+  });
+  let onShiftKey = once(shortcuts, "Shift+a", (key, event) => {
+    is(event.key, "a");
+    ok(!event.altKey);
     ok(!event.ctrlKey);
     ok(!event.metaKey);
     ok(event.shiftKey);
   });
   EventUtils.synthesizeKey(
     "a",
-    { accelKey: false, altKey: true, ctrlKey: false, shiftKey: true},
+    { shiftKey: true},
+    window);
+  yield onShiftKey;
+  ok(!hitFirst, "Didn't fire the explicit shift+a");
+
+  EventUtils.synthesizeKey(
+    "a",
+    { shiftKey: false},
     window);
   yield onKey;
 }