Bug 1499630 - Text nodes that are flex items aren't highlighted correctly in the flexbox highlighter when they wrap r=gl
authorMichael Ratcliffe <mratcliffe@mozilla.com>
Wed, 21 Nov 2018 14:20:30 +0000
changeset 506716 d71137840bd8ccc0a656ab8719289a29e5449d61
parent 506715 8b5e464db40282e66a94eeca41443eaa52d37420
child 506717 9555c7aee866a1e8a66ab82c8373d48272ca228b
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1499630
milestone65.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 1499630 - Text nodes that are flex items aren't highlighted correctly in the flexbox highlighter when they wrap r=gl Depends on D11654 and D12307 Differential Revision: https://phabricator.services.mozilla.com/D12313
devtools/server/actors/highlighters/flexbox.js
devtools/shared/layout/utils.js
--- a/devtools/server/actors/highlighters/flexbox.js
+++ b/devtools/server/actors/highlighters/flexbox.js
@@ -314,17 +314,17 @@ class FlexboxHighlighter extends AutoRef
     const oldMainAxisDirection = this.mainAxisDirection;
     this.mainAxisDirection = flex.mainAxisDirection;
     const newMainAxisDirection = this.mainAxisDirection;
 
     // Concatenate the axes to simplify conditionals.
     this.axes = `${this.mainAxisDirection} ${this.crossAxisDirection}`;
 
     const oldFlexData = this.flexData;
-    this.flexData = getFlexData(flex, this.win);
+    this.flexData = getFlexData(this.currentNode);
     const hasFlexDataChanged = compareFlexData(oldFlexData, this.flexData);
 
     const oldAlignItems = this.alignItemsValue;
     this.alignItemsValue = this.computedStyle.alignItems;
     const newAlignItems = this.alignItemsValue;
 
     const oldFlexDirection = this.flexDirection;
     this.flexDirection = this.computedStyle.flexDirection;
@@ -558,36 +558,26 @@ class FlexboxHighlighter extends AutoRef
     }
 
     const { devicePixelRatio } = this.win;
     const lineWidth = getDisplayPixelRatio(this.win);
     const offset = (lineWidth / 2) % 1;
     const zoom = getCurrentZoom(this.win);
     const canvasX = Math.round(this._canvasPosition.x * devicePixelRatio * zoom);
     const canvasY = Math.round(this._canvasPosition.y * devicePixelRatio * zoom);
-    const containerQuad = getUntransformedQuad(this.currentNode, "content");
-    const containerBounds = containerQuad.getBounds();
 
     this.ctx.save();
     this.ctx.translate(offset - canvasX, offset - canvasY);
     this.ctx.setLineDash(FLEXBOX_LINES_PROPERTIES.item.lineDash);
     this.ctx.lineWidth = lineWidth;
     this.ctx.strokeStyle = this.color;
 
     for (const flexLine of this.flexData.lines) {
       for (const flexItem of flexLine.items) {
-        if (!flexItem.quad) {
-          continue;
-        }
-
-        const itemBounds = flexItem.quad.getBounds();
-        const left = itemBounds.left - containerBounds.left;
-        const top = itemBounds.top - containerBounds.top;
-        const right = itemBounds.right - containerBounds.left;
-        const bottom = itemBounds.bottom - containerBounds.top;
+        const { left, top, right, bottom } = flexItem.rect;
 
         clearRect(this.ctx, left, top, right, bottom, this.currentMatrix);
         drawRect(this.ctx, left, top, right, bottom, this.currentMatrix);
         this.ctx.stroke();
       }
     }
 
     this.ctx.restore();
