Bug 1289433 - Fix Eye dropper focus to support key shortcuts on it when opening from the inspector. r=jdescottes, a=ritu
authorAlexandre Poirot <poirot.alex@gmail.com>
Thu, 11 Aug 2016 04:14:36 -0700
changeset 350052 b7da20bef48fa32a43091d2095226f591e150629
parent 350051 0a787a252077640330496045bdde5ca018ccc727
child 350053 3b7c8b6aa43a6464d6794286bb3e7b887cec5c81
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdescottes, ritu
bugs1289433
milestone50.0a2
Bug 1289433 - Fix Eye dropper focus to support key shortcuts on it when opening from the inspector. r=jdescottes, a=ritu
devtools/client/inspector/test/browser_inspector_highlighter-eyedropper-clipboard.js
devtools/client/inspector/test/browser_inspector_highlighter-eyedropper-events.js
devtools/client/inspector/test/head.js
devtools/server/actors/highlighters/eye-dropper.js
--- a/devtools/client/inspector/test/browser_inspector_highlighter-eyedropper-clipboard.js
+++ b/devtools/client/inspector/test/browser_inspector_highlighter-eyedropper-clipboard.js
@@ -9,29 +9,28 @@ const HIGHLIGHTER_TYPE = "EyeDropper";
 const ID = "eye-dropper-";
 const TEST_URI = "data:text/html;charset=utf-8,<style>html{background:red}</style>";
 
 add_task(function* () {
   let helper = yield openInspectorForURL(TEST_URI)
                .then(getHighlighterHelperFor(HIGHLIGHTER_TYPE));
   helper.prefix = ID;
 
-  let {show, synthesizeKey, finalize,
+  let {show, finalize,
        waitForElementAttributeSet, waitForElementAttributeRemoved} = helper;
 
   info("Show the eyedropper with the copyOnSelect option");
   yield show("html", {copyOnSelect: true});
 
   info("Make sure to wait until the eyedropper is done taking a screenshot of the page");
   yield waitForElementAttributeSet("root", "drawn", helper);
 
   yield waitForClipboard(() => {
     info("Activate the eyedropper so the background color is copied");
-    let generateKey = synthesizeKey({key: "VK_RETURN", options: {}});
-    generateKey.next();
+    EventUtils.synthesizeKey("VK_RETURN", {});
   }, "#FF0000");
 
   ok(true, "The clipboard contains the right value");
 
   yield waitForElementAttributeRemoved("root", "drawn", helper);
   yield waitForElementAttributeSet("root", "hidden", helper);
   ok(true, "The eyedropper is now hidden");
 
--- a/devtools/client/inspector/test/browser_inspector_highlighter-eyedropper-events.js
+++ b/devtools/client/inspector/test/browser_inspector_highlighter-eyedropper-events.js
@@ -30,42 +30,42 @@ add_task(function* () {
   yield respondsToMoveEvents(helper);
   yield respondsToReturnAndEscape(helper);
 
   helper.finalize();
 });
 
 function* respondsToMoveEvents(helper) {
   info("Checking that the eyedropper responds to events from the mouse and keyboard");
-  let {mouse, synthesizeKey} = helper;
+  let {mouse} = helper;
 
   for (let {type, x, y, key, shift, expected} of MOVE_EVENTS_DATA) {
     info(`Simulating a ${type} event to move to ${expected.x} ${expected.y}`);
     if (type === "mouse") {
       yield mouse.move(x, y);
     } else if (type === "keyboard") {
       let options = shift ? {shiftKey: true} : {};
-      yield synthesizeKey({key, options});
+      yield EventUtils.synthesizeKey(key, options);
     }
     yield checkPosition(expected, helper);
   }
 }
 
 function* checkPosition({x, y}, {getElementAttribute}) {
   let style = yield getElementAttribute("root", "style");
   is(style, `top:${y}px;left:${x}px;`,
      `The eyedropper is at the expected ${x} ${y} position`);
 }
 
-function* respondsToReturnAndEscape({synthesizeKey, isElementHidden, show}) {
+function* respondsToReturnAndEscape({isElementHidden, show}) {
   info("Simulating return to select the color and hide the eyedropper");
 
-  yield synthesizeKey({key: "VK_RETURN", options: {}});
+  yield EventUtils.synthesizeKey("VK_RETURN", {});
   let hidden = yield isElementHidden("root");
   ok(hidden, "The eyedropper has been hidden");
 
   info("Showing the eyedropper again and simulating escape to hide it");
 
   yield show("html");
-  yield synthesizeKey({key: "VK_ESCAPE", options: {}});
+  yield EventUtils.synthesizeKey("VK_ESCAPE", {});
   hidden = yield isElementHidden("root");
   ok(hidden, "The eyedropper has been hidden again");
 }
--- a/devtools/client/inspector/test/head.js
+++ b/devtools/client/inspector/test/head.js
@@ -498,20 +498,16 @@ const getHighlighterHelperFor = (type) =
         }, `Waiting for element ${id} to have attribute ${name} removed`);
       },
 
       synthesizeMouse: function* (options) {
         options = Object.assign({selector: ":root"}, options);
         yield testActor.synthesizeMouse(options);
       },
 
-      synthesizeKey: function* (options) {
-        yield testActor.synthesizeKey(options);
-      },
-
       // This object will synthesize any "mouse" prefixed event to the
       // `testActor`, using the name of method called as suffix for the
       // event's name.
       // If no x, y coords are given, the previous ones are used.
       //
       // For example:
       //   mouse.down(10, 20); // synthesize "mousedown" at 10,20
       //   mouse.move(20, 30); // synthesize "mousemove" at 20,30
--- a/devtools/server/actors/highlighters/eye-dropper.js
+++ b/devtools/server/actors/highlighters/eye-dropper.js
@@ -153,17 +153,17 @@ EyeDropper.prototype = {
     this.ctx.mozImageSmoothingEnabled = false;
 
     this.magnifiedArea = {width: MAGNIFIER_WIDTH, height: MAGNIFIER_HEIGHT,
                           x: DEFAULT_START_POS_X, y: DEFAULT_START_POS_Y};
 
     this.moveTo(DEFAULT_START_POS_X, DEFAULT_START_POS_Y);
 
     // Focus the content so the keyboard can be used.
-    this.win.document.documentElement.focus();
+    this.win.focus();
 
     return true;
   },
 
   /**
    * Hide the eye-dropper highlighter.
    */
   hide() {
@@ -375,64 +375,65 @@ EyeDropper.prototype = {
     onColorSelected.then(() => this.hide(), e => console.error(e));
   },
 
   /**
    * Handler for the keydown event. Either select the color or move the panel in a
    * direction depending on the key pressed.
    */
   handleKeyDown(e) {
+    // Bail out early if any unsupported modifier is used, so that we let
+    // keyboard shortcuts through.
+    if (e.metaKey || e.ctrlKey || e.altKey) {
+      return;
+    }
+
     if (e.keyCode === e.DOM_VK_RETURN) {
       this.selectColor();
+      e.preventDefault();
       return;
     }
 
     if (e.keyCode === e.DOM_VK_ESCAPE) {
       this.emit("canceled");
       this.hide();
+      e.preventDefault();
       return;
     }
 
     let offsetX = 0;
     let offsetY = 0;
     let modifier = 1;
 
     if (e.keyCode === e.DOM_VK_LEFT) {
       offsetX = -1;
-    }
-    if (e.keyCode === e.DOM_VK_RIGHT) {
+    } else if (e.keyCode === e.DOM_VK_RIGHT) {
       offsetX = 1;
-    }
-    if (e.keyCode === e.DOM_VK_UP) {
+    } else if (e.keyCode === e.DOM_VK_UP) {
       offsetY = -1;
-    }
-    if (e.keyCode === e.DOM_VK_DOWN) {
+    } else if (e.keyCode === e.DOM_VK_DOWN) {
       offsetY = 1;
     }
+
     if (e.shiftKey) {
       modifier = 10;
     }
 
     offsetY *= modifier;
     offsetX *= modifier;
 
     if (offsetX !== 0 || offsetY !== 0) {
       this.magnifiedArea.x += offsetX;
       this.magnifiedArea.y += offsetY;
 
       this.draw();
 
       this.moveTo(this.magnifiedArea.x / this.pageZoom,
                   this.magnifiedArea.y / this.pageZoom);
-    }
 
-    // Prevent all keyboard interaction with the page, except if a modifier is used to let
-    // keyboard shortcuts through.
-    let hasModifier = e.metaKey || e.ctrlKey || e.altKey || e.shiftKey;
-    if (!hasModifier) {
       e.preventDefault();
     }
   },
 
   /**
    * Copy the currently inspected color to the clipboard.
    * @return {Promise} Resolves when the copy has been done (after a delay that is used to
    * let users know that something was copied).