Bug 1493646 - Browser toolbox shortcut closes the regular toolbox; r=jdescottes
authorJan Odvarko <odvarko@gmail.com>
Wed, 03 Oct 2018 11:38:49 +0000
changeset 439536 ae757626c524f74920547bc09afb4188752fcff4
parent 439535 cefa70540aacffeffaec04a4221c7ebfc0acceca
child 439537 2f1229c179d303c2df8c4fa7b05d7a4662f73422
push id34778
push usernbeleuzu@mozilla.com
push dateThu, 04 Oct 2018 15:22:02 +0000
treeherdermozilla-central@01634947caab [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes
bugs1493646
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1493646 - Browser toolbox shortcut closes the regular toolbox; r=jdescottes Differential Revision: https://phabricator.services.mozilla.com/D7092
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
@@ -218,19 +218,26 @@ KeyShortcuts.prototype = {
     }
     if (shortcut.ctrl != event.ctrlKey) {
       return false;
     }
     if (shortcut.alt != event.altKey) {
       return false;
     }
     if (shortcut.shift != event.shiftKey) {
+      // Check the `keyCode` to see whether it's a character (see also Bug 1493646)
+      const char = String.fromCharCode(event.keyCode);
+      let isAlphabetical = (char.length == 1 && char.match(/[a-zA-Z]/));
+
       // Shift is a special modifier, it may implicitly be required if the expected key
       // is a special character accessible via shift.
-      const isAlphabetical = event.key && event.key.match(/[a-zA-Z]/);
+      if (!isAlphabetical) {
+        isAlphabetical = event.key && event.key.match(/[a-zA-Z]/);
+      }
+
       // OSX: distinguish cmd+[key] from cmd+shift+[key] shortcuts (Bug 1300458)
       const cmdShortcut = shortcut.meta && !shortcut.alt && !shortcut.ctrl;
       if (isAlphabetical || cmdShortcut) {
         return false;
       }
     }
 
     if (shortcut.keyCode) {
--- a/devtools/client/shared/test/browser_key_shortcuts.js
+++ b/devtools/client/shared/test/browser_key_shortcuts.js
@@ -19,16 +19,17 @@ add_task(async function() {
   await testExactModifiers(shortcuts);
   await testLooseShiftModifier(shortcuts);
   await testStrictLetterShiftModifier(shortcuts);
   await testAltModifier(shortcuts);
   await testCommandOrControlModifier(shortcuts);
   await testCtrlModifier(shortcuts);
   await testInvalidShortcutString(shortcuts);
   await testCmdShiftShortcut(shortcuts);
+  await testTabCharacterShortcut(shortcuts);
   shortcuts.destroy();
 
   await 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) {
@@ -417,8 +418,45 @@ function testInvalidShortcutString(short
   info("Test wrong shortcut string");
 
   const shortcut = KeyShortcuts.parseElectronKey(window, "Cmmd+F");
   ok(!shortcut, "Passing a invalid shortcut string should return a null object");
 
   shortcuts.on("Cmmd+F", function() {});
   ok(true, "on() shouldn't throw when passing invalid shortcut string");
 }
+
+/**
+ * Shift+Alt+I generates ^ key (`event.key`) on OSX and KeyShortcuts module
+ * must ensure that this doesn't interfere with shortcuts CmdOrCtrl+Alt+Shift+I
+ * for opening the Browser Toolbox and CmdOrCtrl+Alt+I for toggling the Toolbox.
+ */
+async function testTabCharacterShortcut(shortcuts) {
+  if (!isOSX) {
+    return;
+  }
+
+  info("Test tab character shortcut");
+
+  once(shortcuts, "CmdOrCtrl+Alt+I", event => {
+    ok(false, "This handler must not be executed");
+  });
+
+  const onKey = once(shortcuts, "CmdOrCtrl+Alt+Shift+I", event => {
+    info("Test for CmdOrCtrl+Alt+Shift+I");
+    is(event.key, "^");
+    is(event.keyCode, 73);
+  });
+
+  // Simulate `CmdOrCtrl+Alt+Shift+I` shortcut. Note that EventUtils doesn't
+  // generate `^` like real keyboard, so we need to pass it explicitly
+  // and use proper keyCode for `I` character.
+  EventUtils.synthesizeKey("^", {
+    code: "KeyI",
+    key: "^",
+    keyCode: 73,
+    shiftKey: true,
+    altKey: true,
+    metaKey: true,
+  }, window);
+
+  await onKey;
+}