Bug 1111599 - Shift-click while in picker mode selects the element but doesn't stop the picker; r?pbro draft
authorXue Fuqiao <xfq.free@gmail.com>
Tue, 18 Oct 2016 15:50:51 +0800
changeset 426234 fac050e70f2b7ebcd2eb3c45817fa244c8242acf
parent 422256 49fe455cac957808ed4a5d1685c3a1938dac1d31
child 534135 f9f47556138e8d1b5dca7d97a7fcd7d04399538c
push id32663
push userbmo:xfq.free@gmail.com
push dateTue, 18 Oct 2016 07:53:54 +0000
reviewerspbro
bugs1111599
milestone52.0a1
Bug 1111599 - Shift-click while in picker mode selects the element but doesn't stop the picker; r?pbro MozReview-Commit-ID: 2yUKBJylxF1
devtools/client/framework/toolbox-highlighter-utils.js
devtools/client/inspector/markup/markup.js
devtools/client/inspector/test/browser.ini
devtools/client/inspector/test/browser_inspector_highlighter-preview.js
devtools/server/actors/highlighters.js
devtools/shared/specs/inspector.js
--- a/devtools/client/framework/toolbox-highlighter-utils.js
+++ b/devtools/client/framework/toolbox-highlighter-utils.js
@@ -120,16 +120,17 @@ exports.getHighlighterUtils = function (
 
     toolbox.pickerButtonChecked = true;
     yield toolbox.selectTool("inspector");
     toolbox.on("select", stopPicker);
 
     if (isRemoteHighlightable()) {
       toolbox.walker.on("picker-node-hovered", onPickerNodeHovered);
       toolbox.walker.on("picker-node-picked", onPickerNodePicked);
+      toolbox.walker.on("picker-node-previewed", onPickerNodePreviewed);
       toolbox.walker.on("picker-node-canceled", onPickerNodeCanceled);
 
       yield toolbox.highlighter.pick();
       toolbox.emit("picker-started");
     } else {
       // If the target doesn't have the highlighter actor, we can use the
       // walker's pick method instead, knowing that it only responds when a node
       // is picked (instead of emitting events)
@@ -152,16 +153,17 @@ exports.getHighlighterUtils = function (
     isPicking = false;
 
     toolbox.pickerButtonChecked = false;
 
     if (isRemoteHighlightable()) {
       yield toolbox.highlighter.cancelPick();
       toolbox.walker.off("picker-node-hovered", onPickerNodeHovered);
       toolbox.walker.off("picker-node-picked", onPickerNodePicked);
+      toolbox.walker.off("picker-node-previewed", onPickerNodePreviewed);
       toolbox.walker.off("picker-node-canceled", onPickerNodeCanceled);
     } else {
       // If the target doesn't have the highlighter actor, use the walker's
       // cancelPick method instead
       yield toolbox.walker.cancelPick();
     }
 
     toolbox.off("select", stopPicker);
@@ -181,16 +183,25 @@ exports.getHighlighterUtils = function (
    * @param {Object} data Information about the picked node
    */
   function onPickerNodePicked(data) {
     toolbox.selection.setNodeFront(data.node, "picker-node-picked");
     stopPicker();
   }
 
   /**
+   * When a node has been shift-clicked (previewed) while the highlighter is in
+   * picker mode
+   * @param {Object} data Information about the picked node
+   */
+  function onPickerNodePreviewed(data) {
+    toolbox.selection.setNodeFront(data.node, "picker-node-previewed");
+  }
+
+  /**
    * When the picker is canceled, stop the picker, and make sure the toolbox
    * gets the focus.
    */
   function onPickerNodeCanceled() {
     stopPicker();
     toolbox.win.focus();
   }
 
--- a/devtools/client/inspector/markup/markup.js
+++ b/devtools/client/inspector/markup/markup.js
@@ -615,16 +615,18 @@ MarkupView.prototype = {
    */
   maybeNavigateToNewSelection: function () {
     let {reason, nodeFront} = this.inspector.selection;
 
     // The list of reasons that should lead to navigating to the node.
     let reasonsToNavigate = [
       // If the user picked an element with the element picker.
       "picker-node-picked",
+      // If the user shift-clicked (previewed) an element.
+      "picker-node-previewed",
       // If the user selected an element with the browser context menu.
       "browser-context-menu",
       // If the user added a new node by clicking in the inspector toolbar.
       "node-inserted"
     ];
 
     if (reasonsToNavigate.includes(reason)) {
       this.getContainer(this._rootNode).elt.focus();
--- a/devtools/client/inspector/test/browser.ini
+++ b/devtools/client/inspector/test/browser.ini
@@ -92,16 +92,17 @@ subsuite = clipboard
 [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-measure_01.js]
 [browser_inspector_highlighter-measure_02.js]
 [browser_inspector_highlighter-options.js]
+[browser_inspector_highlighter-preview.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-xbl.js]
 [browser_inspector_highlighter-zoom.js]
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/test/browser_inspector_highlighter-preview.js
@@ -0,0 +1,56 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+"use strict";
+
+// Test that the highlighter is correctly displayed and picker mode is not stopped after
+// a shift-click (preview)
+
+const TEST_URI = `data:text/html;charset=utf-8,
+                  <p id="one">one</p><p id="two">two</p><p id="three">three</p>`;
+
+add_task(function* () {
+  let {inspector, toolbox, testActor} = yield openInspectorForURL(TEST_URI);
+
+  let body = yield getNodeFront("body", inspector);
+  is(inspector.selection.nodeFront, body, "By default the body node is selected");
+
+  info("Start the element picker");
+  yield startPicker(toolbox);
+
+  info("Shift-clicking element #one should select it but keep the picker ON");
+  yield clickElement("#one", testActor, inspector, true);
+  yield checkElementSelected("#one", inspector);
+  checkPickerMode(toolbox, true);
+
+  info("Shift-clicking element #two should select it but keep the picker ON");
+  yield clickElement("#two", testActor, inspector, true);
+  yield checkElementSelected("#two", inspector);
+  checkPickerMode(toolbox, true);
+
+  info("Clicking element #three should select it and turn the picker OFF");
+  yield clickElement("#three", testActor, inspector, false);
+  yield checkElementSelected("#three", inspector);
+  checkPickerMode(toolbox, false);
+});
+
+function* clickElement(selector, testActor, inspector, isShift) {
+  let onSelectionChanged = inspector.once("inspector-updated");
+  yield testActor.synthesizeMouse({
+    selector: selector,
+    center: true,
+    options: { shiftKey: isShift }
+  });
+  yield onSelectionChanged;
+}
+
+function* checkElementSelected(selector, inspector) {
+  let el = yield getNodeFront(selector, inspector);
+  is(inspector.selection.nodeFront, el, `The element ${selector} is now selected`);
+}
+
+function checkPickerMode(toolbox, isOn) {
+  let pickerButton = toolbox.doc.querySelector("#command-button-pick");
+  is(pickerButton.hasAttribute("checked"), isOn, "The picker mode is correct");
+}
--- a/devtools/server/actors/highlighters.js
+++ b/devtools/server/actors/highlighters.js
@@ -240,16 +240,23 @@ var HighlighterActor = exports.Highlight
 
     this._onPick = event => {
       this._preventContentEvent(event);
 
       if (!this._isEventAllowed(event)) {
         return;
       }
 
+      // If shift is pressed, this is only a preview click, send the event to
+      // the client, but don't stop picking.
+      if (event.shiftKey) {
+        events.emit(this._walker, "picker-node-previewed", this._findAndAttachElement(event));
+        return;
+      }
+
       this._stopPickerListeners();
       this._isPicking = false;
       if (this._autohide) {
         this._tabActor.window.setTimeout(() => {
           this._highlighter.hide();
         }, HIGHLIGHTER_PICKED_TIMER);
       }
       if (!this._currentNode) {
--- a/devtools/shared/specs/inspector.js
+++ b/devtools/shared/specs/inspector.js
@@ -100,16 +100,20 @@ const walkerSpec = generateActorSpec({
   events: {
     "new-mutations": {
       type: "newMutations"
     },
     "picker-node-picked": {
       type: "pickerNodePicked",
       node: Arg(0, "disconnectedNode")
     },
+    "picker-node-previewed": {
+      type: "pickerNodePreviewed",
+      node: Arg(0, "disconnectedNode")
+    },
     "picker-node-hovered": {
       type: "pickerNodeHovered",
       node: Arg(0, "disconnectedNode")
     },
     "picker-node-canceled": {
       type: "pickerNodeCanceled"
     },
     "highlighter-ready": {