Bug 1014090 - Toggle inspector on mac with extra cmd+shift+C shortcut; r=pbro draft
authorChris Adams <chris@productscience.co.uk>
Wed, 23 May 2018 08:06:14 +0200
changeset 800508 86575e369b0c
parent 800491 a466172aed4b
push id111392
push userbmo:pbrosset@mozilla.com
push dateMon, 28 May 2018 13:04:51 +0000
reviewerspbro
bugs1014090
milestone62.0a1
Bug 1014090 - Toggle inspector on mac with extra cmd+shift+C shortcut; r=pbro MozReview-Commit-ID: KSdkGmVqVFd
devtools/client/definitions.js
devtools/client/framework/devtools-browser.js
devtools/client/framework/test/browser_keybindings_01.js
devtools/client/locales/en-US/startup.properties
devtools/startup/devtools-startup.js
--- a/devtools/client/definitions.js
+++ b/devtools/client/definitions.js
@@ -68,19 +68,23 @@ Tools.inspector = {
   id: "inspector",
   accesskey: l10n("inspector.accesskey"),
   ordinal: 1,
   icon: "chrome://devtools/skin/images/tool-inspector.svg",
   url: "chrome://devtools/content/inspector/inspector.xhtml",
   label: l10n("inspector.label"),
   panelLabel: l10n("inspector.panelLabel"),
   get tooltip() {
-    return l10n("inspector.tooltip2",
-    (osString == "Darwin" ? "Cmd+Opt+" : "Ctrl+Shift+") +
-    l10n("inspector.commandkey"));
+    if (osString == "Darwin") {
+      let cmdShiftC = "Cmd+Shift+" + l10n("inspector.commandkey");
+      let cmdOptC = "Cmd+Opt+" + l10n("inspector.commandkey");
+      return l10n("inspector.mac.tooltip", cmdShiftC, cmdOptC);
+    }
+
+    return l10n("inspector.tooltip2", "Ctrl+Shift+") + l10n("inspector.commandkey");
   },
   inMenu: true,
   commands: [
     "devtools/client/responsive.html/commands",
     "devtools/client/inspector/inspector-commands"
   ],
 
   preventClosingOnKey: true,
@@ -615,23 +619,23 @@ exports.ToolboxButtons = [
   },
 ];
 
 /**
  * Lookup l10n string from a string bundle.
  *
  * @param {string} name
  *        The key to lookup.
- * @param {string} arg
+ * @param {...string} args
  *        Optional format argument.
  * @returns A localized version of the given key.
  */
