Bug 1497182 - Flexbox justify-content incorrectly drawn when using flex-direction: row-reverse r=gl
authorMichael Ratcliffe <mratcliffe@mozilla.com>
Fri, 26 Oct 2018 12:00:16 +0000
changeset 443111 aa01df6dc4db602fff7b15b8179fa936a5141700
parent 443110 5866c7a7e9066aa7451bf056cd82ca0683093bff
child 443112 f2ea2c762c5dd5a82534ba4a1cf87891b015a59a
push id34939
push usernerli@mozilla.com
push dateFri, 26 Oct 2018 15:47:55 +0000
treeherdermozilla-central@760a16bf0d2b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1497182
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 1497182 - Flexbox justify-content incorrectly drawn when using flex-direction: row-reverse r=gl Differential Revision: https://phabricator.services.mozilla.com/D7999
devtools/server/actors/highlighters/flexbox.js
--- a/devtools/server/actors/highlighters/flexbox.js
+++ b/devtools/server/actors/highlighters/flexbox.js
@@ -160,16 +160,18 @@ class FlexboxHighlighter extends AutoRef
     }
 
     this.markup.destroy();
 
     // Clear the pattern cache to avoid dead object exceptions (Bug 1342051).
     this.clearCache();
 
     this.flexData = null;
