Bug 1297113 - Convert useKeysWithSplitConsole to use key-shorcut.js;r=ochameau
authorJulian Descottes <jdescottes@mozilla.com>
Fri, 16 Sep 2016 14:32:26 +0200
changeset 314779 74c3bbe4739388c9a29334e49334d6752dad2dc0
parent 314778 0bf1a6786a761059171fea8039ce6d1b2b4da82a
child 314780 bd6a9044d4bf00c5f4710e6104f611620dd7bc74
push id30734
push usercbook@mozilla.com
push dateThu, 22 Sep 2016 09:54:25 +0000
treeherdermozilla-central@612a50c53506 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau
bugs1297113
milestone52.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 1297113 - Convert useKeysWithSplitConsole to use key-shorcut.js;r=ochameau MozReview-Commit-ID: 6pbjxNJaTcs
devtools/client/debugger/panel.js
devtools/client/debugger/views/toolbar-view.js
devtools/client/framework/test/browser_toolbox_split_console.js
devtools/client/framework/toolbox.js
--- a/devtools/client/debugger/panel.js
+++ b/devtools/client/debugger/panel.js
@@ -52,27 +52,67 @@ DebuggerPanel.prototype = {
       .then(() => this._controller.connect())
       .then(() => {
         this._toolbox.on("host-changed", this.handleHostChanged);
         // Add keys from this document's keyset to the toolbox, so they
         // can work when the split console is focused.
         let keysToClone = ["resumeKey", "stepOverKey", "stepInKey", "stepOutKey"];
         for (let key of keysToClone) {
           let elm = this.panelWin.document.getElementById(key);
-          this._toolbox.useKeyWithSplitConsole(elm, "jsdebugger");
+          let keycode = elm.getAttribute("keycode");
+          let modifiers = elm.getAttribute("modifiers");
+          let command = elm.getAttribute("command");
+          let handler = this._view.Toolbar.getCommandHandler(command);
+
+          let keyShortcut = this.translateToKeyShortcut(keycode, modifiers);
+          this._toolbox.useKeyWithSplitConsole(keyShortcut, handler, "jsdebugger");
         }
         this.isReady = true;
         this.emit("ready");
         return this;
       })
       .then(null, function onError(aReason) {
         DevToolsUtils.reportException("DebuggerPanel.prototype.open", aReason);
       });
   },
 