@@ -694,25 +684,17 @@ class FlexboxHighlighter extends AutoRef
 
     // Draw a justify content pattern over the whole flex container content area.
     this.drawJustifyContent(0, 0, containerBounds.width, containerBounds.height);
 
     for (const flexLine of this.flexData.lines) {
       const { crossStart, crossSize } = flexLine;
 
       for (const flexItem of flexLine.items) {
-        if (!flexItem.quad) {
-          continue;
-        }
-
-        const itemBounds = flexItem.quad.getBounds();
-        const left = itemBounds.left - containerBounds.left;
-        const top = itemBounds.top - containerBounds.top;
-        const right = itemBounds.right - containerBounds.left;
-        const bottom = itemBounds.bottom - containerBounds.top;
+        const { left, top, right, bottom } = flexItem.rect;
 
         // Clear a rectangular are covering the alignment container.
         switch (this.axes) {
           case "horizontal-lr vertical-tb":
           case "horizontal-lr vertical-bt":
           case "horizontal-rl vertical-tb":
           case "horizontal-rl vertical-bt":
             clearRect(this.ctx, left, crossStart + lineWidth, right,
@@ -773,23 +755,25 @@ class FlexboxHighlighter extends AutoRef
 
     setIgnoreLayoutChanges(false, this.highlighterEnv.document.documentElement);
     return true;
   }
 }
 
 /**
  * Returns an object representation of the Flex data object and its array of FlexLine
- * and FlexItem objects along with the box quads of the flex items.
+ * and FlexItem objects along with the DOMRects of the flex items.
  *
- * @param  {Flex} flex
- *         The Flex data object.
+ * @param  {DOMNode} container
+ *         The flex container.
  * @return {Object|null} representation of the Flex data object.
  */
-function getFlexData(flex) {
+function getFlexData(container) {
+  const flex = container.getAsFlexContainer();
+
   if (!flex) {
     return null;
   }
 
   return {
     lines: flex.getLines().map(line => {
       return {
         crossSize: line.crossSize,
@@ -801,25 +785,51 @@ function getFlexData(flex) {
           return {
             crossMaxSize: item.crossMaxSize,
             crossMinSize: item.crossMinSize,
             mainBaseSize: item.mainBaseSize,
             mainDeltaSize: item.mainDeltaSize,
             mainMaxSize: item.mainMaxSize,
             mainMinSize: item.mainMinSize,
             node: item.node,
-            quad: getUntransformedQuad(item.node, "border"),
+            rect: getRectFromFlexItemValues(item, container),
           };
         }),
       };
     }),
   };
 }
 
 /**
+ * Given a FlexItemValues, return a DOMRect representing the flex item taking
+ * into account its flex container's border and padding.
+ *
+ * @param  {FlexItemValues} item
+ *         The FlexItemValues for which we need the DOMRect.
+ * @param  {DOMNode}
+ *         Flex container containing the flex item.
+ * @return {DOMRect} representing the flex item.
+ */
+function getRectFromFlexItemValues(item, container) {
+  const rect = item.frameRect;
+  const domRect = new DOMRect(rect.x, rect.y, rect.width, rect.height);
+  const win = container.ownerGlobal;
+  const style = win.getComputedStyle(container);
+  const borderLeftWidth = parseInt(style.borderLeftWidth, 10) || 0;
+  const borderTopWidth = parseInt(style.borderTopWidth, 10) || 0;
+  const paddingLeft = parseInt(style.paddingLeft, 10) || 0;
+  const paddingTop = parseInt(style.paddingTop, 10) || 0;
+
+  domRect.x -= borderLeftWidth + paddingLeft;
+  domRect.y -= borderTopWidth + paddingTop;
+
+  return domRect;
+}
+
+/**
  * Returns whether or not the flex data has changed.
  *
  * @param  {Flex} oldFlexData
  *         The old Flex data object.
  * @param  {Flex} newFlexData
  *         The new Flex data object.
  * @return {Boolean} true if the flex data has changed and false otherwise.
  */
@@ -862,41 +872,25 @@ function compareFlexData(oldFlexData, ne
           oldItem.crossMinSize !== newItem.crossMinSize ||
           oldItem.mainBaseSize !== newItem.mainBaseSize ||
           oldItem.mainDeltaSize !== newItem.mainDeltaSize ||
           oldItem.mainMaxSize !== newItem.mainMaxSize ||
           oldItem.mainMinSize !== newItem.mainMinSize) {
         return true;
       }
 
-      // For now text node flex items do not have quads because we cannot get
-      // the untransformed position and dimensions of a text node
-      // (see https://bugzil.la/1505079).
-      if ((!oldItem.quad && newItem.quad) ||
-          (oldItem.quad && !newItem.quad)) {
-        return true;
-      }
-
-      if (!oldItem.quad && !newItem.quad) {
-        return false;
-      }
+      const oldItemRect = oldItem.rect;
+      const newItemRect = newItem.rect;
 
-      // There is now only ever one quad so we don't need to compare quad
-      // lengths... we can just check the bounds.
-      const oldItemBounds = oldItem.quad.getBounds();
-      const newItemBounds = newItem.quad.getBounds();
-
-      if (oldItemBounds.bottom !== newItemBounds.bottom ||
-          oldItemBounds.height !== newItemBounds.height ||
-          oldItemBounds.left !== newItemBounds.left ||
-          oldItemBounds.right !== newItemBounds.right ||
-          oldItemBounds.top !== newItemBounds.top ||
-          oldItemBounds.width !== newItemBounds.width ||
-          oldItemBounds.x !== newItemBounds.x ||
-          oldItemBounds.y !== newItemBounds.y) {
+      // We are using DOMRects so we only need to compare x, y, width and
+      // height (left, top, right and bottom are calculated from these values).
+      if (oldItemRect.x !== newItemRect.x ||
+          oldItemRect.y !== newItemRect.y ||
+          oldItemRect.width !== newItemRect.width ||
+          oldItemRect.height !== newItemRect.height) {
         return true;
       }
     }
   }
 
   return false;
 }
 
--- a/devtools/shared/layout/utils.js
+++ b/devtools/shared/layout/utils.js
@@ -829,24 +829,16 @@ exports.removeSheet = removeSheet;
  *         The node for which the DOMQuad is to be returned.
  * @param  {String} region
  *         The box model region to return: "content", "padding", "border" or
  *         "margin".
  * @return {DOMQuad}
  *         A DOMQuad representation of the node.
  */
 function getUntransformedQuad(node, region = "border") {
-  if (node.nodeType === node.TEXT_NODE) {
-    // For now ignore text node flex items because we cannot get the
-    // untransformed position and dimensions of a text node
-    // (see https://bugzil.la/1505079).
-
-    return null;
-  }
-
   // Get the inverse transformation matrix for the node.
   const matrix = node.getTransformToViewport();
   const inverse = matrix.inverse();
   const win = node.ownerGlobal;
 
   // Get the adjusted quads for the node (including scroll offsets).
   const quads = getAdjustedQuads(win, node, region, {
     ignoreZoom: true,