[css-grid] Spanning Grid item has too much space at the bottom / is too high
authorrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 12:33:34 +0000 (12:33 +0000)
committerrego@igalia.com <rego@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 12:33:34 +0000 (12:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181677

LayoutTests/imported/w3c:

Imported WPT tests to check this change.

Reviewed by Javier Fernandez.

* web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001-expected.txt: Added.
* web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001.html: Added.
* web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002-expected.txt: Added.
* web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002.html: Added.
* web-platform-tests/css/css-grid/layout-algorithm/w3c-import.log:

Source/WebCore:

Reviewed by Javier Fernandez.

In IndefiniteSizeStrategy::findUsedFlexFraction() we were not
subtracting the size of the gutters when we call findFrUnitSize().
If an item spans several tracks, we cannot pass the maxContentForChild()
directly, we need to subtract the gutters as they are treated
as fixed size tracks in the algorithm.

The spec text is pretty clear regarding this
(https://drafts.csswg.org/css-grid/#algo-find-fr-size):
"Let leftover space be the space to fill minus the base sizes
 of the non-flexible grid tracks."

Gutters are treated as fixed-size tracks for the purpose
of the track sizing algorithm, so we need to subtract them from the
leftover space while finding the size of an "fr".

Tests: imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001.html
       imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002.html

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithm::findFrUnitSize const):
(WebCore::IndefiniteSizeStrategy::findUsedFlexFraction const):

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

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001-expected.txt [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001.html [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002-expected.txt [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002.html [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/w3c-import.log
Source/WebCore/ChangeLog
Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp

index c41a5d8..8ce2898 100644 (file)
@@ -1,3 +1,18 @@
+2018-01-22  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [css-grid] Spanning Grid item has too much space at the bottom / is too high
+        https://bugs.webkit.org/show_bug.cgi?id=181677
+
+        Imported WPT tests to check this change.
+
+        Reviewed by Javier Fernandez.
+
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001-expected.txt: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001.html: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002-expected.txt: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002.html: Added.
+        * web-platform-tests/css/css-grid/layout-algorithm/w3c-import.log:
+
 2018-01-20  Youenn Fablet  <youenn@apple.com>
 
         fetch redirect is incompatible with "no-cors" mode
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001-expected.txt
new file mode 100644 (file)
index 0000000..681b89b
--- /dev/null
@@ -0,0 +1,87 @@
+
+PASS .grid 1 
+PASS .grid 2 
+PASS .grid 3 
+PASS .grid 4 
+PASS .grid 5 
+PASS .grid 6 
+PASS .grid 7 
+PASS .grid 8 
+PASS .grid 9 
+PASS .grid 10 
+PASS .grid 11 
+PASS .grid 12 
+PASS .grid 13 
+PASS .grid 14 
+PASS .grid 15 
+PASS .grid 16 
+PASS .grid 17 
+XXXXXXXX
+X
+X
+X
+X
+XXXXXXXX
+X
+X
+X
+X
+X
+X
+X
+XXXXXXXX
+XXXXXXXXXXXXXXXXXXXX
+X
+X
+X
+X
+XXXXXXXX
+X
+X
+X
+X
+XXXXXXXXXXXXXXXXXXXX
+X
+X
+X
+XXXXXXXX
+X
+X
+X
+X
+X
+X
+X
+X
+X
+X
+X
+X
+X
+X
+XXXXXXXX
+X
+X
+X
+X
+XXXXXXXX
+XXXXXXXX
+X
+X
+X
+X
+X
+X
+X
+X
+X
+XXXXXXXXXXXXXXXXXXXX
+X
+X
+X
+X
+X
+X
+X
+X
+X
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001.html
new file mode 100644 (file)
index 0000000..25d236c
--- /dev/null
@@ -0,0 +1,165 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Grid Layout Test: Grid Track Sizing Algorithm Flexible Tracks and Gutters</title>
+<link rel="author" title="Manuel Rego Casasnovas" href="mailto:rego@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css-grid/#gutters">
+<link rel="help" href="https://drafts.csswg.org/css-grid/#algo-find-fr-size">
+<meta name="assert" content="This test checks that the size of flexible tracks is properly computed when you have gaps (with and without spanning items).">
+<style>
+.grid {
+  position: relative;
+  display: grid;
+  grid-gap: 10px 20px;
+  font: 10px/1 Ahem;
+  margin: 50px;
+}
+
+.fixedSize {
+  width: 200px;
+  height: 100px;
+}
+
+.contentBasedSize {
+  float: left;
+  height: auto;
+}
+
+.grid div:nth-child(1) { background: magenta; }
+.grid div:nth-child(2) { background: cyan; }
+.grid div:nth-child(3) { background: yellow; }
+.grid div:nth-child(4) { background: lime; }
+</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>
+
+<div class="grid fixedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+  <div data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="50"></div>
+  <div data-offset-x="0" data-offset-y="60" data-expected-width="100" data-expected-height="40"></div>
+  <div data-offset-x="120" data-offset-y="60" data-expected-width="80" data-expected-height="40"></div>
+</div>
+
+<div class="grid fixedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-column: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="200" data-expected-height="50"></div>
+  <div data-offset-x="0" data-offset-y="60" data-expected-width="100" data-expected-height="40"></div>
+  <div data-offset-x="120" data-offset-y="60" data-expected-width="80" data-expected-height="40"></div>
+</div>
+
+<div class="grid fixedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-column: 1; grid-row: 1;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+  <div style="grid-column: 2; grid-row: 1;"
+    data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="50"></div>
+  <div style="grid-column: span 2;"
+    data-offset-x="0" data-offset-y="60" data-expected-width="200" data-expected-height="40"></div>
+</div>
+
+<div class="grid fixedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-row: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="100"></div>
+  <div data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="50"></div>
+  <div data-offset-x="120" data-offset-y="60" data-expected-width="80" data-expected-height="40"></div>
+</div>
+
+<div class="grid fixedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-column: 1; grid-row: 1;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+  <div style="grid-column: 1; grid-row: 2;"
+    data-offset-x="0" data-offset-y="60" data-expected-width="100" data-expected-height="40"></div>
+  <div style="grid-row: span 2;"
+    data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="100"></div>
+</div>
+
+<div class="grid fixedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-row: span 2; grid-column: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="200" data-expected-height="100"></div>
+</div>
+
+<div class="grid contentBasedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+  <div data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="50">XXXXXXXX</div>
+  <div data-offset-x="0" data-offset-y="60" data-expected-width="100" data-expected-height="40">X<br>X<br>X<br>X</div>
+  <div data-offset-x="120" data-offset-y="60" data-expected-width="80" data-expected-height="40"></div>
+</div>
+
+<div class="grid contentBasedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+  <div data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="50"></div>
+  <div data-offset-x="0" data-offset-y="60" data-expected-width="100" data-expected-height="40"></div>
+  <div data-offset-x="120" data-offset-y="60" data-expected-width="80" data-expected-height="40">XXXXXXXX<br>X<br>X<br>X</div>
+</div>
+
+<div class="grid contentBasedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-column: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="200" data-expected-height="50"></div>
+  <div data-offset-x="0" data-offset-y="60" data-expected-width="100" data-expected-height="40">X<br>X<br>X<br>X</div>
+  <div data-offset-x="120" data-offset-y="60" data-expected-width="80" data-expected-height="40">XXXXXXXX</div>
+</div>
+
+<div class="grid contentBasedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-column: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="200" data-expected-height="50">XXXXXXXXXXXXXXXXXXXX</div>
+  <div data-offset-x="0" data-offset-y="60" data-expected-width="100" data-expected-height="40">X<br>X<br>X<br>X</div>
+  <div data-offset-x="120" data-offset-y="60" data-expected-width="80" data-expected-height="40"></div>
+</div>
+
+<div class="grid contentBasedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-column: 1; grid-row: 1;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+  <div style="grid-column: 2; grid-row: 1;"
+    data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="50">XXXXXXXX</div>
+  <div style="grid-column: span 2;"
+    data-offset-x="0" data-offset-y="60" data-expected-width="200" data-expected-height="40">X<br>X<br>X<br>X</div>
+</div>
+
+<div class="grid contentBasedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-column: 1; grid-row: 1;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+  <div style="grid-column: 2; grid-row: 1;"
+    data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="50"></div>
+  <div style="grid-column: span 2;"
+    data-offset-x="0" data-offset-y="60" data-expected-width="200" data-expected-height="40">XXXXXXXXXXXXXXXXXXXX<br>X<br>X<br>X</div>
+</div>
+
+<div class="grid contentBasedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-row: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="100"></div>
+  <div data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="50">XXXXXXXX</div>
+  <div data-offset-x="120" data-offset-y="60" data-expected-width="80" data-expected-height="40">X<br>X<br>X<br>X</div>
+</div>
+
+<div class="grid contentBasedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-row: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="100">X<br>X<br>X<br>X<br>X<br>X<br>X<br>X<br>X<br>X</div>
+  <div data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="50">XXXXXXXX</div>
+  <div data-offset-x="120" data-offset-y="60" data-expected-width="80" data-expected-height="40"></div>
+</div>
+
+<div class="grid contentBasedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-column: 1; grid-row: 1;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+  <div style="grid-column: 1; grid-row: 2;"
+    data-offset-x="0" data-offset-y="60" data-expected-width="100" data-expected-height="40">X<br>X<br>X<br>X</div>
+  <div style="grid-row: span 2;"
+    data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="100">XXXXXXXX</div>
+</div>
+
+<div class="grid contentBasedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-column: 1; grid-row: 1;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="50"></div>
+  <div style="grid-column: 1; grid-row: 2;"
+    data-offset-x="0" data-offset-y="60" data-expected-width="100" data-expected-height="40"></div>
+  <div style="grid-row: span 2;"
+    data-offset-x="120" data-offset-y="0" data-expected-width="80" data-expected-height="100">XXXXXXXX<br>X<br>X<br>X<br>X<br>X<br>X<br>X<br>X<br>X</div>
+</div>
+
+<div class="grid contentBasedSize" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-row: span 2; grid-column: span 2;"
+       data-offset-x="0" data-offset-y="0" data-expected-width="200" data-expected-height="100">
+XXXXXXXXXXXXXXXXXXXX<br>X<br>X<br>X<br>X<br>X<br>X<br>X<br>X<br>X</div>
+</div>
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002-expected.txt
new file mode 100644 (file)
index 0000000..485c78b
--- /dev/null
@@ -0,0 +1,17 @@
+
+PASS .grid 1 
+PASS .grid 2 
+PASS .grid 3 
+PASS .grid 4 
+PASS .grid 5 
+PASS .grid 6 
+X
+X
+X
+X
+X
+X
+
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002.html
new file mode 100644 (file)
index 0000000..340fe82
--- /dev/null
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>CSS Grid Layout Test: Grid Track Sizing Algorithm Flexible Tracks and Gutters</title>
+<link rel="author" title="Manuel Rego Casasnovas" href="mailto:rego@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css-grid/#gutters">
+<link rel="help" href="https://drafts.csswg.org/css-grid/#algo-find-fr-size">
+<meta name="assert" content="This test checks that the size of flexible tracks is properly computed when the grid container is content based and you have items spanning flexbile tracks that are smaller than the gutter sizes.">
+<style>
+.grid {
+  position: relative;
+  display: inline-grid;
+  grid-gap: 50px 100px;
+  font: 10px/1 Ahem;
+  margin: 50px;
+}
+
+.grid div:nth-child(1) { background: magenta; }
+</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>
+
+<div class="grid" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-column: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="200" data-expected-height="50">X</div>
+</div>
+
+<div class="grid" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-row: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="100">X</div>
+</div>
+
+<div class="grid" style="grid: 50px 1fr / 100px 1fr;">
+  <div style="grid-column: span 2; grid-row: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="200" data-expected-height="100">X</div>
+</div>
+
+<div class="grid" style="grid: 0px 1fr / 0px 1fr;">
+  <div style="grid-column: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="0">X</div>
+</div>
+
+<div class="grid" style="grid: 0px 1fr / 0px 1fr;">
+  <div style="grid-row: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="0" data-expected-height="50">X</div>
+</div>
+
+<div class="grid" style="grid: 0px 1fr / 0px 1fr;">
+  <div style="grid-column: span 2; grid-row: span 2;"
+    data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="50">X</div>
+</div>
index 722d3cc..9cbe118 100644 (file)
@@ -14,6 +14,8 @@ Property values requiring vendor prefixes:
 None
 ------------------------------------------------------------------------
 List of files:
+/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001.html
+/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002.html
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-layout-free-space-unit-expected.html
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-layout-free-space-unit.html
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-stretch-respects-min-size-001-expected.xht
index 6b57da3..4a5a68b 100644 (file)
@@ -1,3 +1,32 @@
+2018-01-22  Manuel Rego Casasnovas  <rego@igalia.com>
+
+        [css-grid] Spanning Grid item has too much space at the bottom / is too high
+        https://bugs.webkit.org/show_bug.cgi?id=181677
+
+        Reviewed by Javier Fernandez.
+
+        In IndefiniteSizeStrategy::findUsedFlexFraction() we were not
+        subtracting the size of the gutters when we call findFrUnitSize().
+        If an item spans several tracks, we cannot pass the maxContentForChild()
+        directly, we need to subtract the gutters as they are treated
+        as fixed size tracks in the algorithm.
+
+        The spec text is pretty clear regarding this
+        (https://drafts.csswg.org/css-grid/#algo-find-fr-size):
+        "Let leftover space be the space to fill minus the base sizes
+         of the non-flexible grid tracks."
+
+        Gutters are treated as fixed-size tracks for the purpose
+        of the track sizing algorithm, so we need to subtract them from the
+        leftover space while finding the size of an "fr".
+
+        Tests: imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-001.html
+               imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-find-fr-size-gutters-002.html
+
+        * rendering/GridTrackSizingAlgorithm.cpp:
+        (WebCore::GridTrackSizingAlgorithm::findFrUnitSize const):
+        (WebCore::IndefiniteSizeStrategy::findUsedFlexFraction const):
+
 2018-01-21  Ryosuke Niwa  <rniwa@webkit.org>
 
         Turning off custom pasteboard data doesn't actually turn it off in WK2
index b599e1b..2ccf470 100644 (file)
@@ -683,6 +683,7 @@ double GridTrackSizingAlgorithm::findFrUnitSize(const GridSpan& tracksSpan, Layo
             flexFactorSum += flexFactor;
         }
     }
+    // We don't remove the gutters from left_over_space here, because that was already done before.
 
     // The function is not called if we don't have <flex> grid tracks.
     ASSERT(!flexibleTracksIndexes.isEmpty());
@@ -848,7 +849,9 @@ double IndefiniteSizeStrategy::findUsedFlexFraction(Vector<unsigned>& flexibleSi
             if (i > 0 && span.startLine() <= flexibleSizedTracksIndex[i - 1])
                 continue;
 
-            flexFraction = std::max(flexFraction, findFrUnitSize(span, maxContentForChild(*gridItem)));
+            // Removing gutters from the max-content contribution of the item, so they are not taken into account in FindFrUnitSize().
+            LayoutUnit leftOverSpace = maxContentForChild(*gridItem) - renderGrid()->guttersSize(m_algorithm.grid(), direction, span.startLine(), span.integerSpan(), availableSpace());
+            flexFraction = std::max(flexFraction, findFrUnitSize(span, leftOverSpace));
         }
     }