Bug 1383870 - CSS shapes highlighter incorrectly handles mouse events on elements in iframe. r=gl
☠☠ backed out by d55c69736b4e ☠ ☠
authorMike Park <mikeparkms@gmail.com>
Tue, 25 Jul 2017 10:33:51 -0400
changeset 385468 3f196fc740bec0ce834244cfb7503e198c8b494f
parent 385467 901a16fec9fc0c875ed274a37efbd379682e62e9
child 385469 d55c69736b4e9690940a764d946eabe6dacacc4d
push id96016
push usergabriel.luong@gmail.com
push dateWed, 11 Oct 2017 04:29:35 +0000
treeherdermozilla-inbound@3f196fc740be [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1383870
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1383870 - CSS shapes highlighter incorrectly handles mouse events on elements in iframe. r=gl MozReview-Commit-ID: HAHop7hSPxs
devtools/client/inspector/rules/views/text-property-editor.js
devtools/client/inspector/test/browser_inspector_highlighter-cssshape_05.js
devtools/server/actors/highlighters.js
devtools/server/actors/highlighters/shapes.js
--- a/devtools/client/inspector/rules/views/text-property-editor.js
+++ b/devtools/client/inspector/rules/views/text-property-editor.js
@@ -969,31 +969,37 @@ TextPropertyEditor.prototype = {
     }
 
     let view = this.ruleView;
     let { highlighters } = view;
     let ruleViewEl = view.element;
     let selector = `.ruleview-shape-point.active`;
     for (let pointNode of ruleViewEl.querySelectorAll(selector)) {
       this._toggleShapePointActive(pointNode, false);
+      // Emit event for unit tests when point is hovered on highlighter
+      // and same point is highlighted in rule view
+      this.ruleView.emit("ruleview-shape-point-highlight");
     }
 
     if (typeof point === "string") {
       if (point.includes(",")) {
         point = point.split(",")[0];
       }
       // Because one inset value can represent multiple points, inset points use classes
       // instead of data.
       selector = (INSET_POINT_TYPES.includes(point)) ?
                  `.ruleview-shape-point.${point}` :
                  `.ruleview-shape-point[data-point='${point}']`;
       for (let pointNode of this.valueSpan.querySelectorAll(selector)) {
         let nodeInfo = view.getNodeInfo(pointNode);
         if (highlighters.isRuleViewShapePoint(nodeInfo)) {
           this._toggleShapePointActive(pointNode, true);
+          // Emit event for unit tests when point is hovered on highlighter
+          // and same point is highlighted in rule view
+          this.ruleView.emit("ruleview-shape-point-highlight");
         }
       }
     }
   },
 
   /**
    * Toggle the class "active" on the given shape point in the rule view if the current
    * inspector selection is highlighted by the shapes highlighter.
--- a/devtools/client/inspector/test/browser_inspector_highlighter-cssshape_05.js
+++ b/devtools/client/inspector/test/browser_inspector_highlighter-cssshape_05.js
@@ -67,17 +67,17 @@ function* highlightFromRuleView(inspecto
 function* highlightFromHighlighter(view, highlighters, testActor, helper) {
   let highlighterFront = highlighters.highlighters[HIGHLIGHTER_TYPE];
   let { mouse } = helper;
 
   yield toggleShapesHighlighter(view, highlighters, "#polygon", "clip-path", true);
   let container = getRuleViewProperty(view, "#polygon", "clip-path").valueSpan;
 
   info("Hover over first point in highlighter");
-  let onEventHandled = highlighters.once("highlighter-event-handled");
+  let onEventHandled = view.once("ruleview-shape-point-highlight");
   yield mouse.move(0, 0);
   yield onEventHandled;
   let markerHidden = yield testActor.getHighlighterNodeAttribute(
     "shapes-marker-hover", "hidden", highlighterFront);
   ok(!markerHidden, "Marker on highlighter is visible");
 
   let pointSpan = container.querySelector(".ruleview-shape-point[data-point='0']");
   ok(pointSpan.classList.contains("active"), "Span for point 0 is active");
@@ -94,17 +94,17 @@ function* highlightFromHighlighter(view,
   container = getRuleViewProperty(view, "element", "clip-path").valueSpan;
   pointSpan = container.querySelector(".ruleview-shape-point[data-point='0']");
   ok(pointSpan.classList.contains("active"),
      "Span for point 0 is active after moving point");
   is(highlighters.state.shapes.hoverPoint, "0",
      "Hovered point is saved to state after moving point");
 
   info("Move mouse off point");
-  onEventHandled = highlighters.once("highlighter-event-handled");
+  onEventHandled = view.once("ruleview-shape-point-highlight");
   yield mouse.move(100, 100);
   yield onEventHandled;
   markerHidden = yield testActor.getHighlighterNodeAttribute(
     "shapes-marker-hover", "hidden", highlighterFront);
   ok(markerHidden, "Marker on highlighter is no longer visible");
   ok(!pointSpan.classList.contains("active"), "Span for point 0 is no longer active");
   is(highlighters.state.shapes.hoverPoint, null, "Hovered point is null");
 }
--- a/devtools/server/actors/highlighters.js
+++ b/devtools/server/actors/highlighters.js
@@ -510,17 +510,17 @@ exports.CustomHighlighterActor = protoco
     if (this._highlighter) {
       this._highlighter.hide();
     }
   },
 
   /**
    * Upon receiving an event from the highlighter, forward it to the client.
    */
