Bug 1014090 - Toggle inspector on mac with extra cmd+shift+C shortcut; r=pbro
☠☠ backed out by d1e0a3f9e945 ☠ ☠
authorChris Adams <chris@productscience.co.uk>
Wed, 23 May 2018 08:06:14 +0200
changeset 419804 123010192259
parent 419803 b902cff89d26
child 419805 857fce2b698b
push id64434
push userpbrosset@mozilla.com
push dateFri, 25 May 2018 09:12:49 +0000
treeherderautoland@123010192259 [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/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/test/browser_keybindings_01.js
+++ b/devtools/client/framework/test/browser_keybindings_01.js
@@ -7,42 +7,44 @@
 
 // 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 = { };
+let allKeys = [];
 
 function buildDevtoolsKeysetMap(keyset) {
+  // Fetches all the keyboard shortcuts which were defined by lazyGetter 'KeyShortcuts' in
+  // devtools-startup.js and added to the DOM by 'hookKeyShortcuts'
   [].forEach.call(
     keyset.querySelectorAll("key"),
     function(key) {
       if (!key.getAttribute("key")) {
         return;
       }
 
       let modifiers = key.getAttribute("modifiers");
-
-      keysetMap[key.id.split("_")[1]] = {
+      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 +56,67 @@ 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 }) => toolId === "inspector");
+
+  const isMac = Services.appinfo.OS === "Darwin";
+  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("The 2nd inspector key (only on Mac) should switch to the inspector again.");
+    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({
+      toolId: "inspector",
+      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 = {