Bug 1359855 - Fix support of per tool key shortcuts in toolboxes opened in a window. r=jdescottes draft
authorAlexandre Poirot <poirot.alex@gmail.com>
Thu, 13 Jul 2017 15:27:37 +0200
changeset 614028 8c04f02c311452f74b8c98ae0368ada30a0c13bf
parent 614027 d0b3ba39a442b7ce15af35ecb40730a8a25efd33
child 614029 1cb9196ccf12bda23616adf92250562787804a24
push id69893
push userbmo:poirot.alex@gmail.com
push dateSun, 23 Jul 2017 20:50:47 +0000
reviewersjdescottes
bugs1359855
milestone56.0a1
Bug 1359855 - Fix support of per tool key shortcuts in toolboxes opened in a window. r=jdescottes MozReview-Commit-ID: kP07KzpzxI
devtools/client/devtools-startup.js
devtools/client/framework/test/browser_toolbox_window_shortcuts.js
devtools/client/framework/toolbox.js
--- a/devtools/client/devtools-startup.js
+++ b/devtools/client/devtools-startup.js
@@ -179,16 +179,17 @@ DevToolsStartup.prototype = {
       // We get an error if the option is given but not followed by a value.
       // By catching and trying again, the value is effectively optional.
       debuggerServerFlag = cmdLine.handleFlag("start-debugger-server", false);
     }
     if (debuggerServerFlag) {
       this.handleDebuggerServerFlag(cmdLine, debuggerServerFlag);
     }
 