-function l10n(name, arg) {
+function l10n(name, ...args) {
   try {
-    return arg ? L10N.getFormatStr(name, arg) : L10N.getStr(name);
+    return args ? L10N.getFormatStr(name, ...args) : L10N.getStr(name);
   } catch (ex) {
     console.log("Error reading '" + name + "'");
     throw new Error("l10n error with " + name);
   }
 }
 
 function functionkey(shortkey) {
   return shortkey.split("_")[1];
--- a/devtools/client/framework/devtools-browser.js
+++ b/devtools/client/framework/devtools-browser.js
@@ -258,16 +258,19 @@ var gDevToolsBrowser = exports.gDevTools
       case "responsiveDesignMode":
         ResponsiveUIManager.toggle(window, window.gBrowser.selectedTab, {
           trigger: "shortcut"
         });
         break;
       case "scratchpad":
         ScratchpadManager.openScratchpad();
         break;
+      case "inspectorMac":
+        gDevToolsBrowser.selectToolCommand(window.gBrowser, "inspector", startTime);
+        break;
     }
   },
 
   /**
    * Open a tab on "about:debugging", optionally pre-select a given tab.
    */
    // Used by browser-sets.inc, command
   openAboutDebugging(gBrowser, hash) {
--- a/devtools/client/framework/test/browser_keybindings_01.js
+++ b/devtools/client/framework/test/browser_keybindings_01.js
@@ -7,43 +7,46 @@
 
 // Tests that the keybindings for opening and closing the inspector work as expected
 // Can probably make this a shared test that tests all of the tools global keybindings
 const TEST_URL = "data:text/html,<html><head><title>Test for the " +
                  "highlighter keybindings</title></head><body>" +
                  "<h1>Keybindings!</h1></body></html>";
 
 const {gDevToolsBrowser} = require("devtools/client/framework/devtools-browser");
-let keysetMap = { };
+ChromeUtils.defineModuleGetter(this, "AppConstants",
+  "resource://gre/modules/AppConstants.jsm");
+const isMac = AppConstants.platform == "macosx";
 
+let allKeys = [];
 function buildDevtoolsKeysetMap(keyset) {
-  [].forEach.call(
-    keyset.querySelectorAll("key"),
-    function(key) {
-      if (!key.getAttribute("key")) {
-        return;
-      }
-
-      let modifiers = key.getAttribute("modifiers");
+  // Fetches all the keyboard shortcuts which were defined by lazyGetter 'KeyShortcuts' in
+  // devtools-startup.js and added to the DOM by 'hookKeyShortcuts'
+  [...keyset.querySelectorAll("key")].forEach(key => {
+    if (!key.getAttribute("key")) {
+      return;
+    }
 
-      keysetMap[key.id.split("_")[1]] = {
-        key: key.getAttribute("key"),
-        modifiers: modifiers,
-        modifierOpt: {
-          shiftKey: modifiers.match("shift"),
-          ctrlKey: modifiers.match("ctrl"),
-          altKey: modifiers.match("alt"),
-          metaKey: modifiers.match("meta"),
-          accelKey: modifiers.match("accel")
-        },
-        synthesizeKey: function() {
-          EventUtils.synthesizeKey(this.key, this.modifierOpt);
-        }
-      };
+    let modifiers = key.getAttribute("modifiers");
+    allKeys.push({
+      toolId: key.id.split("_")[1],
+      key: key.getAttribute("key"),
+      modifiers: modifiers,
+      modifierOpt: {
+        shiftKey: modifiers.match("shift"),
+        ctrlKey: modifiers.match("ctrl"),
+        altKey: modifiers.match("alt"),
+        metaKey: modifiers.match("meta"),
+        accelKey: modifiers.match("accel")
+      },
+      synthesizeKey: function() {
+        EventUtils.synthesizeKey(this.key, this.modifierOpt);
+      }
     });
+  });
 }
 
 function setupKeyBindingsTest() {
   for (let win of gDevToolsBrowser._trackedBrowserWindows) {
     buildDevtoolsKeysetMap(win.document.getElementById("devtoolsKeyset"));
   }
 }
 
@@ -54,46 +57,68 @@ add_task(async function() {
     ["devtools.debugger.new-debugger-frontend", true]
   ]});
 
   await addTab(TEST_URL);
   await new Promise(done => waitForFocus(done));
 
   setupKeyBindingsTest();
 
+  info("Test the first inspector key (there are 2 of them on Mac)");
+  let inspectorKeys = allKeys.filter(({ toolId }) => {
+    return toolId === "inspector" || toolId === "inspectorMac";
+  });
+
+  if (isMac) {
+    is(inspectorKeys.length, 2, "There are 2 inspector keys on Mac");
+  } else {
+    is(inspectorKeys.length, 1, "Only 1 inspector key on non-Mac platforms");
+  }
+
+  info("The first inspector key should open the toolbox.");
   let onToolboxReady = gDevTools.once("toolbox-ready");
-  keysetMap.inspector.synthesizeKey();
+  inspectorKeys[0].synthesizeKey();
   let toolbox = await onToolboxReady;
-
-  await inspectorShouldBeOpenAndHighlighting();
+  await inspectorShouldBeOpenAndHighlighting(inspectorKeys[0]);
 
   let onSelectTool = gDevTools.once("select-tool-command");
-  keysetMap.webconsole.synthesizeKey();
+  let webconsole = allKeys.filter(({ toolId }) => toolId === "webconsole")[0];
+  webconsole.synthesizeKey();
   await onSelectTool;
   await webconsoleShouldBeSelected();
 
   onSelectTool = gDevTools.once("select-tool-command");
-  keysetMap.jsdebugger.synthesizeKey();
+  let jsdebugger = allKeys.filter(({ toolId }) => toolId === "jsdebugger")[0];
+  jsdebugger.synthesizeKey();
   await onSelectTool;
   await jsdebuggerShouldBeSelected();
 
   onSelectTool = gDevTools.once("select-tool-command");
-  keysetMap.netmonitor.synthesizeKey();
+  let netmonitor = allKeys.filter(({ toolId }) => toolId === "netmonitor")[0];
+  netmonitor.synthesizeKey();
   await onSelectTool;
   await netmonitorShouldBeSelected();
 
+  if (isMac) {
+    info("On MacOS, we check the extra inspector shortcut too");
+    onSelectTool = gDevTools.once("select-tool-command");
+    inspectorKeys[1].synthesizeKey();
+    await onSelectTool;
+    await inspectorShouldBeOpenAndHighlighting(inspectorKeys[1]);
+  }
+
   gBrowser.removeCurrentTab();
 
-  async function inspectorShouldBeOpenAndHighlighting() {
+  async function inspectorShouldBeOpenAndHighlighting(inspector) {
     is(toolbox.currentToolId, "inspector", "Correct tool has been loaded");
 
     await toolbox.once("picker-started");
 
     ok(true, "picker-started event received, highlighter started");
-    keysetMap.inspector.synthesizeKey();
+    inspector.synthesizeKey();
 
     await toolbox.once("picker-stopped");
     ok(true, "picker-stopped event received, highlighter stopped");
   }
 
   function webconsoleShouldBeSelected() {
     is(toolbox.currentToolId, "webconsole", "webconsole should be selected.");
   }
--- a/devtools/client/locales/en-US/startup.properties
+++ b/devtools/client/locales/en-US/startup.properties
@@ -149,16 +149,22 @@ inspector.accesskey=I
 # LOCALIZATION NOTE (inspector.panelLabel)
 # Labels applied to the panel and views within the panel in the toolbox
 inspector.panelLabel=Inspector Panel
 
 # LOCALIZATION NOTE (inspector.tooltip2)
 # Keyboard shortcut for DOM and Style Inspector will be shown inside brackets.
 inspector.tooltip2=DOM and Style Inspector (%S)
 
+# LOCALIZATION NOTE (inspector.mac.tooltip)
+# This is the exact same string as inspector.tooltip2, except that we show it
+# on mac only, where we support toggling the inspector with either cmd+shift+C,
+# or cmd+opt+C
+inspector.mac.tooltip=DOM and Style Inspector (%1$S or %2$S)
+
 # LOCALIZATION NOTE (netmonitor.label):
 # This string is displayed in the title of the tab when the Network Monitor is
 # displayed inside the developer tools window and in the Developer Tools Menu.
 netmonitor.label=Network
 
 # LOCALIZATION NOTE (netmonitor.panelLabel):
 # This is used as the label for the toolbox panel.
 netmonitor.panelLabel=Network Panel
--- a/devtools/startup/devtools-startup.js
+++ b/devtools/startup/devtools-startup.js
@@ -64,17 +64,17 @@ XPCOMUtils.defineLazyGetter(this, "KeySh
 XPCOMUtils.defineLazyGetter(this, "KeyShortcuts", function() {
   const isMac = AppConstants.platform == "macosx";
 
   // Common modifier shared by most key shortcuts
   const modifiers = isMac ? "accel,alt" : "accel,shift";
 
   // List of all key shortcuts triggering installation UI
   // `id` should match tool's id from client/definitions.js
-  return [
+  let shortcuts = [
     // The following keys are also registered in /client/menus.js
     // And should be synced.
 
     // Both are toggling the toolbox on the last selected panel
     // or the default one.
     {
       id: "toggleToolbox",
       shortcut: KeyShortcutsBundle.GetStringFromName("toggleToolbox.commandkey"),
@@ -164,16 +164,28 @@ XPCOMUtils.defineLazyGetter(this, "KeySh
     },
     // Key for opening the DOM Panel
     {
       toolId: "dom",
       shortcut: KeyShortcutsBundle.GetStringFromName("dom.commandkey"),
       modifiers
     },
   ];
+
+  if (isMac) {
+    // Add the extra key command for macOS, so you can open the inspector with cmd+shift+C
+    // like on Chrome DevTools.
+    shortcuts.push({
+      id: "inspectorMac",
+      shortcut: KeyShortcutsBundle.GetStringFromName("inspector.commandkey"),
+      modifiers: "accel,shift"
+    });
+  }
+
+  return shortcuts;
 });
 
 function DevToolsStartup() {
   this.onEnabledPrefChanged = this.onEnabledPrefChanged.bind(this);
   this.onWindowReady = this.onWindowReady.bind(this);
 }
 
 DevToolsStartup.prototype = {