Changeset 225741 in webkit


Ignore:
Timestamp:
Dec 11, 2017 1:11:19 AM (6 years ago)
Author:
Manuel Rego Casasnovas
Message:

REGRESSION(r221931): Row stretch doesn't work for grid container with min-height
​https://bugs.webkit.org/show_bug.cgi?id=180287

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

  • web-platform-tests/css/css-grid/grid-model/grid-box-sizing-001-expected.txt:

Update expectations as the test is now passing.

Source/WebCore:

In r221931 we moved the stretch phase as the last step of
the track sizing algorithm.
However this introduced a regression as we were no longer
taking into account the grid container min-width|height constraints
during this step.

The CSS WG modified the spec so it now defines what to do
in these situations (​https://drafts.csswg.org/css-grid/#algo-stretch):

"If the free space is indefinite, but the grid container

has a definite min-width/height, use that size to calculate
the free space for this step instead."

This patch adds a new method
GridTrackSizingAlgorithmStrategy::freeSpaceForStretchAutoTracksStep().
When we're in the DefiniteSizeStrategy it just returns the current
free space.
For the IndefiniteSizeStrategy in the columns case we don't need
any special computation (the same that happens in
recomputeUsedFlexFractionIfNeeded()); for rows it uses the min size
of the grid container (respecting min-width|height properties)
to calculate the free space.

Test: imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-stretch-respects-min-size-001.html

  • rendering/GridTrackSizingAlgorithm.cpp:

(WebCore::IndefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep const):
(WebCore::DefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep const):
(WebCore::GridTrackSizingAlgorithm::stretchAutoTracks):

  • rendering/GridTrackSizingAlgorithm.h:

LayoutTests:

from WPT is passing, so this patch removes it from TestExpectations file.

Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r225736 r225741  
     12017-12-11  Manuel Rego Casasnovas  <rego@igalia.com>
     2
     3        REGRESSION(r221931): Row stretch doesn't work for grid container with min-height
     4        https://bugs.webkit.org/show_bug.cgi?id=180287
     5
     6        Reviewed by Darin Adler.
     7
     8        * TestExpectations: Now layout-algorithm/grid-stretch-respects-min-size-001.html
     9        from WPT is passing, so this patch removes it from TestExpectations file.
     10
    1112017-12-10  Minsheng Liu  <lambda@liu.ms>
    212

  • trunk/LayoutTests/TestExpectations

    r225702 r225741  
    480480webkit.org/b/169271 imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-sizing-alignment-001.html [ ImageOnlyFailure ]
    481481webkit.org/b/180283 imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-017.html [ ImageOnlyFailure ]
    482 webkit.org/b/180287 imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-stretch-respects-min-size-001.html [ ImageOnlyFailure ]
    483482webkit.org/b/180290 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-gutters-001.html [ ImageOnlyFailure ]
    484483webkit.org/b/180290 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-gutters-002.html [ ImageOnlyFailure ]

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r225718 r225741  
     12017-12-11  Manuel Rego Casasnovas  <rego@igalia.com>
     2
     3        REGRESSION(r221931): Row stretch doesn't work for grid container with min-height
     4        https://bugs.webkit.org/show_bug.cgi?id=180287
     5
     6        Reviewed by Darin Adler.
     7
     8        * web-platform-tests/css/css-grid/grid-model/grid-box-sizing-001-expected.txt:
     9        Update expectations as the test is now passing.
     10
    1112017-12-08  Chris Dumez  <cdumez@apple.com>
    212

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-model/grid-box-sizing-001-expected.txt

    r225561 r225741  
    6060  </div>
    6161height expected 56 but got 0
    62 FAIL .grid 13 assert_equals:
    63 <div class="grid wholeWidth" style="min-height: 100px; bottom: 0; box-sizing: border-box;" data-expected-width="200" data-expected-height="100">
    64     <div class="item" data-expected-width="154" data-expected-height="56"></div>
    65   </div>
    66 height expected 56 but got 0
     62PASS .grid 13
    6763PASS .grid 14
    68 FAIL .grid 15 assert_equals:
    69 <div class="grid wholeWidth" style="min-height: 100px; bottom: 0;" data-expected-width="200" data-expected-height="144">
    70     <div class="item" data-expected-width="154" data-expected-height="100"></div>
    71   </div>
    72 height expected 100 but got 0
     64PASS .grid 15
    7365PASS .grid 16
    7466FAIL .grid 17 assert_equals:
    … …  
    9284  </div>
    9385height expected 56 but got 0
    94 FAIL .grid 21 assert_equals:
    95 <div class="grid wholeWidth" style="min-height: 100px; box-sizing: border-box;" data-expected-width="200" data-expected-height="100">
    96     <div class="item" data-expected-width="154" data-expected-height="56"></div>
    97   </div>
    98 height expected 56 but got 0
     86PASS .grid 21
    9987PASS .grid 22
    100 FAIL .grid 23 assert_equals:
    101 <div class="grid wholeWidth" style="min-height: 100px;" data-expected-width="200" data-expected-height="144">
    102     <div class="item" data-expected-width="154" data-expected-height="100"></div>
    103   </div>
    104 height expected 100 but got 0
     88PASS .grid 23
    10589PASS .grid 24
    10690

  • trunk/Source/WebCore/ChangeLog

    r225736 r225741  
     12017-12-11  Manuel Rego Casasnovas  <rego@igalia.com>
     2
     3        REGRESSION(r221931): Row stretch doesn't work for grid container with min-height
     4        https://bugs.webkit.org/show_bug.cgi?id=180287
     5
     6        Reviewed by Darin Adler.
     7
     8        In r221931 we moved the stretch phase as the last step of
     9        the track sizing algorithm.
     10        However this introduced a regression as we were no longer
     11        taking into account the grid container min-width|height constraints
     12        during this step.
     13
     14        The CSS WG modified the spec so it now defines what to do
     15        in these situations (https://drafts.csswg.org/css-grid/#algo-stretch):
     16          "If the free space is indefinite, but the grid container
     17           has a definite min-width/height, use that size to calculate
     18           the free space for this step instead."
     19
     20        This patch adds a new method
     21        GridTrackSizingAlgorithmStrategy::freeSpaceForStretchAutoTracksStep().
     22        When we're in the DefiniteSizeStrategy it just returns the current
     23        free space.
     24        For the IndefiniteSizeStrategy in the columns case we don't need
     25        any special computation (the same that happens in
     26        recomputeUsedFlexFractionIfNeeded()); for rows it uses the min size
     27        of the grid container (respecting min-width|height properties)
     28        to calculate the free space.
     29
     30        Test: imported/w3c/web-platform-tests/css/css-grid/layout-algorithm/grid-stretch-respects-min-size-001.html
     31
     32        * rendering/GridTrackSizingAlgorithm.cpp:
     33        (WebCore::IndefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep const):
     34        (WebCore::DefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep const):
     35        (WebCore::GridTrackSizingAlgorithm::stretchAutoTracks):
     36        * rendering/GridTrackSizingAlgorithm.h:
     37
    1382017-12-10  Minsheng Liu  <lambda@liu.ms>
    239

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

    r225104 r225741  
    790790    double findUsedFlexFraction(Vector<unsigned>& flexibleSizedTracksIndex, GridTrackSizingDirection, std::optional<LayoutUnit> freeSpace) const override;
    791791    bool recomputeUsedFlexFractionIfNeeded(double& flexFraction, LayoutUnit& totalGrowth) const override;
     792    LayoutUnit freeSpaceForStretchAutoTracksStep() const override;
    792793};
    793794
    … …  
    887888    double findUsedFlexFraction(Vector<unsigned>& flexibleSizedTracksIndex, GridTrackSizingDirection, std::optional<LayoutUnit> freeSpace) const override;
    888889    bool recomputeUsedFlexFractionIfNeeded(double& flexFraction, LayoutUnit& totalGrowth) const override;
     890    LayoutUnit freeSpaceForStretchAutoTracksStep() const override;
    889891};
     892
     893LayoutUnit IndefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep() const
     894{
     895    ASSERT(!m_algorithm.freeSpace(direction()));
     896    if (direction() == ForColumns)
     897        return LayoutUnit();
     898
     899    auto minSize = renderGrid()->computeContentLogicalHeight(MinSize, renderGrid()->style().logicalMinHeight(), std::nullopt);
     900    return minSize.value() - computeTrackBasedSize();
     901}
    890902
    891903LayoutUnit DefiniteSizeStrategy::minLogicalWidthForChild(RenderBox& child, Length childMinSize, LayoutUnit availableSize) const
    … …  
    925937    ASSERT(freeSpace);
    926938    return findFrUnitSize(allTracksSpan, freeSpace.value());
     939}
     940
     941LayoutUnit DefiniteSizeStrategy::freeSpaceForStretchAutoTracksStep() const
     942{
     943    return m_algorithm.freeSpace(direction()).value();
    927944}
    928945
    … …  
    10411058void GridTrackSizingAlgorithm::stretchAutoTracks()
    10421059{
    1043     auto currentFreeSpace = freeSpace(m_direction);
    1044     if (m_autoSizedTracksForStretchIndex.isEmpty()
    1045         || !currentFreeSpace
    1046         || currentFreeSpace.value() <= 0
     1060    auto currentFreeSpace = m_strategy->freeSpaceForStretchAutoTracksStep();
     1061    if (m_autoSizedTracksForStretchIndex.isEmpty() || currentFreeSpace <= 0
    10471062        || (m_renderGrid->contentAlignment(m_direction).distribution() != ContentDistributionStretch))
    10481063        return;
    … …  
    10501065    Vector<GridTrack>& allTracks = tracks(m_direction);
    10511066    unsigned numberOfAutoSizedTracks = m_autoSizedTracksForStretchIndex.size();
    1052     LayoutUnit sizeToIncrease = currentFreeSpace.value() / numberOfAutoSizedTracks;
     1067    LayoutUnit sizeToIncrease = currentFreeSpace / numberOfAutoSizedTracks;
    10531068    for (const auto& trackIndex : m_autoSizedTracksForStretchIndex) {
    10541069        auto& track = allTracks[trackIndex];

  • trunk/Source/WebCore/rendering/GridTrackSizingAlgorithm.h

    r223955 r225741  
    224224    virtual double findUsedFlexFraction(Vector<unsigned>& flexibleSizedTracksIndex, GridTrackSizingDirection, std::optional<LayoutUnit> initialFreeSpace) const = 0;
    225225    virtual bool recomputeUsedFlexFractionIfNeeded(double& flexFraction, LayoutUnit& totalGrowth) const = 0;
     226    virtual LayoutUnit freeSpaceForStretchAutoTracksStep() const = 0;
    226227
    227228protected:
Note: See TracChangeset for help on using the changeset viewer.