+    // Only top level Firefox Windows fire a browser-delayed-startup-finished event
     let onWindowReady = window => {
       this.hookWindow(window);
 
       if (devtoolsFlag) {
         this.handleDevToolsFlag(window);
         // This listener is called for all Firefox windows, but we want to execute
         // that command only once
         devtoolsFlag = false;
@@ -507,16 +508,25 @@ DevToolsStartup.prototype = {
       dump("Unable to start debugger server on " + portOrPath + ": " + e);
     }
 
     if (cmdLine.state == Ci.nsICommandLine.STATE_REMOTE_AUTO) {
       cmdLine.preventDefault = true;
     }
   },
 
+  // Used by tests and the toolbox to register the same key shortcuts in toolboxes loaded
+  // in a window window.
+  get KeyShortcuts() {
+    return KeyShortcuts;
+  },
+  get wrappedJSObject() {
+    return this;
+  },
+
   /* eslint-disable max-len */
   helpInfo: "  --jsconsole        Open the Browser Console.\n" +
             "  --jsdebugger       Open the Browser Toolbox.\n" +
             "  --wait-for-jsdebugger Spin event loop until JS debugger connects.\n" +
             "                     Enables debugging (some) application startup code paths.\n" +
             "                     Only has an effect when `--jsdebugger` is also supplied.\n" +
             "  --devtools         Open DevTools on initial load.\n" +
             "  --start-debugger-server [ws:][ <port> | <path> ] Start the debugger server on\n" +
--- a/devtools/client/framework/test/browser_toolbox_window_shortcuts.js
+++ b/devtools/client/framework/test/browser_toolbox_window_shortcuts.js
@@ -1,37 +1,42 @@
 /* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set ft=javascript ts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 "use strict";
 
+var Startup = Cc["@mozilla.org/devtools/startup-clh;1"].getService(Ci.nsISupports)
+  .wrappedJSObject;
 var {Toolbox} = require("devtools/client/framework/toolbox");
 
-var toolbox, toolIDs, idIndex, modifiedPrefs = [];
+var toolbox, toolIDs, toolShortcuts = [], idIndex, modifiedPrefs = [];
 
 function test() {
   addTab("about:blank").then(function () {
     toolIDs = [];
     for (let [id, definition] of gDevTools._tools) {
-      if (definition.key) {
-        toolIDs.push(id);
+      let shortcut = Startup.KeyShortcuts.filter(s => s.toolId == id)[0];
+      if (!shortcut) {
+        continue;
+      }
+      toolIDs.push(id);
+      toolShortcuts.push(shortcut);
 
-        // Enable disabled tools
-        let pref = definition.visibilityswitch, prefValue;
-        try {
-          prefValue = Services.prefs.getBoolPref(pref);
-        } catch (e) {
-          continue;
-        }
-        if (!prefValue) {
-          modifiedPrefs.push(pref);
-          Services.prefs.setBoolPref(pref, true);
-        }
+      // Enable disabled tools
+      let pref = definition.visibilityswitch, prefValue;
+      try {
+        prefValue = Services.prefs.getBoolPref(pref);
+      } catch (e) {
+        continue;
+      }
+      if (!prefValue) {
+        modifiedPrefs.push(pref);
+        Services.prefs.setBoolPref(pref, true);
       }
     }
     let target = TargetFactory.forTab(gBrowser.selectedTab);
     idIndex = 0;
     gDevTools.showToolbox(target, toolIDs[0], Toolbox.HostType.WINDOW)
              .then(testShortcuts);
   });
 }
@@ -44,18 +49,19 @@ function testShortcuts(aToolbox, aIndex)
     return;
   }
 
   toolbox = aToolbox;
   info("Toolbox fired a `ready` event");
 
   toolbox.once("select", selectCB);
 
-  let key = gDevTools._tools.get(toolIDs[aIndex]).key;
-  let toolModifiers = gDevTools._tools.get(toolIDs[aIndex]).modifiers;
+  let shortcut = toolShortcuts[aIndex];
+  let key = shortcut.shortcut;
+  let toolModifiers = shortcut.modifiers;
   let modifiers = {
     accelKey: toolModifiers.includes("accel"),
     altKey: toolModifiers.includes("alt"),
     shiftKey: toolModifiers.includes("shift"),
   };
   idIndex = aIndex;
   info("Testing shortcut for tool " + aIndex + ":" + toolIDs[aIndex] +
        " using key " + key);
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -9,29 +9,31 @@ const SOURCE_MAP_WORKER = "resource://de
 const MAX_ORDINAL = 99;
 const SPLITCONSOLE_ENABLED_PREF = "devtools.toolbox.splitconsoleEnabled";
 const SPLITCONSOLE_HEIGHT_PREF = "devtools.toolbox.splitconsoleHeight";
 const DISABLE_AUTOHIDE_PREF = "ui.popup.disable_autohide";
 const HOST_HISTOGRAM = "DEVTOOLS_TOOLBOX_HOST";
 const SCREENSIZE_HISTOGRAM = "DEVTOOLS_SCREEN_RESOLUTION_ENUMERATED_PER_USER";
 const HTML_NS = "http://www.w3.org/1999/xhtml";
 
-var {Ci, Cu} = require("chrome");
+var {Ci, Cu, Cc} = require("chrome");
 var promise = require("promise");
 var defer = require("devtools/shared/defer");
 var Services = require("Services");
 var {Task} = require("devtools/shared/task");
 var {gDevTools} = require("devtools/client/framework/devtools");
 var EventEmitter = require("devtools/shared/event-emitter");
 var Telemetry = require("devtools/client/shared/telemetry");
 var { attachThread, detachThread } = require("./attach-thread");
 var Menu = require("devtools/client/framework/menu");
 var MenuItem = require("devtools/client/framework/menu-item");
 var { DOMHelpers } = require("resource://devtools/client/shared/DOMHelpers.jsm");
 const { KeyCodes } = require("devtools/client/shared/keycodes");
+var Startup = Cc["@mozilla.org/devtools/startup-clh;1"].getService(Ci.nsISupports)
+  .wrappedJSObject;
 
 const { BrowserLoader } =
   Cu.import("resource://devtools/client/shared/browser-loader.js", {});
 
 const {LocalizationHelper} = require("devtools/shared/l10n");
 const L10N = new LocalizationHelper("devtools/client/locales/toolbox.properties");
 
 loader.lazyRequireGetter(this, "getHighlighterUtils",
@@ -852,34 +854,35 @@ Toolbox.prototype = {
    */
   _addKeysToWindow: function () {
     if (this.hostType != Toolbox.HostType.WINDOW) {
       return;
     }
 
     let doc = this.win.parent.document;
 
-    for (let [id, toolDefinition] of gDevTools.getToolDefinitionMap()) {
-      // Prevent multiple entries for the same tool.
-      if (!toolDefinition.key || doc.getElementById("key_" + id)) {
+    for (let item of Startup.KeyShortcuts) {
+      // KeyShortcuts contain tool-specific and global key shortcuts,
+      // here we only need to copy shortcut specific to each tool.
+      if (!item.toolId) {
         continue;
       }
+      let { toolId, shortcut, modifiers } = item;
 
-      let toolId = id;
       let key = doc.createElement("key");
 
       key.id = "key_" + toolId;
 
-      if (toolDefinition.key.startsWith("VK_")) {
-        key.setAttribute("keycode", toolDefinition.key);
+      if (shortcut.startsWith("VK_")) {
+        key.setAttribute("keycode", shortcut);
       } else {
-        key.setAttribute("key", toolDefinition.key);
+        key.setAttribute("key", shortcut);
       }
 
-      key.setAttribute("modifiers", toolDefinition.modifiers);
+      key.setAttribute("modifiers", modifiers);
       // needed. See bug 371900
       key.setAttribute("oncommand", "void(0);");
       key.addEventListener("command", () => {
         this.selectTool(toolId).then(() => this.fireCustomKey(toolId));
       }, true);
       doc.getElementById("toolbox-keyset").appendChild(key);
     }