+    this.crossAxisDirection = null;
+    this.mainAxisDirection = null;
 
     AutoRefreshHighlighter.prototype.destroy.call(this);
   }
 
   /**
    * Draw the justify content for a given flex item (left, top, right, bottom) position.
    */
   drawJustifyContent(left, top, right, bottom) {
@@ -296,18 +298,22 @@ class FlexboxHighlighter extends AutoRef
    */
   _hasMoved() {
     const hasMoved = AutoRefreshHighlighter.prototype._hasMoved.call(this);
 
     if (!this.computedStyle) {
       this.computedStyle = getComputedStyle(this.currentNode);
     }
 
+    const flex = this.currentNode.getAsFlexContainer();
+    this.crossAxisDirection = flex.crossAxisDirection;
+    this.mainAxisDirection = flex.mainAxisDirection;
+
     const oldFlexData = this.flexData;
-    this.flexData = getFlexData(this.currentNode.getAsFlexContainer(), this.win);
+    this.flexData = getFlexData(flex, this.win);
     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;
@@ -583,103 +589,128 @@ class FlexboxHighlighter extends AutoRef
     const canvasY = Math.round(this._canvasPosition.y * devicePixelRatio * zoom);
 
     this.ctx.save();
     this.ctx.translate(offset - canvasX, offset - canvasY);
     this.ctx.lineWidth = lineWidth;
     this.ctx.strokeStyle = this.color;
 
     const { bounds } = this.currentQuads.content[0];
-    const isColumn = this.flexDirection.startsWith("column");
     const options = { matrix: this.currentMatrix };
 
     for (const flexLine of this.flexData.lines) {
       const { crossStart, crossSize } = flexLine;
 
-      if (isColumn) {
-        clearRect(this.ctx, crossStart, 0, crossStart + crossSize, bounds.height,
-          this.currentMatrix);
+      switch (this.mainAxisDirection) {
+        case "horizontal-lr":
+        case "horizontal-rl":
+          clearRect(this.ctx, 0, crossStart, bounds.width, crossStart + crossSize,
+            this.currentMatrix);
 
-        // Avoid drawing the start flex line when they overlap with the flex container.
-        if (crossStart != 0) {
-          drawLine(this.ctx, crossStart, 0, crossStart, bounds.height, options);
-          this.ctx.stroke();
-        }
+          // Avoid drawing the start flex line when they overlap with the flex container.
+          if (crossStart != 0) {
+            drawLine(this.ctx, 0, crossStart, bounds.width, crossStart, options);
+            this.ctx.stroke();
+          }
+
+          // Avoid drawing the end flex line when they overlap with the flex container.
+          if (bounds.height - crossStart - crossSize >= lineWidth) {
+            drawLine(this.ctx, 0, crossStart + crossSize, bounds.width,
+              crossStart + crossSize, options);
+            this.ctx.stroke();
+          }
+          break;
+        case "vertical-tb":
+          clearRect(this.ctx, crossStart, 0, crossStart + crossSize, bounds.height,
+            this.currentMatrix);
 
-        // Avoid drawing the end flex line when they overlap with the flex container.
-        if (bounds.width - crossStart - crossSize >= lineWidth) {
-          drawLine(this.ctx, crossStart + crossSize, 0, crossStart + crossSize,
-            bounds.height, options);
-          this.ctx.stroke();
-        }
-      } else {
-        clearRect(this.ctx, 0, crossStart, bounds.width, crossStart + crossSize,
-          this.currentMatrix);
+          // Avoid drawing the start flex line when they overlap with the flex container.
+          if (crossStart != 0) {
+            drawLine(this.ctx, crossStart, 0, crossStart, bounds.height, options);
+            this.ctx.stroke();
+          }
 
-        // Avoid drawing the start flex line when they overlap with the flex container.
-        if (crossStart != 0) {
-          drawLine(this.ctx, 0, crossStart, bounds.width, crossStart, options);
-          this.ctx.stroke();
-        }
+          // Avoid drawing the end flex line when they overlap with the flex container.
+          if (bounds.width - crossStart - crossSize >= lineWidth) {
+            drawLine(this.ctx, crossStart + crossSize, 0, crossStart + crossSize,
+              bounds.height, options);
+            this.ctx.stroke();
+          }
+          break;
+        case "vertical-bt":
+          clearRect(this.ctx, bounds.width - crossStart, 0,
+            bounds.width - crossStart - crossSize, bounds.height, this.currentMatrix);
 
-        // Avoid drawing the end flex line when they overlap with the flex container.
-        if (bounds.height - crossStart - crossSize >= lineWidth) {
-          drawLine(this.ctx, 0, crossStart + crossSize, bounds.width,
-            crossStart + crossSize, options);
-          this.ctx.stroke();
-        }
+          // Avoid drawing the start flex line when they overlap with the flex container.
+          if (crossStart != 0) {
+            drawLine(this.ctx, bounds.width - crossStart, 0, bounds.width - crossStart,
+              bounds.height, options);
+            this.ctx.stroke();
+          }
+
+          // Avoid drawing the end flex line when they overlap with the flex container.
+          if (bounds.width - crossStart - crossSize >= lineWidth) {
+            drawLine(this.ctx, bounds.width - crossStart - crossSize, 0,
+              bounds.width - crossStart - crossSize, bounds.height, options);
+            this.ctx.stroke();
+          }
+          break;
       }
     }
 
     this.ctx.restore();
   }
 
+  /**
+   * Clear the whole alignment container along the main axis for each flex item.
+   */
   renderJustifyContent() {
     if (!this.flexData || !this.currentQuads.content || !this.currentQuads.content[0]) {
       return;
     }
 
     const zoom = getCurrentZoom(this.win);
     const { bounds } = this.currentQuads.content[0];
-    const isColumn = this.flexDirection.startsWith("column");
+
+    // Draw a justify content pattern over the whole flex container.
+    this.drawJustifyContent(0, 0, bounds.width, bounds.height, this.currentMatrix);
 
     for (const flexLine of this.flexData.lines) {
       const { crossStart, crossSize } = flexLine;
-      let mainStart = 0;
 
       for (const flexItem of flexLine.items) {
         const quads = flexItem.quads;
         if (!quads.length) {
           continue;
         }
 
         // Adjust the flex item bounds relative to the current quads.
         const { bounds: flexItemBounds } = quads[0];
         const left = Math.round(flexItemBounds.left / zoom - bounds.left);
         const top = Math.round(flexItemBounds.top / zoom - bounds.top);
         const right = Math.round(flexItemBounds.right / zoom - bounds.left);
         const bottom = Math.round(flexItemBounds.bottom / zoom - bounds.top);
 
-        if (isColumn) {
-          this.drawJustifyContent(crossStart, mainStart, crossStart + crossSize, top);
-          mainStart = bottom;
-        } else {
-          this.drawJustifyContent(mainStart, crossStart, left, crossStart + crossSize);
-          mainStart = right;
+        // Clear a rectangular are covering the alignment container.
+        switch (this.mainAxisDirection) {
+          case "horizontal-lr":
+          case "horizontal-rl":
+            clearRect(this.ctx, left, Math.round(crossStart) + 2, right,
+              Math.round(crossStart + crossSize) - 2, this.currentMatrix);
+            break;
+          case "vertical-tb":
+            clearRect(this.ctx, Math.round(crossStart) + 1, top,
+              Math.round(crossStart + crossSize), bottom, this.currentMatrix);
+            break;
+          case "vertical-bt":
+            clearRect(this.ctx, Math.round(bounds.width - crossStart - crossSize) + 1,
+              top, Math.round(bounds.width - crossStart), bottom, this.currentMatrix);
+            break;
         }
       }
-
-      // Draw the last justify-content area after the last flex item.
-      if (isColumn) {
-        this.drawJustifyContent(crossStart, mainStart, crossStart + crossSize,
-          bounds.height);
-      } else {
-        this.drawJustifyContent(mainStart, crossStart, bounds.width,
-          crossStart + crossSize);
-      }
     }
   }
 
   _update() {
     setIgnoreLayoutChanges(true);
 
     const root = this.getElement("root");