Bug 1351081 - Always retrieve quads relatively to the top window, correctly; r=gl draft
authorPatrick Brosset <pbrosset@mozilla.com>
Fri, 30 Mar 2018 11:24:45 +0200
changeset 777309 6406571849afb1d3dcec176f68ef4d3d122a1abf
parent 777163 ff0efa4132f0efd78af0910762aec7dcc1a8de66
push id105162
push userbmo:pbrosset@mozilla.com
push dateWed, 04 Apr 2018 16:10:34 +0000
reviewersgl
bugs1351081
milestone61.0a1
Bug 1351081 - Always retrieve quads relatively to the top window, correctly; r=gl Use the getBoxQuads's relativeTo option to avoid having to calculate the offset due to frames. Also reduce the precision of numbers used when checking if the highlighter is correctly displayed. Finally, for some strange reasons, this patch seems to cause a totally unrelated events mutation event to be sent during the test. This polutes the mutations received and made the test fail. So I filtered the list of mutations to only preserve the ones we care about here. I could not reproduce this extra mutation when running Firefox. Only during the test. So I did not investigate further. MozReview-Commit-ID: 1ZQ6FGULjHG
devtools/client/inspector/test/browser_inspector_addNode_03.js
devtools/client/shared/test/test-actor.js
devtools/server/actors/highlighters/auto-refresh.js
devtools/shared/layout/utils.js
--- a/devtools/client/inspector/test/browser_inspector_addNode_03.js
+++ b/devtools/client/inspector/test/browser_inspector_addNode_03.js
@@ -45,16 +45,20 @@ async function testAddNode(parentNode, i
 
   info("Clicking 'add node' and expecting a markup mutation and a new container");
   let onMutation = inspector.once("markupmutation");
   let onNewContainer = inspector.once("container-created");
   btn.click();
   let mutations = await onMutation;
   await onNewContainer;
 
+  // We are only interested in childList mutations here. Filter everything else out as
+  // there may be unrelated mutations (e.g. "events") grouped in.
+  mutations = mutations.filter(({ type }) => type === "childList");
+
   is(mutations.length, 1, "There is one mutation only");
   is(mutations[0].added.length, 1, "There is one new node only");
 
   let newNode = mutations[0].added[0];
 
   is(parentNode, inspector.selection.nodeFront, "The parent node is still selected");
   is(newNode.parentNode(), parentNode, "The new node is inside the right parent");
 
--- a/devtools/client/shared/test/test-actor.js
+++ b/devtools/client/shared/test/test-actor.js
@@ -1097,16 +1097,21 @@ var TestActorFront = exports.TestActorFr
  * @param {Array} polygon An array of [x,y] points
  * @return {Boolean}
  */
 function isInside(point, polygon) {
   if (polygon.length === 0) {
     return false;
   }
 
+  // Reduce the length of the fractional part because this is likely to cause errors when
+  // the point is on the edge of the polygon.
+  point = point.map(n => n.toFixed(2));
+  polygon = polygon.map(p => p.map(n => n.toFixed(2)));
+
   const n = polygon.length;
   const newPoints = polygon.slice(0);
   newPoints.push(polygon[0]);
   let wn = 0;
 
   // loop through all edges of the polygon
   for (let i = 0; i < n; i++) {
     // Accept points on the edges
--- a/devtools/server/actors/highlighters/auto-refresh.js
+++ b/devtools/server/actors/highlighters/auto-refresh.js
@@ -7,38 +7,39 @@
 const { Cu } = require("chrome");
 const EventEmitter = require("devtools/shared/event-emitter");
 const { isNodeValid } = require("./utils/markup");
 const { getAdjustedQuads, getWindowDimensions } = require("devtools/shared/layout/utils");
 
 // Note that the order of items in this array is important because it is used
 // for drawing the BoxModelHighlighter's path elements correctly.
 const BOX_MODEL_REGIONS = ["margin", "border", "padding", "content"];
-const QUADS_PROPS = ["p1", "p2", "p3", "p4", "bounds"];
+const QUADS_PROPS = ["p1", "p2", "p3", "p4"];
 
-function areValuesDifferent(oldValue, newValue) {
-  let delta = Math.abs(oldValue.toFixed(4) - newValue.toFixed(4));
-  return delta >= .5;
+function arePointsDifferent(pointA, pointB) {
+  return (Math.abs(pointA.x - pointB.x) >= .5) ||
+         (Math.abs(pointA.y - pointB.y) >= .5) ||
+         (Math.abs(pointA.w - pointB.w) >= .5);
 }
 
 function areQuadsDifferent(oldQuads, newQuads) {
   for (let region of BOX_MODEL_REGIONS) {
-    if (oldQuads[region].length !== newQuads[region].length) {
+    let { length } = oldQuads[region];
+
+    if (length !== newQuads[region].length) {
       return true;
     }
 
-    for (let i = 0; i < oldQuads[region].length; i++) {
+    for (let i = 0; i < length; i++) {
       for (let prop of QUADS_PROPS) {
-        let oldProp = oldQuads[region][i][prop];
-        let newProp = newQuads[region][i][prop];
+        let oldPoint = oldQuads[region][i][prop];
+        let newPoint = newQuads[region][i][prop];
 
-        for (let key of Object.keys(oldProp)) {
-          if (areValuesDifferent(oldProp[key], newProp[key])) {
-            return true;
-          }
+        if (arePointsDifferent(oldPoint, newPoint)) {
+          return true;
         }
       }
     }
   }
 
   return false;
 }
 
--- a/devtools/shared/layout/utils.js
+++ b/devtools/shared/layout/utils.js
@@ -195,29 +195,29 @@ exports.getFrameOffsets = getFrameOffset
  *        getBoxQuads. An empty array if the node has no quads or is invalid.
  */
 function getAdjustedQuads(boundaryWindow, node, region) {
   if (!node || !node.getBoxQuads) {
     return [];
   }
 
   let quads = node.getBoxQuads({
-    box: region
+    box: region,
+    relativeTo: boundaryWindow.document
   });
 
   if (!quads.length) {
     return [];
   }
 
-  let [xOffset, yOffset] = getFrameOffsets(boundaryWindow, node);
   let scale = getCurrentZoom(node);
   let { scrollX, scrollY } = boundaryWindow;
 
-  xOffset += scrollX * scale;
-  yOffset += scrollY * scale;
+  let xOffset = scrollX * scale;
+  let yOffset = scrollY * scale;
 
   let adjustedQuads = [];
   for (let quad of quads) {
     adjustedQuads.push({
       p1: {
         w: quad.p1.w * scale,
         x: quad.p1.x * scale + xOffset,
         y: quad.p1.y * scale + yOffset,