[css-grid] Logical margin incorrectly applied during the tracks sizing algorithm...
authorjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Jun 2017 11:35:27 +0000 (11:35 +0000)
committerjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 Jun 2017 11:35:27 +0000 (11:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172836

Reviewed by Manuel Rego Casasnovas.

Source/WebCore:

When computing min-content and max-content of the content-sized tracks
we are using the marginIntrinsicLogicalWidthForChild function, which
uses the grid's writing-mode to determine wether to use the child's
margin width or height. This is not correct when the grid item is
orthogonal.

This patch changes how we compute the tracks width so we use always
the item's marginLogicalWidth, which depends only on its own writing
mode.

Test: fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows.html

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithmStrategy::minContentForChild):
(WebCore::GridTrackSizingAlgorithmStrategy::maxContentForChild):

LayoutTests:

Test to verify different cases of auto-sized tracks and orthogonal items with margins.

* fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows-expected.txt: Added.
* fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows.html [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-014-expected.txt
LayoutTests/imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-lr-014-expected.txt
LayoutTests/imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-rl-014-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp

index 440b4f7..8dacc70 100644 (file)
@@ -1,5 +1,17 @@
 2017-06-02  Javier Fernandez  <jfernandez@igalia.com>
 
+        [css-grid] Logical margin incorrectly applied during the tracks sizing algorithm of auto tracks
+        https://bugs.webkit.org/show_bug.cgi?id=172836
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        Test to verify different cases of auto-sized tracks and orthogonal items with margins.
+
+        * fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows-expected.txt: Added.
+        * fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows.html: Added.
+
+2017-06-02  Javier Fernandez  <jfernandez@igalia.com>
+
         [css-grid] Margin wrong applied when stretching an orthogonal item in fixed size track
         https://bugs.webkit.org/show_bug.cgi?id=172590
 
index d6c5a7c..bdf4894 100644 (file)
@@ -286,9 +286,6 @@ webkit.org/b/165062 fast/css-grid-layout/grid-baseline.html [ ImageOnlyFailure ]
 webkit.org/b/165062 fast/css-grid-layout/grid-baseline-margins.html [ ImageOnlyFailure ]
 webkit.org/b/149891 imported/w3c/web-platform-tests/css/css-grid-1/grid-layout-properties.html [ Failure ]
 webkit.org/b/169271 imported/w3c/web-platform-tests/css/css-grid-1/grid-items/grid-items-sizing-alignment-001.html [ ImageOnlyFailure ]
-webkit.org/b/172836 imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-014.html [ Failure ]
-webkit.org/b/172836 imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-rl-014.html [ Failure ]
-webkit.org/b/172836 imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-lr-014.html [ Failure ]
 
 # selectors4
 webkit.org/b/64861 imported/w3c/web-platform-tests/css/selectors4/selectors-dir-selector-ltr-001.html [ ImageOnlyFailure ]
diff --git a/LayoutTests/fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows-expected.txt b/LayoutTests/fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows-expected.txt
new file mode 100644 (file)
index 0000000..c2d3748
--- /dev/null
@@ -0,0 +1,26 @@
+
+PASS .grid 1 
+PASS .grid 2 
+PASS .grid 3 
+PASS .grid 4 
+This test checks that grid items' margin is computed correctly for content-sized tracks when using different writing modes.
+
+X
+X
+X
+X
+
+X
+X
+X
+X
+
+X
+X
+X
+X
+
+X
+X
+X
+X
diff --git a/LayoutTests/fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows.html b/LayoutTests/fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows.html
new file mode 100644 (file)
index 0000000..b15a428
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<link href="resources/grid.css" rel="stylesheet">
+<link href="resources/grid-alignment.css" rel="stylesheet">
+<link href="../css-intrinsic-dimensions/resources/width-keyword-classes.css" rel="stylesheet">
+<style>
+.grid { font: 20px/1 Ahem; }
+.item { margin: 10px 5px 20px 15px; }
+</style>
+<script src="../../resources/testharness.js"></script>
+<script src="../../resources/testharnessreport.js"></script>
+<script src="../../resources/check-layout-th.js"></script>
+
+<body onload="checkLayout('.grid')">
+<div id="log"></div>
+<p>This test checks that grid items' margin is computed correctly for content-sized tracks when using different writing modes.</p>
+
+<div style="position: relative">
+    <div class="grid fit-content itemsStart" data-expected-width="40" data-expected-height="200">
+        <div class="item verticalLR" data-offset-x="15" data-offset-y="10"  data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item verticalLR" data-offset-x="15" data-offset-y="60"  data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item verticalLR" data-offset-x="15" data-offset-y="110" data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item verticalLR" data-offset-x="15" data-offset-y="160" data-expected-width="20" data-expected-height="20">X</div>
+    </div>
+</div>
+
+<br>
+
+<div style="position: relative">
+    <div class="grid gridAutoFlowColumnSparse fit-content itemsStart" data-expected-width="160" data-expected-height="50">
+        <div class="item verticalLR" data-offset-x="15"  data-offset-y="10" data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item verticalLR" data-offset-x="55"  data-offset-y="10" data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item verticalLR" data-offset-x="95"  data-offset-y="10" data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item verticalLR" data-offset-x="135" data-offset-y="10" data-expected-width="20" data-expected-height="20">X</div>
+    </div>
+</div>
+
+<br>
+
+<div style="position: relative">
+    <div class="grid fit-content verticalLR itemsStart" data-expected-width="160" data-expected-height="50">
+        <div class="item horizontalTB" data-offset-x="15"  data-offset-y="10" data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item horizontalTB" data-offset-x="55"  data-offset-y="10" data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item horizontalTB" data-offset-x="95"  data-offset-y="10" data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item horizontalTB" data-offset-x="135" data-offset-y="10" data-expected-width="20" data-expected-height="20">X</div>
+    </div>
+</div>
+
+<br>
+
+<div style="position: relative">
+    <div class="grid gridAutoFlowColumnSparse fit-content verticalLR itemsStart" data-expected-width="40" data-expected-height="200">
+        <div class="item horizontalTB" data-offset-x="15" data-offset-y="10"  data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item horizontalTB" data-offset-x="15" data-offset-y="60"  data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item horizontalTB" data-offset-x="15" data-offset-y="110" data-expected-width="20" data-expected-height="20">X</div>
+        <div class="item horizontalTB" data-offset-x="15" data-offset-y="160" data-expected-width="20" data-expected-height="20">X</div>
+    </div>
+</div>
+
+</body>
index f7e2b54..60a8366 100644 (file)
@@ -3,12 +3,5 @@ XX X
 XX X
 XX X
 
-FAIL .grid 1 assert_equals: 
-<div class="grid">
-  <div data-offset-x="0" data-offset-y="0" data-expected-width="110" data-expected-height="80" class="firstRowFirstColumn">XX X</div>
-  <div data-offset-x="120" data-offset-y="0" data-expected-width="20" data-expected-height="110" class="firstRowSecondColumn">XX X</div>
-  <div data-offset-x="0" data-offset-y="130" data-expected-width="20" data-expected-height="80" class="secondRowFirstColumn">XX X</div>
-  <div data-offset-x="120" data-offset-y="130" data-expected-width="110" data-expected-height="110" class="secondRowSecondColumn">XX X</div>
-</div>
-width expected 110 but got 120
+PASS .grid 1 
 
index 51458ca..60a8366 100644 (file)
@@ -3,12 +3,5 @@ XX X
 XX X
 XX X
 
-FAIL .grid 1 assert_equals: 
-<div class="grid">
-  <div data-offset-x="0" data-offset-y="0" data-expected-width="80" data-expected-height="110" class="firstRowFirstColumn">XX X</div>
-  <div data-offset-x="0" data-offset-y="120" data-expected-width="110" data-expected-height="20" class="firstRowSecondColumn">XX X</div>
-  <div data-offset-x="130" data-offset-y="0" data-expected-width="80" data-expected-height="20" class="secondRowFirstColumn">XX X</div>
-  <div data-offset-x="130" data-offset-y="120" data-expected-width="110" data-expected-height="110" class="secondRowSecondColumn">XX X</div>
-</div>
-height expected 110 but got 120
+PASS .grid 1 
 
index b613898..60a8366 100644 (file)
@@ -3,12 +3,5 @@ XX X
 XX X
 XX X
 
-FAIL .grid 1 assert_equals: 
-<div class="grid">
-  <div data-offset-x="170" data-offset-y="0" data-expected-width="80" data-expected-height="110" class="firstRowFirstColumn">XX X</div>
-  <div data-offset-x="120" data-offset-y="120" data-expected-width="110" data-expected-height="20" class="firstRowSecondColumn">XX X</div>
-  <div data-offset-x="40" data-offset-y="0" data-expected-width="80" data-expected-height="20" class="secondRowFirstColumn">XX X</div>
-  <div data-offset-x="0" data-offset-y="120" data-expected-width="110" data-expected-height="110" class="secondRowSecondColumn">XX X</div>
-</div>
-height expected 110 but got 120
+PASS .grid 1 
 
index de817f2..fcddbb2 100644 (file)
@@ -1,3 +1,26 @@
+2017-06-02  Javier Fernandez  <jfernandez@igalia.com>
+
+        [css-grid] Logical margin incorrectly applied during the tracks sizing algorithm of auto tracks
+        https://bugs.webkit.org/show_bug.cgi?id=172836
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        When computing min-content and max-content of the content-sized tracks
+        we are using the marginIntrinsicLogicalWidthForChild function, which
+        uses the grid's writing-mode to determine wether to use the child's
+        margin width or height. This is not correct when the grid item is
+        orthogonal.
+
+        This patch changes how we compute the tracks width so we use always
+        the item's marginLogicalWidth, which depends only on its own writing
+        mode.
+
+        Test: fast/css-grid-layout/grid-track-sizing-with-margins-and-orthogonal-flows.html
+
+        * rendering/GridTrackSizingAlgorithm.cpp:
+        (WebCore::GridTrackSizingAlgorithmStrategy::minContentForChild):
+        (WebCore::GridTrackSizingAlgorithmStrategy::maxContentForChild):
+
 2017-06-02  Emilio Cobos Ãlvarez  <ecobos@igalia.com>
 
         Invalidate the shadow subtree style when slotted pseudo rules are present.
index 1b089bf..25afaa8 100644 (file)
@@ -749,7 +749,8 @@ LayoutUnit GridTrackSizingAlgorithmStrategy::minContentForChild(RenderBox& child
 
         // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
         // See http://lists.w3.org/Archives/Public/www-style/2013Jan/0245.html
-        return child.minPreferredLogicalWidth() + marginIntrinsicLogicalWidthForChild(renderGrid(), child);
+        LayoutUnit marginLogicalWidth = child.needsLayout() ? computeMarginLogicalSizeForChild(childInlineDirection, *renderGrid(), child) : child.marginLogicalWidth();
+        return child.minPreferredLogicalWidth() + marginLogicalWidth;
     }
 
     if (updateOverrideContainingBlockContentSizeForChild(child, childInlineDirection))
@@ -768,7 +769,8 @@ LayoutUnit GridTrackSizingAlgorithmStrategy::maxContentForChild(RenderBox& child
 
         // FIXME: It's unclear if we should return the intrinsic width or the preferred width.
         // See http://lists.w3.org/Archives/Public/www-style/2013Jan/0245.html
-        return child.maxPreferredLogicalWidth() + marginIntrinsicLogicalWidthForChild(renderGrid(), child);
+        LayoutUnit marginLogicalWidth = child.needsLayout() ? computeMarginLogicalSizeForChild(childInlineDirection, *renderGrid(), child) : child.marginLogicalWidth();
+        return child.maxPreferredLogicalWidth() + marginLogicalWidth;
     }
 
     if (updateOverrideContainingBlockContentSizeForChild(child, childInlineDirection))