From fd53c00637caba91f73781f3c22abf559a52be93 Mon Sep 17 00:00:00 2001 From: Patrick Brosset Date: Mon, 13 Apr 2015 10:51:49 +0200 Subject: [PATCH] Bug 988278 - Fixes ESCape keypress mess in the inspector to make sure the split console opens; r=miker This fixes 2 problems related to the split console not opening when it should. 1 - After the inspector selection mode (pick mode) was canceled with ESC, pressing ESC once again did not open the split console, because the toolbox did not have the focus. 2 - The markup-view tooltip (used to preview images) was eating the first ESC keypress when the markup-view was focused, even though the tooltip was hidden. This was forcing users to press ESC twice to open the split console. --HG-- extra : rebase_source : 503e6f0a8fa2ef153f714af1361e85ad844a49d2 --- .../framework/toolbox-highlighter-utils.js | 4 +- browser/devtools/inspector/test/browser.ini | 1 + ...ser_inspector_highlighter-keybinding_04.js | 45 +++++++++++++++++++ browser/devtools/markupview/test/browser.ini | 1 + .../test/browser_markupview_keybindings_02.js | 30 +++++++++++++ browser/devtools/shared/widgets/Tooltip.js | 3 +- 6 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 browser/devtools/inspector/test/browser_inspector_highlighter-keybinding_04.js create mode 100644 browser/devtools/markupview/test/browser_markupview_keybindings_02.js diff --git a/browser/devtools/framework/toolbox-highlighter-utils.js b/browser/devtools/framework/toolbox-highlighter-utils.js index ac3435c8f3e4..e8f8faa17ad2 100644 --- a/browser/devtools/framework/toolbox-highlighter-utils.js +++ b/browser/devtools/framework/toolbox-highlighter-utils.js @@ -181,10 +181,12 @@ exports.getHighlighterUtils = function(toolbox) { } /** - * When the picker is canceled + * When the picker is canceled, stop the picker, and make sure the toolbox + * gets the focus. */ function onPickerNodeCanceled() { stopPicker(); + toolbox.frame.focus(); } /** diff --git a/browser/devtools/inspector/test/browser.ini b/browser/devtools/inspector/test/browser.ini index 0fa2e8b8639a..2e0d4f8ebff4 100644 --- a/browser/devtools/inspector/test/browser.ini +++ b/browser/devtools/inspector/test/browser.ini @@ -58,6 +58,7 @@ skip-if = e10s # GCLI isn't e10s compatible. See bug 1128988. [browser_inspector_highlighter-keybinding_01.js] [browser_inspector_highlighter-keybinding_02.js] [browser_inspector_highlighter-keybinding_03.js] +[browser_inspector_highlighter-keybinding_04.js] [browser_inspector_highlighter-options.js] [browser_inspector_highlighter-rect_01.js] [browser_inspector_highlighter-rect_02.js] diff --git a/browser/devtools/inspector/test/browser_inspector_highlighter-keybinding_04.js b/browser/devtools/inspector/test/browser_inspector_highlighter-keybinding_04.js new file mode 100644 index 000000000000..f6155779d3c7 --- /dev/null +++ b/browser/devtools/inspector/test/browser_inspector_highlighter-keybinding_04.js @@ -0,0 +1,45 @@ +/* vim: set 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 pressing ESC twice while in picker mode first stops the picker and +// then opens the split-console (see bug 988278). + +const TEST_URL = "data:text/html;charset=utf8,
"; + +add_task(function*() { + let {inspector, toolbox} = yield openInspectorForURL(TEST_URL); + + info("Start the element picker"); + yield toolbox.highlighterUtils.startPicker(); + + info("Start using the picker by hovering over nodes"); + let onHover = toolbox.once("picker-node-hovered"); + executeInContent("Test:SynthesizeMouse", { + options: {type: "mousemove"}, + center: true, + selector: "div" + }, null, false); + yield onHover; + + info("Press escape and wait for the picker to stop"); + let onPickerStopped = toolbox.once("picker-stopped"); + executeInContent("Test:SynthesizeKey", { + key: "VK_ESCAPE", + options: {} + }, null, false); + yield onPickerStopped; + + info("Press escape again and wait for the split console to open"); + let onSplitConsole = toolbox.once("split-console"); + // The escape key is synthesized in the main process, which is where the focus + // should be after the picker was stopped. + EventUtils.synthesizeKey("VK_ESCAPE", {}); + yield onSplitConsole; + ok(toolbox.splitConsole, "The split console is shown."); + + // Hide the split console. + yield toolbox.toggleSplitConsole(); +}); diff --git a/browser/devtools/markupview/test/browser.ini b/browser/devtools/markupview/test/browser.ini index c0f52351c6be..1f0f182d29f3 100644 --- a/browser/devtools/markupview/test/browser.ini +++ b/browser/devtools/markupview/test/browser.ini @@ -73,6 +73,7 @@ skip-if = e10s # Bug 1040751 - CodeMirror editor.destroy() isn't e10s compatible [browser_markupview_html_edit_03.js] [browser_markupview_image_tooltip.js] [browser_markupview_keybindings_01.js] +[browser_markupview_keybindings_02.js] [browser_markupview_mutation_01.js] [browser_markupview_mutation_02.js] [browser_markupview_navigation.js] diff --git a/browser/devtools/markupview/test/browser_markupview_keybindings_02.js b/browser/devtools/markupview/test/browser_markupview_keybindings_02.js new file mode 100644 index 000000000000..dab1910a265d --- /dev/null +++ b/browser/devtools/markupview/test/browser_markupview_keybindings_02.js @@ -0,0 +1,30 @@ +/* vim: set 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 pressing ESC when a node in the markup-view is focused toggles +// the split-console (see bug 988278) + +const TEST_URL = "data:text/html;charset=utf8,
"; + +add_task(function*() { + let {inspector, toolbox} = yield addTab(TEST_URL).then(openInspector); + + info("Focusing the tag editor of the test element"); + let {editor} = yield getContainerForSelector("div", inspector); + editor.tag.focus(); + + info("Pressing ESC and wait for the split-console to open"); + let onSplitConsole = toolbox.once("split-console"); + EventUtils.synthesizeKey("VK_ESCAPE", {}, inspector.panelWin); + yield onSplitConsole; + ok(toolbox.splitConsole, "The split console is shown."); + + info("Pressing ESC again and wait for the split-console to close"); + onSplitConsole = toolbox.once("split-console"); + EventUtils.synthesizeKey("VK_ESCAPE", {}, inspector.panelWin); + yield onSplitConsole; + ok(!toolbox.splitConsole, "The split console is hidden."); +}); diff --git a/browser/devtools/shared/widgets/Tooltip.js b/browser/devtools/shared/widgets/Tooltip.js index 0891376ceba6..bc58325e73c1 100644 --- a/browser/devtools/shared/widgets/Tooltip.js +++ b/browser/devtools/shared/widgets/Tooltip.js @@ -212,7 +212,8 @@ function Tooltip(doc, options) { } this.emit("keypress", event.keyCode); - if (this.options.get("closeOnKeys").indexOf(event.keyCode) !== -1) { + if (this.options.get("closeOnKeys").indexOf(event.keyCode) !== -1 && + this.isShown()) { event.stopPropagation(); this.hide(); }