[css-grid] absolute positioned child is sized wrongly when using auto-fit, generating...
authorjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Nov 2018 12:09:04 +0000 (12:09 +0000)
committerjfernandez@igalia.com <jfernandez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Nov 2018 12:09:04 +0000 (12:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191938

Reviewed by Manuel Rego Casasnovas.

LayoutTests/imported/w3c:

This change makes several cases of the following tests to pass now.

* web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-004-expected.txt:
* web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-005-expected.txt:
* web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-006-expected.txt:
* web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-007-expected.txt:

Source/WebCore:

The guttersSize function has a complex logic to compute the gaps in a
specific GridSpan, considering different scenarios of collapsed tracks
for such span.

The first case is avoiding the duplicated gap because of trailing
collapsed tracks.

The second case considered is looking for non-empty tracks before the
GridSpan end, if it points to an empty track, so we must add this gap.

The last case is to consider the gap of non-empty tracks after the
GridSpan end line, if it points to an empty track.

There are several cases that are not considered or incorrectly computed.
This patch addresses those cases; basically, we should only consider gaps
when there are non-empty tracks before and after the collapsed tracks.
Additionally, we should avoid duplicating the gaps size adding both,
before and after non-empty track's gap.

No new tests, this change is covered by current tests and make several cases to pass now.

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

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

LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-004-expected.txt
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-005-expected.txt
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-006-expected.txt
LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-007-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderGrid.cpp

index 128906b..cdc2f9b 100644 (file)
@@ -1,3 +1,17 @@
+2018-11-26  Javier Fernandez  <jfernandez@igalia.com>
+
+        [css-grid] absolute positioned child is sized wrongly when using auto-fit, generating spurious collapsed tracks
+        https://bugs.webkit.org/show_bug.cgi?id=191938
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        This change makes several cases of the following tests to pass now.
+
+        * web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-004-expected.txt:
+        * web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-005-expected.txt:
+        * web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-006-expected.txt:
+        * web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-007-expected.txt:
+
 2018-11-26  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-grid] Fix grid container sizing under min-content height
index 7da2ad6..f36da64 100644 (file)
@@ -1,11 +1,4 @@
 
-FAIL .grid 1 assert_equals: 
-<div class="container">
-    <div class="grid">
-        <span style="grid-column: 1 / 5" class="abs" data-expected-width="30" data-expected-height="10"></span>
-        <span style="grid-column: 1" data-expected-width="30" data-expected-height="10"></span>
-    </div>
-</div>
-width expected 30 but got 25
+PASS .grid 1 
 
 
index 95d81ce..f36da64 100644 (file)
@@ -1,11 +1,4 @@
 
-FAIL .grid 1 assert_equals: 
-<div class="container">
-    <div class="grid">
-        <span style="grid-column: 1 / 5" class="abs" data-expected-width="30" data-expected-height="10"></span>
-        <span style="grid-column: 2" data-expected-width="30" data-expected-height="10"></span>
-    </div>
-</div>
-width expected 30 but got 25
+PASS .grid 1 
 
 
index 209810d..f36da64 100644 (file)
@@ -1,11 +1,4 @@
 
-FAIL .grid 1 assert_equals: 
-<div class="container">
-    <div class="grid">
-        <span style="grid-column: 2 / 5" class="abs" data-expected-width="65" data-expected-height="10"></span>
-        <span style="grid-column: 2 / 4" data-expected-width="65" data-expected-height="10"></span>
-    </div>
-</div>
-width expected 65 but got 60
+PASS .grid 1 
 
 
index 209810d..f36da64 100644 (file)
@@ -1,11 +1,4 @@
 
-FAIL .grid 1 assert_equals: 
-<div class="container">
-    <div class="grid">
-        <span style="grid-column: 2 / 5" class="abs" data-expected-width="65" data-expected-height="10"></span>
-        <span style="grid-column: 2 / 4" data-expected-width="65" data-expected-height="10"></span>
-    </div>
-</div>
-width expected 65 but got 60
+PASS .grid 1 
 
 
index 919c37b..25d4452 100644 (file)
@@ -1,3 +1,34 @@
+2018-11-26  Javier Fernandez  <jfernandez@igalia.com>
+
+        [css-grid] absolute positioned child is sized wrongly when using auto-fit, generating spurious collapsed tracks
+        https://bugs.webkit.org/show_bug.cgi?id=191938
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        The guttersSize function has a complex logic to compute the gaps in a
+        specific GridSpan, considering different scenarios of collapsed tracks
+        for such span.
+
+        The first case is avoiding the duplicated gap because of trailing
+        collapsed tracks.
+
+        The second case considered is looking for non-empty tracks before the
+        GridSpan end, if it points to an empty track, so we must add this gap.
+
+        The last case is to consider the gap of non-empty tracks after the
+        GridSpan end line, if it points to an empty track.
+
+        There are several cases that are not considered or incorrectly computed.
+        This patch addresses those cases; basically, we should only consider gaps
+        when there are non-empty tracks before and after the collapsed tracks.
+        Additionally, we should avoid duplicating the gaps size adding both,
+        before and after non-empty track's gap.
+
+        No new tests, this change is covered by current tests and make several cases to pass now.
+
+        * rendering/RenderGrid.cpp:
+        (WebCore::RenderGrid::guttersSize const):
+
 2018-11-26  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-grid] Fix grid container sizing under min-content height
index 1580eca..56dc621 100644 (file)
@@ -355,8 +355,9 @@ LayoutUnit RenderGrid::guttersSize(const Grid& grid, GridTrackSizingDirection di
 
     // If the startLine is the start line of a collapsed track we need to go backwards till we reach
     // a non collapsed track. If we find a non collapsed track we need to add that gap.
+    size_t nonEmptyTracksBeforeStartLine = 0;
     if (startLine && grid.isEmptyAutoRepeatTrack(direction, startLine)) {
-        unsigned nonEmptyTracksBeforeStartLine = startLine;
+        nonEmptyTracksBeforeStartLine = startLine;
         auto begin = grid.autoRepeatEmptyTracks(direction)->begin();
         for (auto it = begin; *it != startLine; ++it) {
             ASSERT(nonEmptyTracksBeforeStartLine);
@@ -377,8 +378,14 @@ LayoutUnit RenderGrid::guttersSize(const Grid& grid, GridTrackSizingDirection di
             ASSERT(nonEmptyTracksAfterEndLine >= 1);
             --nonEmptyTracksAfterEndLine;
         }
-        if (nonEmptyTracksAfterEndLine)
-            gapAccumulator += gap;
+        if (nonEmptyTracksAfterEndLine) {
+            // We shouldn't count the gap twice if the span starts and ends in a collapsed track bewtween two non-empty tracks.
+            if (!nonEmptyTracksBeforeStartLine)
+                gapAccumulator += gap;
+        } else if (nonEmptyTracksBeforeStartLine) {
+            // We shouldn't count the gap if the the span starts and ends in a collapsed but there isn't non-empty tracks afterwards (it's at the end of the grid).
+            gapAccumulator -= gap;
+        }
     }
 
     return gapAccumulator;