Bug 988278 - Fixes ESCape keypress mess in the inspector to make sure the split console opens; r=miker
authorPatrick Brosset <pbrosset@mozilla.com>
Mon, 13 Apr 2015 10:51:49 +0200
changeset 257719 67683aaa2041b08f074bc6a2ec5a439d9969f052
parent 257718 5ac6a0ec6274c900f023f54249d5c08142b2ce8d
child 257720 5109c1949c7a5418d571aa9df80d82c179279cc9
push id8007
push userraliiev@mozilla.com
push dateMon, 11 May 2015 19:23:16 +0000
treeherdermozilla-aurora@e2ce1aac996e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiker
bugs988278
milestone40.0a1
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.
browser/devtools/framework/toolbox-highlighter-utils.js
browser/devtools/inspector/test/browser.ini
browser/devtools/inspector/test/browser_inspector_highlighter-keybinding_04.js
browser/devtools/markupview/test/browser.ini
browser/devtools/markupview/test/browser_markupview_keybindings_02.js
browser/devtools/shared/widgets/Tooltip.js
--- a/browser/devtools/framework/toolbox-highlighter-utils.js
+++ b/browser/devtools/framework/toolbox-highlighter-utils.js
@@ -176,20 +176,22 @@ exports.getHighlighterUtils = function(t
    * @param {Object} data Information about the picked node
    */
   function onPickerNodePicked(data) {
     toolbox.selection.setNodeFront(data.node, "picker-node-picked");
     stopPicker();
   }
 
   /**
-   * 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();
   }
 
   /**
    * Show the box model highlighter on a node in the content page.
    * The node needs to be a NodeFront, as defined by the inspector actor
    * @see toolkit/devtools/server/actors/inspector.js
    * @param {NodeFront} nodeFront The node to highlight
    * @param {Object} options
--- a/browser/devtools/inspector/test/browser.ini
+++ b/browser/devtools/inspector/test/browser.ini
@@ -53,16 +53,17 @@ skip-if = e10s # GCLI isn't e10s compati
 [browser_inspector_highlighter-hover_01.js]
 [browser_inspector_highlighter-hover_02.js]
 [browser_inspector_highlighter-hover_03.js]
 [browser_inspector_highlighter-iframes.js]
 [browser_inspector_highlighter-inline.js]
 [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]
 [browser_inspector_highlighter-rulers_01.js]
 [browser_inspector_highlighter-rulers_02.js]
 [browser_inspector_highlighter-selector_01.js]
 [browser_inspector_highlighter-selector_02.js]
 [browser_inspector_highlighter-zoom.js]
new file mode 100644
--- /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,<div></div>";
+
+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();
+});
--- a/browser/devtools/markupview/test/browser.ini
+++ b/browser/devtools/markupview/test/browser.ini
@@ -68,16 +68,17 @@ skip-if = e10s # Bug 1040751 - CodeMirro
 skip-if = e10s # Bug 1040751 - CodeMirror editor.destroy() isn't e10s compatible
 [browser_markupview_events_jquery_2.1.1.js]
 skip-if = e10s # Bug 1040751 - CodeMirror editor.destroy() isn't e10s compatible
 [browser_markupview_html_edit_01.js]
 [browser_markupview_html_edit_02.js]
 [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]
 [browser_markupview_node_not_displayed_01.js]
 [browser_markupview_node_not_displayed_02.js]
 [browser_markupview_pagesize_01.js]
 [browser_markupview_pagesize_02.js]
 [browser_markupview_remove_xul_attributes.js]
new file mode 100644
--- /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,<div></div>";
+
+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.");
+});
--- a/browser/devtools/shared/widgets/Tooltip.js
+++ b/browser/devtools/shared/widgets/Tooltip.js
@@ -207,17 +207,18 @@ function Tooltip(doc, options) {
   // Listen to keypress events to close the tooltip if configured to do so
   let win = this.doc.querySelector("window");
   this._onKeyPress = event => {
     if (this.panel.hidden) {
       return;
     }
 
     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();
     }
   };
   win.addEventListener("keypress", this._onKeyPress, false);
 
   // Listen to custom emitters' events to close the tooltip
   this.hide = this.hide.bind(this);