[css-grid] Fix auto repeat with multiple tracks and gutters
authorobrufau@igalia.com <obrufau@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 May 2020 20:24:52 +0000 (20:24 +0000)
committerobrufau@igalia.com <obrufau@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 May 2020 20:24:52 +0000 (20:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=182922

Reviewed by Manuel Rego Casasnovas.

LayoutTests/imported/w3c:

Import WPT test.

* web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001-expected.html: Added.
* web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001.html: Added.
* web-platform-tests/css/css-grid/grid-definition/w3c-import.log:

Source/WebCore:

The code that computes the number of auto repeat tracks wrongly assumes
that the second argument of the repeat() notation is a single track
function. That was true in the beginning, however specs were later on
modified to allow a <track-list>. We support a <track-list> as a second
argument since long ago but the code that computes the number of
auto-repeat tracks was never updated.

This patch modifies two places that relate to the gaps between the
auto-repeat tracks, which ensures the proper total length.

This is a port of https://crrev.com/620278 from Chromium.

Tests: fast/css-grid-layout/grid-auto-repeat-huge-grid.html
       imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001.html

* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::computeAutoRepeatTracksCount const):

LayoutTests:

Update test expectations.

* fast/css-grid-layout/grid-auto-repeat-huge-grid.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/css-grid-layout/grid-auto-repeat-huge-grid.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001-expected.html [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001.html [new file with mode: 0644]
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/w3c-import.log
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderGrid.cpp

index fb4e562..00ca514 100644 (file)
@@ -1,3 +1,14 @@
+2020-05-20  Oriol Brufau  <obrufau@igalia.com>
+
+        [css-grid] Fix auto repeat with multiple tracks and gutters
+        https://bugs.webkit.org/show_bug.cgi?id=182922
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        Update test expectations.
+
+        * fast/css-grid-layout/grid-auto-repeat-huge-grid.html:
+
 2020-05-20  Chris Dumez  <cdumez@apple.com>
 
         Disable support for BeforeLoadEvent
index 61b7e42..24827bf 100644 (file)
@@ -180,8 +180,8 @@ test(function() {
     autoFillGridElementCols.style.gridGap = "1000000px";
     autoFitGridElementCols.style.gridGap = "1000000px";
 
-    getTracksCheckingLength(autoFillGridElementCols, "grid-template-columns", 130, ["wideGrid", "lotsOfAutoRepeatWithAutoFillCols"]);
-    getTracksCheckingLength(autoFitGridElementCols, "grid-template-columns", 82, ["wideGrid", "lotsOfAutoRepeatWithAutoFitCols"]);
+    getTracksCheckingLength(autoFillGridElementCols, "grid-template-columns", 30, ["wideGrid", "lotsOfAutoRepeatWithAutoFillCols"]);
+    getTracksCheckingLength(autoFitGridElementCols, "grid-template-columns", 34, ["wideGrid", "lotsOfAutoRepeatWithAutoFitCols"]);
 
     autoFillGridElementCols.style.gridGap = "0px";
     autoFitGridElementCols.style.gridGap = "0px";
@@ -202,8 +202,8 @@ test(function() {
     autoFillGridElementRows.style.gridGap = "1000000px";
     autoFitGridElementRows.style.gridGap = "1000000px";
 
-    getTracksCheckingLength(autoFillGridElementRows, "grid-template-rows", 130, ["tallGrid", "lotsOfAutoRepeatWithAutoFillRows"]);
-    getTracksCheckingLength(autoFitGridElementRows, "grid-template-rows", 82, ["tallGrid", "lotsOfAutoRepeatWithAutoFitRows"]);
+    getTracksCheckingLength(autoFillGridElementRows, "grid-template-rows", 30, ["tallGrid", "lotsOfAutoRepeatWithAutoFillRows"]);
+    getTracksCheckingLength(autoFitGridElementRows, "grid-template-rows", 34, ["tallGrid", "lotsOfAutoRepeatWithAutoFitRows"]);
 
     autoFillGridElementRows.style.gridGap = "0px";
     autoFitGridElementRows.style.gridGap = "0px";
index d8c5c39..240cde1 100644 (file)
@@ -1,5 +1,18 @@
 2020-05-20  Oriol Brufau  <obrufau@igalia.com>
 
+        [css-grid] Fix auto repeat with multiple tracks and gutters
+        https://bugs.webkit.org/show_bug.cgi?id=182922
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        Import WPT test.
+
+        * web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001-expected.html: Added.
+        * web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001.html: Added.
+        * web-platform-tests/css/css-grid/grid-definition/w3c-import.log:
+
+2020-05-20  Oriol Brufau  <obrufau@igalia.com>
+
         Fix computeMarginLogicalSizeForChild to check auto margins in the right axis
         https://bugs.webkit.org/show_bug.cgi?id=212113
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001-expected.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001-expected.html
new file mode 100644 (file)
index 0000000..0b8ef6d
--- /dev/null
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+    <meta charset="utf-8">
+    <title>CSS Grid Layout Test: Auto Repaeat with Multiple Tracks and Gutters</title>
+    <link rel="author" title="Yu Shen" href="shenyu.tcv@gmail.com">
+    <style>
+        .match-container {
+            border: solid thick black;
+            position: relative;
+            margin: 10px;
+        }
+
+        .column {
+            width: 300px;
+        }
+
+        .row {
+            width: min-content;
+            height: 300px;
+        }
+
+        .item {
+            background: lime;
+            width: 50px;
+            height: 50px;
+        }
+
+        .column-second {
+            position: absolute;
+            top: 0;
+            left: 150px;
+        }
+
+        .row-second {
+            position: absolute;
+            top: 150px;
+            left: 0px;
+        }
+    </style>
+</head>
+
+<body>
+    <p>The test passes if it has the same visual effect as reference.</p>
+    <div class="match-container column">
+        <div class="item"></div>
+        <div class="item column-second"></div>
+    </div>
+    <div class="match-container row">
+        <div class="item"></div>
+        <div class="item row-second"></div>
+    </div>
+</body>
+
+</html>
\ No newline at end of file
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001.html
new file mode 100644 (file)
index 0000000..085d949
--- /dev/null
@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+    <meta charset="utf-8">
+    <title>CSS Grid Layout Test: Auto Repaeat with Multiple Tracks and Gutters</title>
+    <link rel="author" title="Yu Shen" href="shenyu.tcv@gmail.com">
+    <link rel="help" href="https://www.w3.org/TR/css-grid-1/#repeat-notation">
+    <link rel="match" href="../reference/grid-auto-repeat-multiple-values-001-ref.html">
+    <style>
+        .grid-container {
+            display: grid;
+            border: solid thick;
+            margin: 10px;
+        }
+
+        .columns {
+            grid-template-columns: repeat(auto-fill, 50px 50px);
+            grid-auto-rows: 25px;
+            grid-column-gap: 100px;
+            width: 300px;
+        }
+
+        .rows {
+            grid-auto-flow: column;
+            grid-template-rows: repeat(auto-fill, 50px 50px);
+            grid-auto-columns: 25px;
+            grid-row-gap: 100px;
+            width: min-content;
+            height: 300px;
+        }
+
+        .grid-container>div {
+            background: lime;
+        }
+    </style>
+</head>
+
+<body>
+    <p>The test passes if it has the same visual effect as reference.</p>
+    <div class="grid-container columns">
+        <div></div>
+        <div></div>
+        <div></div>
+        <div></div>
+    </div>
+    <div class="grid-container rows">
+        <div></div>
+        <div></div>
+        <div></div>
+        <div></div>
+    </div>
+</body>
+
+</html>
\ No newline at end of file
index 9d5da38..598630c 100644 (file)
@@ -23,6 +23,8 @@ List of files:
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-min-max-size-001.html
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-min-size-001.html
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-min-size-002.html
+/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001-expected.html
+/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001.html
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-change-auto-repeat-tracks.html
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-change-fit-content-argument-001.html
 /LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-inline-auto-repeat-001.html
index 4eeea5d..3dbeb12 100644 (file)
@@ -1,3 +1,28 @@
+2020-05-20  Oriol Brufau  <obrufau@igalia.com>
+
+        [css-grid] Fix auto repeat with multiple tracks and gutters
+        https://bugs.webkit.org/show_bug.cgi?id=182922
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        The code that computes the number of auto repeat tracks wrongly assumes
+        that the second argument of the repeat() notation is a single track
+        function. That was true in the beginning, however specs were later on
+        modified to allow a <track-list>. We support a <track-list> as a second
+        argument since long ago but the code that computes the number of
+        auto-repeat tracks was never updated.
+
+        This patch modifies two places that relate to the gaps between the
+        auto-repeat tracks, which ensures the proper total length.
+
+        This is a port of https://crrev.com/620278 from Chromium.
+
+        Tests: fast/css-grid-layout/grid-auto-repeat-huge-grid.html
+               imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-auto-repeat-multiple-values-001.html
+
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::computeAutoRepeatTracksCount const):
+
 2020-05-20  Simon Fraser  <simon.fraser@apple.com>
 
         Plumb the display's nominal refresh rate down to ScrollingTree for use in scroll synchronization
index 095004d..421219d 100644 (file)
@@ -507,16 +507,16 @@ unsigned RenderGrid::computeAutoRepeatTracksCount(GridTrackSizingDirection direc
         tracksSize += valueForLength(hasDefiniteMaxTrackBreadth ? track.maxTrackBreadth().length() : track.minTrackBreadth().length(), availableSize.value());
     }
 
-    // Add gutters as if there where only 1 auto repeat track. Gaps between auto repeat tracks will be added later when
-    // computing the repetitions.
+    // Add gutters as if auto repeat tracks were only repeated once. Gaps between different repetitions will be added later when
+    // computing the number of repetitions of the auto repeat().
     LayoutUnit gapSize = gridGap(direction, availableSize);
-    tracksSize += gapSize * trackSizes.size();
+    tracksSize += gapSize * (trackSizes.size() + autoRepeatTrackListLength - 1);
 
     LayoutUnit freeSpace = availableSize.value() - tracksSize;
     if (freeSpace <= 0)
         return autoRepeatTrackListLength;
 
-    LayoutUnit autoRepeatSizeWithGap = autoRepeatTracksSize + gapSize;
+    LayoutUnit autoRepeatSizeWithGap = autoRepeatTracksSize + gapSize * autoRepeatTrackListLength;
     unsigned repetitions = 1 + (freeSpace / autoRepeatSizeWithGap).toUnsigned();
     freeSpace -= autoRepeatSizeWithGap * (repetitions - 1);
     ASSERT(freeSpace >= 0);