From e3949549a8d509b337cc58438c48a9696b599784 Mon Sep 17 00:00:00 2001
From: Jan Odvarko <odvarko@gmail.com>
Date: Wed, 3 Oct 2018 11:38:49 +0000
Subject: [PATCH] Bug 1493646 - Browser toolbox shortcut closes the regular
 toolbox; r=jdescottes

Differential Revision: https://phabricator.services.mozilla.com/D7092

--HG--
extra : moz-landing-system : lando
---
 devtools/client/shared/key-shortcuts.js       |  9 ++++-
 .../shared/test/browser_key_shortcuts.js      | 38 +++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/devtools/client/shared/key-shortcuts.js b/devtools/client/shared/key-shortcuts.js
index ebfefb8debfa..39e848a8f001 100644
--- a/devtools/client/shared/key-shortcuts.js
+++ b/devtools/client/shared/key-shortcuts.js
@@ -223,9 +223,16 @@ KeyShortcuts.prototype = {
       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) {
diff --git a/devtools/client/shared/test/browser_key_shortcuts.js b/devtools/client/shared/test/browser_key_shortcuts.js
index 1021f5d17ddb..3e1d4a37ab77 100644
--- a/devtools/client/shared/test/browser_key_shortcuts.js
+++ b/devtools/client/shared/test/browser_key_shortcuts.js
@@ -24,6 +24,7 @@ add_task(async function() {
   await testCtrlModifier(shortcuts);
   await testInvalidShortcutString(shortcuts);
   await testCmdShiftShortcut(shortcuts);
+  await testTabCharacterShortcut(shortcuts);
   shortcuts.destroy();
 
   await testTarget();
@@ -422,3 +423,40 @@ function testInvalidShortcutString(shortcuts) {
   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;
+}