Changeset 238491 in webkit


Ignore:
Timestamp:
Nov 26, 2018 4:09:04 AM (5 years ago)
Author:
jfernandez@igalia.com
Message:

[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.

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):

Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r238488 r238491  
     12018-11-26  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        [css-grid] absolute positioned child is sized wrongly when using auto-fit, generating spurious collapsed tracks
     4        https://bugs.webkit.org/show_bug.cgi?id=191938
     5
     6        Reviewed by Manuel Rego Casasnovas.
     7
     8        This change makes several cases of the following tests to pass now.
     9
     10        * web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-004-expected.txt:
     11        * web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-005-expected.txt:
     12        * web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-006-expected.txt:
     13        * web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-007-expected.txt:
     14
    1152018-11-26  Manuel Rego Casasnovas  <rego@igalia.com>
    216

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-004-expected.txt

    r232903 r238491  
    11
    2 FAIL .grid 1 assert_equals:
    3 <div class="container">
    4     <div class="grid">
    5         <span style="grid-column: 1 / 5" class="abs" data-expected-width="30" data-expected-height="10"></span>
    6         <span style="grid-column: 1" data-expected-width="30" data-expected-height="10"></span>
    7     </div>
    8 </div>
    9 width expected 30 but got 25
     2PASS .grid 1
    103
    114

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-005-expected.txt

    r232903 r238491  
    11
    2 FAIL .grid 1 assert_equals:
    3 <div class="container">
    4     <div class="grid">
    5         <span style="grid-column: 1 / 5" class="abs" data-expected-width="30" data-expected-height="10"></span>
    6         <span style="grid-column: 2" data-expected-width="30" data-expected-height="10"></span>
    7     </div>
    8 </div>
    9 width expected 30 but got 25
     2PASS .grid 1
    103
    114

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-006-expected.txt

    r232903 r238491  
    11
    2 FAIL .grid 1 assert_equals:
    3 <div class="container">
    4     <div class="grid">
    5         <span style="grid-column: 2 / 5" class="abs" data-expected-width="65" data-expected-height="10"></span>
    6         <span style="grid-column: 2 / 4" data-expected-width="65" data-expected-height="10"></span>
    7     </div>
    8 </div>
    9 width expected 65 but got 60
     2PASS .grid 1
    103
    114

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/abspos/grid-positioned-items-and-autofit-tracks-007-expected.txt

    r232903 r238491  
    11
    2 FAIL .grid 1 assert_equals:
    3 <div class="container">
    4     <div class="grid">
    5         <span style="grid-column: 2 / 5" class="abs" data-expected-width="65" data-expected-height="10"></span>
    6         <span style="grid-column: 2 / 4" data-expected-width="65" data-expected-height="10"></span>
    7     </div>
    8 </div>
    9 width expected 65 but got 60
     2PASS .grid 1
    103
    114

  • trunk/Source/WebCore/ChangeLog

    r238488 r238491  
     12018-11-26  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        [css-grid] absolute positioned child is sized wrongly when using auto-fit, generating spurious collapsed tracks
     4        https://bugs.webkit.org/show_bug.cgi?id=191938
     5
     6        Reviewed by Manuel Rego Casasnovas.
     7
     8        The guttersSize function has a complex logic to compute the gaps in a
     9        specific GridSpan, considering different scenarios of collapsed tracks
     10        for such span.
     11
     12        The first case is avoiding the duplicated gap because of trailing
     13        collapsed tracks.
     14
     15        The second case considered is looking for non-empty tracks before the
     16        GridSpan end, if it points to an empty track, so we must add this gap.
     17
     18        The last case is to consider the gap of non-empty tracks after the
     19        GridSpan end line, if it points to an empty track.
     20
     21        There are several cases that are not considered or incorrectly computed.
     22        This patch addresses those cases; basically, we should only consider gaps
     23        when there are non-empty tracks before and after the collapsed tracks.
     24        Additionally, we should avoid duplicating the gaps size adding both,
     25        before and after non-empty track's gap.
     26
     27        No new tests, this change is covered by current tests and make several cases to pass now.
     28
     29        * rendering/RenderGrid.cpp:
     30        (WebCore::RenderGrid::guttersSize const):
     31
    1322018-11-26  Manuel Rego Casasnovas  <rego@igalia.com>
    233

  • trunk/Source/WebCore/rendering/RenderGrid.cpp

    r238488 r238491  
    356356    // If the startLine is the start line of a collapsed track we need to go backwards till we reach
    357357    // a non collapsed track. If we find a non collapsed track we need to add that gap.
     358    size_t nonEmptyTracksBeforeStartLine = 0;
    358359    if (startLine && grid.isEmptyAutoRepeatTrack(direction, startLine)) {
    359         unsigned nonEmptyTracksBeforeStartLine = startLine;
     360        nonEmptyTracksBeforeStartLine = startLine;
    360361        auto begin = grid.autoRepeatEmptyTracks(direction)->begin();
    361362        for (auto it = begin; *it != startLine; ++it) {
    … …  
    378379            --nonEmptyTracksAfterEndLine;
    379380        }
    380         if (nonEmptyTracksAfterEndLine)
    381             gapAccumulator += gap;
     381        if (nonEmptyTracksAfterEndLine) {
     382            // We shouldn't count the gap twice if the span starts and ends in a collapsed track bewtween two non-empty tracks.
     383            if (!nonEmptyTracksBeforeStartLine)
     384                gapAccumulator += gap;
     385        } else if (nonEmptyTracksBeforeStartLine) {
     386            // 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).
     387            gapAccumulator -= gap;
     388        }
    382389    }
    383390
Note: See TracChangeset for help on using the changeset viewer.