+  /**
+   * Translate a VK_ keycode, with modifiers, to a key shortcut that can be used with
+   * shared/key-shortcut.
+   *
+   * @param {String} keycode
+   *        The VK_* keycode to translate
+   * @param {String} modifiers
+   *        The list (blank-space separated) of modifiers applying to this keycode.
+   * @return {String} a key shortcut ready to be used with shared/key-shortcut.js
+   */
+  translateToKeyShortcut: function (keycode, modifiers) {
+    // Remove the VK_ prefix.
+    keycode = keycode.replace("VK_", "");
+
+    // Translate modifiers
+    if (modifiers.includes("shift")) {
+      keycode = "Shift+" + keycode;
+    }
+    if (modifiers.includes("alt")) {
+      keycode = "Alt+" + keycode;
+    }
+    if (modifiers.includes("control")) {
+      keycode = "Ctrl+" + keycode;
+    }
+    if (modifiers.includes("meta")) {
+      keycode = "Cmd+" + keycode;
+    }
+    if (modifiers.includes("accel")) {
+      keycode = "CmdOrCtrl+" + keycode;
+    }
+
+    return keycode;
+  },
+
   // DevToolPanel API
 
   get target() {
     return this._toolbox.target;
   },
 
   destroy: function () {
     // Make sure this panel is not already destroyed.
--- a/devtools/client/debugger/views/toolbar-view.js
+++ b/devtools/client/debugger/views/toolbar-view.js
@@ -96,24 +96,46 @@ ToolbarView.prototype = {
     this._stepOutButton.removeEventListener("mousedown", this._onStepOutPressed, false);
   },
 
   /**
    * Add commands that XUL can fire.
    */
   _addCommands: function () {
     XULUtils.addCommands(document.getElementById("debuggerCommands"), {
-      resumeCommand: () => this._onResumePressed(),
-      stepOverCommand: () => this._onStepOverPressed(),
-      stepInCommand: () => this._onStepInPressed(),
-      stepOutCommand: () => this._onStepOutPressed()
+      resumeCommand: this.getCommandHandler("resumeCommand"),
+      stepOverCommand: this.getCommandHandler("stepOverCommand"),
+      stepInCommand: this.getCommandHandler("stepInCommand"),
+      stepOutCommand: this.getCommandHandler("stepOutCommand")
     });
   },
 
   /**
+   * Retrieve the callback associated with the provided  debugger command.
+   *
+   * @param {String} command
+   *        The debugger command id.
+   * @return {Function} the corresponding callback.
+   */
+  getCommandHandler: function (command) {
+    switch (command) {
+      case "resumeCommand":
+        return () => this._onResumePressed();
+      case "stepOverCommand":
+        return () => this._onStepOverPressed();
+      case "stepInCommand":
+        return () => this._onStepInPressed();
+      case "stepOutCommand":
+        return () => this._onStepOutPressed();
+      default:
+        return () => {};
+    }
+  },
+
+  /**
    * Display a warning when trying to resume a debuggee while another is paused.
    * Debuggees must be unpaused in a Last-In-First-Out order.
    *
    * @param string aPausedUrl
    *        The URL of the last paused debuggee.
    */
   showResumeWarning: function (aPausedUrl) {
     let label = L10N.getFormatStr("resumptionOrderPanelTitle", aPausedUrl);
--- a/devtools/client/framework/test/browser_toolbox_split_console.js
+++ b/devtools/client/framework/test/browser_toolbox_split_console.js
@@ -1,22 +1,23 @@
 /* -*- 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";
+
 // Tests that these toolbox split console APIs work:
 //  * toolbox.useKeyWithSplitConsole()
 //  * toolbox.isSplitConsoleFocused
 
 let gToolbox = null;
 let panelWin = null;
 
 const URL = "data:text/html;charset=utf8,test split console key delegation";
-const XULNS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
 // Force the old debugger UI since it's directly used (see Bug 1301705)
 Services.prefs.setBoolPref("devtools.debugger.new-debugger-frontend", false);
 registerCleanupFunction(function* () {
   Services.prefs.clearUserPref("devtools.debugger.new-debugger-frontend");
 });
 
 add_task(function* () {
@@ -40,48 +41,42 @@ function* testIsSplitConsoleFocused() {
   panelWin.focus();
   ok(!gToolbox.isSplitConsoleFocused(), "Split console is no longer focused");
 }
 
 // A key bound to the selected tool should trigger it's command
 function* testUseKeyWithSplitConsole() {
   let commandCalled = false;
 
-  let keyElm = panelWin.document.createElementNS(XULNS, "key");
-  keyElm.setAttribute("keycode", "VK_F3");
-  keyElm.addEventListener("command", () => {commandCalled = true;}, false);
-  panelWin.document.getElementsByTagName("keyset")[0].appendChild(keyElm);
-
   info("useKeyWithSplitConsole on debugger while debugger is focused");
-  gToolbox.useKeyWithSplitConsole(keyElm, "jsdebugger");
+  gToolbox.useKeyWithSplitConsole("F3", () => {
+    commandCalled = true;
+  }, "jsdebugger");
 
   info("synthesizeKey with the console focused");
   let consoleInput = gToolbox.getPanel("webconsole").hud.jsterm.inputNode;
   consoleInput.focus();
-  synthesizeKeyElement(keyElm);
+  synthesizeKeyShortcut("F3", panelWin);
 
   ok(commandCalled, "Shortcut key should trigger the command");
 }
 
 // A key bound to a *different* tool should not trigger it's command
 function* testUseKeyWithSplitConsoleWrongTool() {
   let commandCalled = false;
 
-  let keyElm = panelWin.document.createElementNS(XULNS, "key");
-  keyElm.setAttribute("keycode", "VK_F4");
-  keyElm.addEventListener("command", () => {commandCalled = true;}, false);
-  panelWin.document.getElementsByTagName("keyset")[0].appendChild(keyElm);
-
   info("useKeyWithSplitConsole on inspector while debugger is focused");
-  gToolbox.useKeyWithSplitConsole(keyElm, "inspector");
+  gToolbox.useKeyWithSplitConsole("F4", () => {
+    commandCalled = true;
+  }, "inspector");
 
   info("synthesizeKey with the console focused");
   let consoleInput = gToolbox.getPanel("webconsole").hud.jsterm.inputNode;
   consoleInput.focus();
-  synthesizeKeyElement(keyElm);
+  synthesizeKeyShortcut("F4", panelWin);
 
   ok(!commandCalled, "Shortcut key shouldn't trigger the command");
 }
 
 function* cleanup() {
   // We don't want the open split console to confuse other tests..
   Services.prefs.clearUserPref("devtools.toolbox.splitconsoleEnabled");
   yield gToolbox.destroy();
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -401,27 +401,27 @@ Toolbox.prototype = {
       let noautohideMenu = this.doc.getElementById("command-button-noautohide");
       noautohideMenu.addEventListener("click", this._toggleAutohide, true);
 
       this.textboxContextMenuPopup =
         this.doc.getElementById("toolbox-textbox-context-popup");
       this.textboxContextMenuPopup.addEventListener("popupshowing",
         this._updateTextboxMenuItems, true);
 
-      let shortcuts = new KeyShortcuts({
+      this.shortcuts = new KeyShortcuts({
         window: this.doc.defaultView
       });
       this._buildDockButtons();
-      this._buildOptions(shortcuts);
+      this._buildOptions();
       this._buildTabs();
       this._applyCacheSettings();
       this._applyServiceWorkersTestingSettings();
       this._addKeysToWindow();
-      this._addReloadKeys(shortcuts);
-      this._addHostListeners(shortcuts);
+      this._addReloadKeys();
+      this._addHostListeners();
       this._registerOverlays();
       if (!this._hostOptions || this._hostOptions.zoom === true) {
         ZoomKeys.register(this.win);
       }
 
       this.tabbar = this.doc.querySelector(".devtools-tabbar");
       this.tabbar.addEventListener("focus", this._onTabbarFocus, true);
       this.tabbar.addEventListener("click", this._onTabbarFocus, true);
@@ -525,31 +525,31 @@ Toolbox.prototype = {
         this._applyCacheSettings();
         break;
       case "devtools.serviceWorkers.testing.enabled":
         this._applyServiceWorkersTestingSettings();
         break;
     }
   },
 
-  _buildOptions: function (shortcuts) {
+  _buildOptions: function () {
     let selectOptions = (name, event) => {
       // Flip back to the last used panel if we are already
       // on the options panel.
       if (this.currentToolId === "options" &&
           gDevTools.getToolDefinition(this.lastUsedToolId)) {
         this.selectTool(this.lastUsedToolId);
       } else {
         this.selectTool("options");
       }
       // Prevent the opening of bookmarks window on toolbox.options.key
       event.preventDefault();
     };
-    shortcuts.on(L10N.getStr("toolbox.options.key"), selectOptions);
-    shortcuts.on(L10N.getStr("toolbox.help.key"), selectOptions);
+    this.shortcuts.on(L10N.getStr("toolbox.options.key"), selectOptions);
+    this.shortcuts.on(L10N.getStr("toolbox.help.key"), selectOptions);
   },
 
   _splitConsoleOnKeypress: function (e) {
     if (e.keyCode === KeyCodes.DOM_VK_ESCAPE) {
       this.toggleSplitConsole();
       // If the debugger is paused, don't let the ESC key stop any pending
       // navigation.
       if (this._threadClient.state == "paused") {
@@ -557,69 +557,67 @@ Toolbox.prototype = {
       }
     }
   },
 
   /**
    * Add a shortcut key that should work when a split console
    * has focus to the toolbox.
    *
-   * @param {element} keyElement
-   *        They <key> XUL element describing the shortcut key
-   * @param {string} whichTool
-   *        The tool the key belongs to. The corresponding command
-   *        will only trigger if this tool is active.
+   * @param {String} key
+   *        The electron key shortcut.
+   * @param {Function} handler
+   *        The callback that should be called when the provided key shortcut is pressed.
+   * @param {String} whichTool
+   *        The tool the key belongs to. The corresponding handler will only be triggered
+   *        if this tool is active.
    */
-  useKeyWithSplitConsole: function (keyElement, whichTool) {
-    let cloned = keyElement.cloneNode();
-    cloned.setAttribute("oncommand", "void(0)");
-    cloned.removeAttribute("command");
-    cloned.addEventListener("command", (e) => {
-      // Only forward the command if the tool is active
+  useKeyWithSplitConsole: function (key, handler, whichTool) {
+    this.shortcuts.on(key, (name, event) => {
       if (this.currentToolId === whichTool && this.isSplitConsoleFocused()) {
-        keyElement.doCommand();
+        handler();
+        event.preventDefault();
       }
-    }, true);
-    this.doc.getElementById("toolbox-keyset").appendChild(cloned);
+    });
   },
 
-  _addReloadKeys: function (shortcuts) {
+  _addReloadKeys: function () {
     [
       ["reload", false],
       ["reload2", false],
       ["forceReload", true],
       ["forceReload2", true]
     ].forEach(([id, force]) => {
       let key = L10N.getStr("toolbox." + id + ".key");
-      shortcuts.on(key, (name, event) => {
+      this.shortcuts.on(key, (name, event) => {
         this.reloadTarget(force);
 
         // Prevent Firefox shortcuts from reloading the page
         event.preventDefault();
       });
     });
   },
 
-  _addHostListeners: function (shortcuts) {
-    shortcuts.on(L10N.getStr("toolbox.nextTool.key"),
+  _addHostListeners: function () {
+    this.shortcuts.on(L10N.getStr("toolbox.nextTool.key"),
                  (name, event) => {
                    this.selectNextTool();
                    event.preventDefault();
                  });
-    shortcuts.on(L10N.getStr("toolbox.previousTool.key"),
+    this.shortcuts.on(L10N.getStr("toolbox.previousTool.key"),
                  (name, event) => {
                    this.selectPreviousTool();
                    event.preventDefault();
                  });
-    shortcuts.on(L10N.getStr("toolbox.minimize.key"),
+    this.shortcuts.on(L10N.getStr("toolbox.minimize.key"),
                  (name, event) => {
                    this._toggleMinimizeMode();
                    event.preventDefault();
                  });
-    shortcuts.on(L10N.getStr("toolbox.toggleHost.key"),
+    this.shortcuts.on(L10N.getStr("toolbox.toggleHost.key"),
                  (name, event) => {
                    this.switchToPreviousHost();
                    event.preventDefault();
                  });
 
     this.doc.addEventListener("keypress", this._splitConsoleOnKeypress, false);
     this.doc.addEventListener("focus", this._onFocus, true);
     this.win.addEventListener("unload", this.destroy);