-  _onHighlighterEvent: function (type, data) {
+  _onHighlighterEvent: function (data) {
     this.emit("highlighter-event", data);
   },
 
   /**
    * Kill this actor. This method is called automatically just before the actor
    * is destroyed.
    */
   finalize: function () {
--- a/devtools/server/actors/highlighters/shapes.js
+++ b/devtools/server/actors/highlighters/shapes.js
@@ -1,17 +1,18 @@
 /* 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";
 
 const { CanvasFrameAnonymousContentHelper, getCSSStyleRules,
         createSVGNode, createNode, getComputedStyle } = require("./utils/markup");
-const { setIgnoreLayoutChanges, getCurrentZoom } = require("devtools/shared/layout/utils");
+const { setIgnoreLayoutChanges, getCurrentZoom,
+        getAdjustedQuads } = require("devtools/shared/layout/utils");
 const { AutoRefreshHighlighter } = require("./auto-refresh");
 const {
   getDistance,
   clickedOnEllipseEdge,
   distanceToLine,
   projection,
   clickedOnPoint
 } = require("devtools/server/actors/utils/shapes-geometry-utils");
@@ -171,16 +172,37 @@ class ShapesHighlighter extends AutoRefr
     return {
       top: top / zoom,
       left: left / zoom,
       width: width / zoom,
       height: height / zoom
     };
   }
 
+  get frameDimensions() {
+    // In an iframe, we get the node's quads relative to the frame,
+    // instead of the parent document.
+    let dims = getAdjustedQuads(this.currentNode.ownerGlobal,
+      this.currentNode, this.referenceBox)[0].bounds;
+    let zoom = getCurrentZoom(this.win);
+
+    if (this.currentNode.getBBox &&
+        getComputedStyle(this.currentNode).stroke !== "none" && !this.useStrokeBox) {
+      dims = getObjectBoundingBox(dims.top, dims.left,
+        dims.width, dims.height, this.currentNode);
+    }
+
+    return {
+      top: dims.top / zoom,
+      left: dims.left / zoom,
+      width: dims.width / zoom,
+      height: dims.height / zoom
+    };
+  }
+
   handleEvent(event, id) {
     // No event handling if the highlighter is hidden
     if (this.areShapesHidden()) {
       return;
     }
 
     const { target, type, pageX, pageY } = event;
 
@@ -697,17 +719,20 @@ class ShapesHighlighter extends AutoRefr
    * Convert the given coordinates on the page to percentages relative to the current
    * element.
    * @param {Number} pageX the x coordinate on the page
    * @param {Number} pageY the y coordinate on the page
    * @returns {Object} object of form {percentX, percentY}, which are the x/y coords
    *          in percentages relative to the element.
    */
   convertPageCoordsToPercent(pageX, pageY) {
-    let { top, left, width, height } = this.zoomAdjustedDimensions;
+    // If the current node is in an iframe, we get dimensions relative to the frame.
+    let dims = (this.highlighterEnv.window.document === this.currentNode.ownerDocument) ?
+               this.zoomAdjustedDimensions : this.frameDimensions;
+    let { top, left, width, height } = dims;
     pageX -= left;
     pageY -= top;
     let percentX = pageX * 100 / width;
     let percentY = pageY * 100 / height;
     return { percentX, percentY };
   }
 
   /**