Bug 1351081 - Always retrieve quads relatively to the top window, correctly; r=gl
authorPatrick Brosset <pbrosset@mozilla.com>
Fri, 30 Mar 2018 11:24:45 +0200
changeset 777811 65e0fefdab510230b90a367f0204852553e03605
parent 777810 a4a0483a84d9e52c8d49417af24588b62575de79
child 777822 93fda7f5d1f8e66663e8adcd36cb17ac90c1c938
child 777951 1d7c8c50aeabe87e9c0df4959d6b1be0f265c3e0
push id105296
push userpaolo.mozmail@amadzone.org
push dateThu, 05 Apr 2018 10:49:16 +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,