From d48833fbbc1cfde6393785d77c719880096e47f9 Mon Sep 17 00:00:00 2001 From: Jan Odvarko Date: Wed, 19 Sep 2018 09:58:08 +0000 Subject: [PATCH] Bug 1491749 - Stop using xul:keyset and xul:key in toolbox-window.xul; r=bgrins,flod Differential Revision: https://phabricator.services.mozilla.com/D6015 --HG-- extra : moz-landing-system : lando --- devtools/client/framework/toolbox-hosts.js | 9 --- devtools/client/framework/toolbox-window.xul | 30 -------- devtools/client/framework/toolbox.js | 75 +++++++++---------- .../client/locales/en-US/toolbox.properties | 17 +++-- devtools/client/shared/key-shortcuts.js | 26 ++++++- 5 files changed, 71 insertions(+), 86 deletions(-) diff --git a/devtools/client/framework/toolbox-hosts.js b/devtools/client/framework/toolbox-hosts.js index c7fec8cfaf61..a8691ef405e9 100644 --- a/devtools/client/framework/toolbox-hosts.js +++ b/devtools/client/framework/toolbox-hosts.js @@ -11,7 +11,6 @@ const promise = require("promise"); const Services = require("Services"); const {DOMHelpers} = require("resource://devtools/client/shared/DOMHelpers.jsm"); -loader.lazyRequireGetter(this, "AppConstants", "resource://gre/modules/AppConstants.jsm", true); loader.lazyRequireGetter(this, "gDevToolsBrowser", "devtools/client/framework/devtools-browser", true); /* A host should always allow this much space for the page to be displayed. @@ -257,14 +256,6 @@ WindowHost.prototype = { win.removeEventListener("load", frameLoad, true); win.focus(); - let key; - if (AppConstants.platform === "macosx") { - key = win.document.getElementById("toolbox-key-toggle-osx"); - } else { - key = win.document.getElementById("toolbox-key-toggle"); - } - key.removeAttribute("disabled"); - this.frame = win.document.getElementById("toolbox-iframe"); this.emit("ready", this.frame); resolve(this.frame); diff --git a/devtools/client/framework/toolbox-window.xul b/devtools/client/framework/toolbox-window.xul index 7e5e1d0b14c8..52675c7e961b 100644 --- a/devtools/client/framework/toolbox-window.xul +++ b/devtools/client/framework/toolbox-window.xul @@ -2,10 +2,6 @@ - - %toolboxDTD; -]> @@ -17,32 +13,6 @@ windowtype="devtools:toolbox" width="900" height="320" persist="screenX screenY width height sizemode"> - - - - - - - - - - - - diff --git a/devtools/client/framework/toolbox.js b/devtools/client/framework/toolbox.js index eb435efe66fb..213bbc24b42a 100644 --- a/devtools/client/framework/toolbox.js +++ b/devtools/client/framework/toolbox.js @@ -37,6 +37,8 @@ const { BrowserLoader } = const {LocalizationHelper} = require("devtools/shared/l10n"); const L10N = new LocalizationHelper("devtools/client/locales/toolbox.properties"); +loader.lazyRequireGetter(this, "AppConstants", + "resource://gre/modules/AppConstants.jsm", true); loader.lazyRequireGetter(this, "getHighlighterUtils", "devtools/client/framework/toolbox-highlighter-utils", true); loader.lazyRequireGetter(this, "Selection", @@ -901,6 +903,7 @@ Toolbox.prototype = { }, _addHostListeners: function() { + // Add navigation keys this.shortcuts.on(L10N.getStr("toolbox.nextTool.key"), event => { this.selectNextTool(); @@ -917,6 +920,24 @@ Toolbox.prototype = { event.preventDefault(); }); + // Close toolbox key-shortcut handler + const onClose = event => this.destroy(); + this.shortcuts.on(L10N.getStr("toolbox.toggleToolboxF12.key"), onClose); + + // CmdOrCtrl+W is registered only when the toolbox is running in + // detached window. In the other case the entire browser tab + // is closed when the user uses this shortcut. + if (this.hostType == "window") { + this.shortcuts.on(L10N.getStr("toolbox.closeToolbox.key"), onClose); + } + + if (AppConstants.platform == "macosx") { + this.shortcuts.on(L10N.getStr("toolbox.toggleToolboxOSX.key"), onClose); + } else { + this.shortcuts.on(L10N.getStr("toolbox.toggleToolbox.key"), onClose); + } + + // Add event listeners this.doc.addEventListener("keypress", this._splitConsoleOnKeypress); this.doc.addEventListener("focus", this._onFocus, true); this.win.addEventListener("unload", this.destroy); @@ -991,48 +1012,22 @@ Toolbox.prototype = { return; } - const doc = this.win.parent.document; - for (const 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; + const { id, toolId, shortcut, modifiers } = item; + const electronKey = KeyShortcuts.parseXulKey(modifiers, shortcut); + + if (id == "browserConsole") { + // Add key for toggling the browser console from the detached window + this.shortcuts.on(electronKey, () => { + HUDService.toggleBrowserConsole(); + }); + } else if (toolId) { + // KeyShortcuts contain tool-specific and global key shortcuts, + // here we only need to copy shortcut specific to each tool. + this.shortcuts.on(electronKey, () => { + this.selectTool(toolId, "key_shortcut").then(() => this.fireCustomKey(toolId)); + }); } - const { toolId, shortcut, modifiers } = item; - - const key = doc.createXULElement("key"); - - key.id = "key_" + toolId; - - if (shortcut.startsWith("VK_")) { - key.setAttribute("keycode", shortcut); - } else { - key.setAttribute("key", shortcut); - } - - key.setAttribute("modifiers", modifiers); - // needed. See bug 371900 - key.setAttribute("oncommand", "void(0);"); - key.addEventListener("command", () => { - this.selectTool(toolId, "key_shortcut").then(() => this.fireCustomKey(toolId)); - }, true); - doc.getElementById("toolbox-keyset").appendChild(key); - } - - // Add key for toggling the browser console from the detached window - if (!doc.getElementById("key_browserconsole")) { - const key = doc.createXULElement("key"); - key.id = "key_browserconsole"; - - key.setAttribute("key", L10N.getStr("browserConsoleCmd.commandkey")); - key.setAttribute("modifiers", "accel,shift"); - // needed. See bug 371900 - key.setAttribute("oncommand", "void(0)"); - key.addEventListener("command", () => { - HUDService.toggleBrowserConsole(); - }, true); - doc.getElementById("toolbox-keyset").appendChild(key); } }, diff --git a/devtools/client/locales/en-US/toolbox.properties b/devtools/client/locales/en-US/toolbox.properties index 136e8a56c182..3128387af5b4 100644 --- a/devtools/client/locales/en-US/toolbox.properties +++ b/devtools/client/locales/en-US/toolbox.properties @@ -32,11 +32,6 @@ options.toolNotSupportedMarker=%1$S * # Needs to match scratchpad.keycode from browser.dtd scratchpad.keycode=VK_F4 -# LOCALIZATION NOTE (browserConsoleCmd.commandkey) -# Used for toggling the browser console from the detached toolbox window -# Needs to match browserConsoleCmd.commandkey from browser.dtd -browserConsoleCmd.commandkey=j - # LOCALIZATION NOTE (pickButton.tooltip) # This is the tooltip of the pick button in the toolbox toolbar pickButton.tooltip=Pick an element from the page @@ -103,6 +98,18 @@ toolbox.forceReload2.key=CmdOrCtrl+F5 # Key shortcut used to move the toolbox in bottom or side of the browser window toolbox.toggleHost.key=CmdOrCtrl+Shift+D +# LOCALIZATION NOTE (toolbox.closeToolbox.key) Key shortcut used to close the toolbox +toolbox.closeToolbox.key=CmdOrCtrl+W + +# LOCALIZATION NOTE (toolbox.toggleToolbox.key) Key shortcut used to toggle the toolbox +toolbox.toggleToolbox.key=CmdOrCtrl+Shift+I + +# LOCALIZATION NOTE (toolbox.toggleToolboxOSX.key) Key shortcut used to toggle the toolbox +toolbox.toggleToolboxOSX.key=CmdOrCtrl+Alt+I + +# LOCALIZATION NOTE (toolbox.toggleToolboxF12.key) Key shortcut used to toggle the toolbox +toolbox.toggleToolboxF12.key=F12 + # LOCALIZATION NOTE (toolbox.frames.tooltip): This is the label for # the iframes menu list that appears only when the document has some. # It allows you to switch the context of the whole toolbox. diff --git a/devtools/client/shared/key-shortcuts.js b/devtools/client/shared/key-shortcuts.js index 138c5c628b72..ebfefb8debfa 100644 --- a/devtools/client/shared/key-shortcuts.js +++ b/devtools/client/shared/key-shortcuts.js @@ -59,7 +59,7 @@ const ElectronKeysMapping = { }; /** - * Helper to listen for keyboard events decribed in .properties file. + * Helper to listen for keyboard events described in .properties file. * * let shortcuts = new KeyShortcuts({ * window @@ -184,6 +184,28 @@ KeyShortcuts.stringify = function(shortcut) { return list.join("+"); }; +/* + * Parse an xul-like key string and return an electron-like string. + */ +KeyShortcuts.parseXulKey = function(modifiers, shortcut) { + modifiers = modifiers.split(",").map(mod => { + if (mod == "alt") { + return "Alt"; + } else if (mod == "shift") { + return "Shift"; + } else if (mod == "accel") { + return "CmdOrCtrl"; + } + return mod; + }).join("+"); + + if (shortcut.startsWith("VK_")) { + shortcut = shortcut.substr(3); + } + + return modifiers + "+" + shortcut; +}; + KeyShortcuts.prototype = { destroy() { this.target.removeEventListener("keydown", this); @@ -201,7 +223,7 @@ KeyShortcuts.prototype = { return false; } if (shortcut.shift != event.shiftKey) { - // Shift is a special modifier, it may implicitely be required if the expected key + // Shift is a special modifier, it may implicitly be required if the expected key // is a special character accessible via shift. const isAlphabetical = event.key && event.key.match(/[a-zA-Z]/); // OSX: distinguish cmd+[key] from cmd+shift+[key] shortcuts (Bug 1300458)