From 6505f8e6d7d3b6c9870697c50f6d44d3a97163cd Mon Sep 17 00:00:00 2001 From: Brian Grinstead Date: Fri, 2 Aug 2013 11:28:37 -0700 Subject: [PATCH] Bug 891556 - Add Ctrl-Shift-C (Cmd-Opt-C) shortcut to toggle highlighting; r=jwalker --- browser/devtools/framework/gDevTools.jsm | 13 +- browser/devtools/framework/test/Makefile.in | 1 + .../framework/test/browser_keybindings.js | 119 ++++++++++++++++++ browser/devtools/framework/toolbox.js | 19 ++- browser/devtools/inspector/highlighter.js | 2 - browser/devtools/inspector/inspector-panel.js | 4 - .../test/browser_inspector_bug_665880.js | 1 + .../test/browser_inspector_bug_674871.js | 1 + .../test/browser_inspector_iframeTest.js | 1 + .../test/browser_inspector_scrolling.js | 1 + browser/devtools/main.js | 7 ++ .../browser/devtools/inspector.properties | 2 +- 12 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 browser/devtools/framework/test/browser_keybindings.js diff --git a/browser/devtools/framework/gDevTools.jsm b/browser/devtools/framework/gDevTools.jsm index 5e1650cbd02..8db13b9f759 100644 --- a/browser/devtools/framework/gDevTools.jsm +++ b/browser/devtools/framework/gDevTools.jsm @@ -345,15 +345,24 @@ let gDevToolsBrowser = { selectToolCommand: function(gBrowser, toolId) { let target = devtools.TargetFactory.forTab(gBrowser.selectedTab); let toolbox = gDevTools.getToolbox(target); + let tools = gDevTools.getToolDefinitionMap(); + let toolDefinition = tools.get(toolId); if (toolbox && toolbox.currentToolId == toolId) { - if (toolbox.hostType == devtools.Toolbox.HostType.WINDOW) { + toolbox.fireCustomKey(toolId); + + if (toolDefinition.preventClosingOnKey || toolbox.hostType == devtools.Toolbox.HostType.WINDOW) { toolbox.raise(); } else { toolbox.destroy(); } } else { - gDevTools.showToolbox(target, toolId); + gDevTools.showToolbox(target, toolId).then(() => { + let target = devtools.TargetFactory.forTab(gBrowser.selectedTab); + let toolbox = gDevTools.getToolbox(target); + + toolbox.fireCustomKey(toolId); + }); } }, diff --git a/browser/devtools/framework/test/Makefile.in b/browser/devtools/framework/test/Makefile.in index 9d9ab0d9b2b..c9618e24880 100644 --- a/browser/devtools/framework/test/Makefile.in +++ b/browser/devtools/framework/test/Makefile.in @@ -30,6 +30,7 @@ MOCHITEST_BROWSER_FILES = \ browser_toolbox_options_disablejs_iframe.html \ browser_toolbox_highlight.js \ browser_toolbox_raise.js \ + browser_keybindings.js \ $(NULL) include $(topsrcdir)/config/rules.mk diff --git a/browser/devtools/framework/test/browser_keybindings.js b/browser/devtools/framework/test/browser_keybindings.js new file mode 100644 index 00000000000..b49a8f822a1 --- /dev/null +++ b/browser/devtools/framework/test/browser_keybindings.js @@ -0,0 +1,119 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +// 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 + +function test() +{ + waitForExplicitFinish(); + + let doc; + let node; + let inspector; + let keysetMap = { }; + + gBrowser.selectedTab = gBrowser.addTab(); + gBrowser.selectedBrowser.addEventListener("load", function onload() { + gBrowser.selectedBrowser.removeEventListener("load", onload, true); + doc = content.document; + waitForFocus(setupKeyBindingsTest, content); + }, true); + + content.location = "data:text/html,Test for the " + + "highlighter keybindings" + + "

Keybindings!

"; + + function buildDevtoolsKeysetMap(keyset) { + [].forEach.call(keyset.querySelectorAll("key"), function(key) { + + if (!key.getAttribute("key")) { + return; + } + + let modifiers = key.getAttribute("modifiers"); + + 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); + } + } + }); + } + + function setupKeyBindingsTest() + { + for (let win of gDevToolsBrowser._trackedBrowserWindows) { + buildDevtoolsKeysetMap(win.document.getElementById("devtoolsKeyset")); + } + + gDevTools.once("toolbox-ready", (e, toolbox) => { + inspectorShouldBeOpenAndHighlighting(toolbox.getCurrentPanel(), toolbox) + }); + + keysetMap.inspector.synthesizeKey(); + } + + function inspectorShouldBeOpenAndHighlighting(aInspector, aToolbox) + { + is (aToolbox.currentToolId, "inspector", "Correct tool has been loaded"); + is (aInspector.highlighter.locked, true, "Highlighter should be locked"); + + aInspector.highlighter.once("unlocked", () => { + is (aInspector.highlighter.locked, false, "Highlighter should be unlocked"); + keysetMap.inspector.synthesizeKey(); + is (aInspector.highlighter.locked, true, "Highlighter should be locked"); + keysetMap.inspector.synthesizeKey(); + is (aInspector.highlighter.locked, false, "Highlighter should be unlocked"); + keysetMap.inspector.synthesizeKey(); + is (aInspector.highlighter.locked, true, "Highlighter should be locked"); + + aToolbox.once("webconsole-ready", (e, panel) => { + webconsoleShouldBeSelected(aToolbox, panel); + }); + keysetMap.webconsole.synthesizeKey(); + }); + } + + function webconsoleShouldBeSelected(aToolbox, panel) + { + is (aToolbox.currentToolId, "webconsole"); + + aToolbox.once("jsdebugger-ready", (e, panel) => { + jsdebuggerShouldBeSelected(aToolbox, panel); + }); + keysetMap.jsdebugger.synthesizeKey(); + } + + function jsdebuggerShouldBeSelected(aToolbox, panel) + { + is (aToolbox.currentToolId, "jsdebugger"); + + aToolbox.once("netmonitor-ready", (e, panel) => { + netmonitorShouldBeSelected(aToolbox, panel); + }); + + keysetMap.netmonitor.synthesizeKey(); + } + + function netmonitorShouldBeSelected(aToolbox, panel) + { + is (aToolbox.currentToolId, "netmonitor"); + finishUp(); + } + + function finishUp() { + doc = node = inspector = keysetMap = null; + gBrowser.removeCurrentTab(); + finish(); + } +} diff --git a/browser/devtools/framework/toolbox.js b/browser/devtools/framework/toolbox.js index efece2c1225..37568fb72e1 100644 --- a/browser/devtools/framework/toolbox.js +++ b/browser/devtools/framework/toolbox.js @@ -264,13 +264,30 @@ Toolbox.prototype = { key.setAttribute("modifiers", toolDefinition.modifiers); key.setAttribute("oncommand", "void(0);"); // needed. See bug 371900 key.addEventListener("command", function(toolId) { - this.selectTool(toolId); + this.selectTool(toolId).then(() => { + this.fireCustomKey(toolId); + }); }.bind(this, id), true); doc.getElementById("toolbox-keyset").appendChild(key); } } }, + + /** + * Handle any custom key events. Returns true if there was a custom key binding run + * @param {string} toolId + * Which tool to run the command on (skip if not current) + */ + fireCustomKey: function TBOX_fireCustomKey(toolId) { + let tools = gDevTools.getToolDefinitionMap(); + let activeToolDefinition = tools.get(toolId); + + if (activeToolDefinition.onkey && this.currentToolId === toolId) { + activeToolDefinition.onkey(this.getCurrentPanel()); + } + }, + /** * Build the buttons for changing hosts. Called every time * the host changes. diff --git a/browser/devtools/inspector/highlighter.js b/browser/devtools/inspector/highlighter.js index f50f99081e6..02f07a1775e 100644 --- a/browser/devtools/inspector/highlighter.js +++ b/browser/devtools/inspector/highlighter.js @@ -129,8 +129,6 @@ Highlighter.prototype = { this.transitionDisabler = null; this.pageEventsMuter = null; - this.unlockAndFocus(); - this.selection.on("new-node", this.highlight); this.selection.on("new-node", this.updateInfobar); this.selection.on("pseudoclass", this.updateInfobar); diff --git a/browser/devtools/inspector/inspector-panel.js b/browser/devtools/inspector/inspector-panel.js index bd2565ab13f..ee446cacdfd 100644 --- a/browser/devtools/inspector/inspector-panel.js +++ b/browser/devtools/inspector/inspector-panel.js @@ -137,10 +137,6 @@ InspectorPanel.prototype = { // All the components are initialized. Let's select a node. this._selection.setNodeFront(defaultSelection); - if (this.highlighter) { - this.highlighter.unlock(); - } - this.markup.expandNode(this.selection.nodeFront); this.emit("ready"); diff --git a/browser/devtools/inspector/test/browser_inspector_bug_665880.js b/browser/devtools/inspector/test/browser_inspector_bug_665880.js index 6d990d5c8bb..58ddcabeeb6 100644 --- a/browser/devtools/inspector/test/browser_inspector_bug_665880.js +++ b/browser/devtools/inspector/test/browser_inspector_bug_665880.js @@ -29,6 +29,7 @@ function test() function runObjectInspectionTest(inspector) { inspector.highlighter.once("locked", performTestComparison); + inspector.highlighter.unlock(); inspector.selection.setNode(objectNode, ""); } diff --git a/browser/devtools/inspector/test/browser_inspector_bug_674871.js b/browser/devtools/inspector/test/browser_inspector_bug_674871.js index ed448d8f161..b4c0699879e 100644 --- a/browser/devtools/inspector/test/browser_inspector_bug_674871.js +++ b/browser/devtools/inspector/test/browser_inspector_bug_674871.js @@ -50,6 +50,7 @@ function test() function runTests(inspector) { + inspector.highlighter.unlock(); executeSoon(function() { inspector.highlighter.once("highlighting", isTheIframeSelected); moveMouseOver(iframeNode, 1, 1); diff --git a/browser/devtools/inspector/test/browser_inspector_iframeTest.js b/browser/devtools/inspector/test/browser_inspector_iframeTest.js index d95242a3daa..342028661ce 100644 --- a/browser/devtools/inspector/test/browser_inspector_iframeTest.js +++ b/browser/devtools/inspector/test/browser_inspector_iframeTest.js @@ -51,6 +51,7 @@ function moveMouseOver(aElement) function runIframeTests() { + getActiveInspector().highlighter.unlock(); getActiveInspector().selection.once("new-node", performTestComparisons1); moveMouseOver(div1) } diff --git a/browser/devtools/inspector/test/browser_inspector_scrolling.js b/browser/devtools/inspector/test/browser_inspector_scrolling.js index 9fa5ba43155..b2ef6e22601 100644 --- a/browser/devtools/inspector/test/browser_inspector_scrolling.js +++ b/browser/devtools/inspector/test/browser_inspector_scrolling.js @@ -35,6 +35,7 @@ function inspectNode(aInspector) inspector.highlighter.once("locked", performScrollingTest); executeSoon(function() { + inspector.highlighter.unlock(); inspector.selection.setNode(div, ""); }); } diff --git a/browser/devtools/main.js b/browser/devtools/main.js index 086d55079ac..55bfe74727a 100644 --- a/browser/devtools/main.js +++ b/browser/devtools/main.js @@ -97,6 +97,13 @@ Tools.inspector = { label: l10n("inspector.label", inspectorStrings), tooltip: l10n("inspector.tooltip", inspectorStrings), + preventClosingOnKey: true, + onkey: function(panel) { + if (panel.highlighter) { + panel.highlighter.toggleLockState(); + } + }, + isTargetSupported: function(target) { return !target.isRemote; }, diff --git a/browser/locales/en-US/chrome/browser/devtools/inspector.properties b/browser/locales/en-US/chrome/browser/devtools/inspector.properties index 692d73f0f51..cb46aafced0 100644 --- a/browser/locales/en-US/chrome/browser/devtools/inspector.properties +++ b/browser/locales/en-US/chrome/browser/devtools/inspector.properties @@ -35,7 +35,7 @@ nodeMenu.tooltiptext=Node operations # LOCALIZATION NOTE (inspector.*) # Used for the menuitem in the tool menu inspector.label=Inspector -inspector.commandkey=I +inspector.commandkey=C inspector.accesskey=I # LOCALIZATION NOTE (markupView.more.*)