[css-grid] Display issues with child with max-width
authorjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Nov 2017 09:29:07 +0000 (09:29 +0000)
committerjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Nov 2017 09:29:07 +0000 (09:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178973

Reviewed by Darin Adler.

Source/WebCore:

We have an utility function to compute the grid item's margins
when the item still needs to layout. We used the function
RenderStyle::hasMarings to decide whether such margin computation
worths. However, we need that function to operate on a specific
axis, hence I added a new function adding such logic.

Additionally, we must treat any 'auto' margin as 0px during the
tracks sizing algorithm, as the CSS Grid spec states:

  - https://drafts.csswg.org/css-grid/#auto-margins

Test: fast/css-grid-layout/auto-margins-ignored-during-track-sizing.html

* rendering/GridLayoutFunctions.cpp:
(WebCore::GridLayoutFunctions::childHasMargin): New funciton with axis dependent logic.
(WebCore::GridLayoutFunctions::computeMarginLogicalSizeForChild): Ignore auto margins.
(WebCore::GridLayoutFunctions::marginLogicalSizeForChild): Ignore auto margins.

LayoutTests:

Regression test for the bug.

* fast/css-grid-layout/auto-margins-ignored-during-track-sizing-expected.html: Added.
* fast/css-grid-layout/auto-margins-ignored-during-track-sizing.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@225163 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css-grid-layout/auto-margins-ignored-during-track-sizing-expected.html [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/auto-margins-ignored-during-track-sizing.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/GridLayoutFunctions.cpp

index 7aac5d8..260a418 100644 (file)
@@ -1,3 +1,15 @@
+2017-11-27  Javier Fernandez  <jfernandez@igalia.com>
+
+        [css-grid] Display issues with child with max-width
+        https://bugs.webkit.org/show_bug.cgi?id=178973
+
+        Reviewed by Darin Adler.
+
+        Regression test for the bug.
+
+        * fast/css-grid-layout/auto-margins-ignored-during-track-sizing-expected.html: Added.
+        * fast/css-grid-layout/auto-margins-ignored-during-track-sizing.html: Added.
+
 2017-11-25  Frederic Wang  <fwang@igalia.com>
 
         Import MathML WPT tests
diff --git a/LayoutTests/fast/css-grid-layout/auto-margins-ignored-during-track-sizing-expected.html b/LayoutTests/fast/css-grid-layout/auto-margins-ignored-during-track-sizing-expected.html
new file mode 100644 (file)
index 0000000..7ff637d
--- /dev/null
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<style>
+body { overflow: hidden; }
+.grid {
+  display: grid;
+  grid-template-columns: 1fr 1fr 1fr;
+}
+.margin { margin-top: 10px; }
+.center { justify-self: center; }
+.i1 { background: magenta;  }
+.i2 { background: cyan; }
+.i3 { background: yellow; }
+.i4 { background: lime; }
+</style>
+<div class="grid">
+  <div class="i1">
+    In a few questions, you’ll get an expert-designed investment portfolio to fit your financial needs.
+  </div>
+  <div class="i2">
+    Open and fund your account with $10,000 or more and we’ll put your money to work.
+  </div>
+  <div class="i3">
+    We’ll take it from here, monitoring your portfolio daily to help keep it on track.
+  </div>
+  <div class="i4 margin center">Learn More</a>
+</div>
diff --git a/LayoutTests/fast/css-grid-layout/auto-margins-ignored-during-track-sizing.html b/LayoutTests/fast/css-grid-layout/auto-margins-ignored-during-track-sizing.html
new file mode 100644 (file)
index 0000000..f114e3e
--- /dev/null
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<style>
+body { overflow: hidden; }
+.grid {
+  display: grid;
+  grid-template-columns: 1fr 1fr 1fr;
+}
+.margin-center {
+  margin-top: 10px;
+  margin-left: auto;
+  margin-right: auto;
+}
+.i1 { background: magenta;  }
+.i2 { background: cyan; }
+.i3 { background: yellow; }
+.i4 { background: lime; }
+</style>
+<div class="grid">
+  <div class="i1">
+    In a few questions, you’ll get an expert-designed investment portfolio to fit your financial needs.
+  </div>
+  <div class="i2">
+    Open and fund your account with $10,000 or more and we’ll put your money to work.
+  </div>
+  <div class="i3">
+    We’ll take it from here, monitoring your portfolio daily to help keep it on track.
+  </div>
+  <div class="i4 margin-center">Learn More</a>
+</div>
index fc68530..23c1192 100644 (file)
@@ -1,3 +1,28 @@
+2017-11-27  Javier Fernandez  <jfernandez@igalia.com>
+
+        [css-grid] Display issues with child with max-width
+        https://bugs.webkit.org/show_bug.cgi?id=178973
+
+        Reviewed by Darin Adler.
+
+        We have an utility function to compute the grid item's margins
+        when the item still needs to layout. We used the function
+        RenderStyle::hasMarings to decide whether such margin computation
+        worths. However, we need that function to operate on a specific
+        axis, hence I added a new function adding such logic.
+
+        Additionally, we must treat any 'auto' margin as 0px during the
+        tracks sizing algorithm, as the CSS Grid spec states:
+
+          - https://drafts.csswg.org/css-grid/#auto-margins
+
+        Test: fast/css-grid-layout/auto-margins-ignored-during-track-sizing.html
+
+        * rendering/GridLayoutFunctions.cpp:
+        (WebCore::GridLayoutFunctions::childHasMargin): New funciton with axis dependent logic.
+        (WebCore::GridLayoutFunctions::computeMarginLogicalSizeForChild): Ignore auto margins.
+        (WebCore::GridLayoutFunctions::marginLogicalSizeForChild): Ignore auto margins.
+
 2017-11-26  Simon Fraser  <simon.fraser@apple.com>
 
         feImage as filter input has skewed colors
index 8e232e6..cb6bb32 100644 (file)
@@ -32,9 +32,27 @@ namespace WebCore {
 
 namespace GridLayoutFunctions {
 
+static inline bool marginStartIsAuto(const RenderBox& child, GridTrackSizingDirection direction)
+{
+    return direction == ForColumns ? child.style().marginStart().isAuto() : child.style().marginBefore().isAuto();
+}
+
+static inline bool marginEndIsAuto(const RenderBox& child, GridTrackSizingDirection direction)
+{
+    return direction == ForColumns ? child.style().marginEnd().isAuto() : child.style().marginAfter().isAuto();
+}
+
+static bool childHasMargin(const RenderBox& child, GridTrackSizingDirection direction)
+{
+    // Length::IsZero returns true for 'auto' margins, which is aligned with the purpose of this function.
+    if (direction == ForColumns)
+        return !child.style().marginStart().isZero() || !child.style().marginEnd().isZero();
+    return !child.style().marginBefore().isZero() || !child.style().marginAfter().isZero();
+}
+
 LayoutUnit computeMarginLogicalSizeForChild(const RenderGrid& grid, GridTrackSizingDirection direction, const RenderBox& child)
 {
-    if (!child.style().hasMargin())
+    if (!childHasMargin(child, flowAwareDirectionForChild(grid, child, direction)))
         return 0;
 
     LayoutUnit marginStart;
@@ -43,15 +61,17 @@ LayoutUnit computeMarginLogicalSizeForChild(const RenderGrid& grid, GridTrackSiz
         child.computeInlineDirectionMargins(grid, child.containingBlockLogicalWidthForContentInFragment(nullptr), child.logicalWidth(), marginStart, marginEnd);
     else
         child.computeBlockDirectionMargins(grid, marginStart, marginEnd);
-
-    return marginStart + marginEnd;
+    return marginStartIsAuto(child, direction) ? marginEnd : marginEndIsAuto(child, direction) ? marginStart : marginStart + marginEnd;
 }
 
 LayoutUnit marginLogicalSizeForChild(const RenderGrid& grid, GridTrackSizingDirection direction, const RenderBox& child)
 {
     if (child.needsLayout())
         return computeMarginLogicalSizeForChild(grid, direction, child);
-    return flowAwareDirectionForChild(grid, child, direction) == ForColumns ? child.marginLogicalWidth() : child.marginLogicalHeight();
+    bool isRowAxis = flowAwareDirectionForChild(grid, child, direction) == ForColumns;
+    LayoutUnit marginStart = marginStartIsAuto(child, direction) ? LayoutUnit() : isRowAxis ? child.marginStart() : child.marginBefore();
+    LayoutUnit marginEnd = marginEndIsAuto(child, direction) ? LayoutUnit() : isRowAxis ? child.marginEnd() : child.marginAfter();
+    return marginStart + marginEnd;
 }
 
 bool isOrthogonalChild(const RenderGrid& grid, const RenderBox& child)