Bug 1014090 - Toggle inspector on mac with extra cmd+shift+C shortcut; r=pbro
authorChris Adams <chris@productscience.co.uk>
Wed, 23 May 2018 08:06:14 +0200
changeset 420199 4342f8a66345
parent 420198 3f21b5dd70a8
child 420200 238eb0ed2874
child 420219 f01bb6245db1
push id64545
push userpbrosset@mozilla.com
push dateTue, 29 May 2018 07:10:46 +0000
treeherderautoland@4342f8a66345 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1014090
milestone62.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 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 = {