Changeset 225776 in webkit


Ignore:
Timestamp:
Dec 11, 2017 11:11:04 PM (6 years ago)
Author:
Manuel Rego Casasnovas
Message:

[css-grid] Automatic minimum size is not clamped if min track sizing function is auto
​https://bugs.webkit.org/show_bug.cgi?id=180283

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Update expected result in the following WPT tests, as now everything
is passing there.

  • web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-022-expected.txt:
  • web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-023-expected.txt:

Source/WebCore:

We were not clamping the automatic minimum size when
the min track sizing function was intrinsic (e.g. minmax(auto, 0px)).
However the spec (​https://drafts.csswg.org/css-grid/#min-size-auto)
is very clear regarding that.

This patch modifies
GridTrackSizingAlgorithm::sizeTrackToFitNonSpanningItem(),
so in the case of a fixed max track sizing function it clamps
the automatic minimum size of the item to the stretch fit
of the grid area's size.
It needs to take into account if the item has fixed size, margin, border
and/or padding as those cannot be clamped.

Using WPT tests to verify this behavior,
and corrected a bunch of other tests that were wrong.

Test: imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-017.html

imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-022.html
imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-023.html

  • rendering/GridTrackSizingAlgorithm.cpp:

(WebCore::GridTrackSizingAlgorithm::sizeTrackToFitNonSpanningItem):

  • rendering/GridTrackSizingAlgorithm.h:

(WebCore::GridTrack::growthLimitIsInfinite const):

  • rendering/style/GridTrackSize.h:

(WebCore::GridTrackSize::cacheMinMaxTrackBreadthTypes):
(WebCore::GridTrackSize::hasFixedMaxTrackBreadth const):

LayoutTests:

This patch updates a bunch of tests that were wrong
to follow the new behavior.

  • TestExpectations: Now we're passing one WPT test more.
  • fast/css-grid-layout/min-height-border-box.html:
  • fast/css-grid-layout/min-width-margin-box.html:
  • fast/css-grid-layout/percent-of-indefinite-track-size-in-auto.html:
  • fast/css-grid-layout/percent-of-indefinite-track-size.html:
Location:
trunk
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r225764 r225776  
     12017-12-11  Manuel Rego Casasnovas  <rego@igalia.com>
     2
     3        [css-grid] Automatic minimum size is not clamped if min track sizing function is auto
     4        https://bugs.webkit.org/show_bug.cgi?id=180283
     5
     6        Reviewed by Darin Adler.
     7
     8        This patch updates a bunch of tests that were wrong
     9        to follow the new behavior.
     10
     11        * TestExpectations: Now we're passing one WPT test more.
     12        * fast/css-grid-layout/min-height-border-box.html:
     13        * fast/css-grid-layout/min-width-margin-box.html:
     14        * fast/css-grid-layout/percent-of-indefinite-track-size-in-auto.html:
     15        * fast/css-grid-layout/percent-of-indefinite-track-size.html:
     16
    1172017-12-11  Eric Carlson  <eric.carlson@apple.com>
    218

  • trunk/LayoutTests/TestExpectations

    r225741 r225776  
    479479webkit.org/b/165062 fast/css-grid-layout/grid-baseline-margins.html [ ImageOnlyFailure ]
    480480webkit.org/b/169271 imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-sizing-alignment-001.html [ ImageOnlyFailure ]
    481 webkit.org/b/180283 imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-017.html [ ImageOnlyFailure ]
    482481webkit.org/b/180290 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-gutters-001.html [ ImageOnlyFailure ]
    483482webkit.org/b/180290 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-gutters-002.html [ ImageOnlyFailure ]

  • trunk/LayoutTests/fast/css-grid-layout/min-height-border-box.html

    r200510 r225776  
    3030<h3>min-height: auto. grid height: auto</h3>
    3131<div class="container">
    32     <div class="grid" data-expected-width="150" data-expected-height="35">
    33         <div class="item" data-expected-width="140" data-expected-height="25">XXXX</div>
     32    <div class="grid" data-expected-width="150" data-expected-height="10">
     33        <div class="item" data-expected-width="140" data-expected-height="0">XXXX</div>
    3434    </div>
    3535</div>
    3636
    3737<div class="container">
    38     <div class="grid" data-expected-width="150" data-expected-height="77">
    39         <div class="item borderPaddingMargin" data-expected-width="116" data-expected-height="48">XXX</div>
     38    <div class="grid" data-expected-width="150" data-expected-height="52">
     39        <div class="item borderPaddingMargin" data-expected-width="116" data-expected-height="23">XXX</div>
    4040    </div>
    4141</div>

  • trunk/LayoutTests/fast/css-grid-layout/min-width-margin-box.html

    r200510 r225776  
    3939<div class="container">
    4040    <div class="grid" data-expected-width="150" data-expected-height="50">
    41         <div class="item" data-expected-width="100" data-expected-height="25">XXXX</div>
    42         <div class="stretchedItem" data-expected-width="100" data-expected-height="15"></div>
     41        <div class="item" data-expected-width="0" data-expected-height="25">XXXX</div>
     42        <div class="stretchedItem" data-expected-width="0" data-expected-height="15"></div>
    4343    </div>
    4444</div>
    … …  
    4646<div class="container">
    4747    <div class="grid" data-expected-width="150" data-expected-height="92">
    48         <div class="item borderPaddingMargin" data-expected-width="141" data-expected-height="48">XXXX</div>
    49         <div class="stretchedItem" data-expected-width="165" data-expected-height="15"></div>
     48        <div class="item borderPaddingMargin" data-expected-width="41" data-expected-height="48">XXXX</div>
     49        <div class="stretchedItem" data-expected-width="65" data-expected-height="15"></div>
    5050    </div>
    5151</div>

  • trunk/LayoutTests/fast/css-grid-layout/percent-of-indefinite-track-size-in-auto.html

    r200510 r225776  
    4040</div>
    4141<div style="position: relative;">
    42     <div class="grid min-content gridWithPercentInMinOfMinMax" data-expected-width="30" data-expected-height="20">
     42    <div class="grid min-content gridWithPercentInMinOfMinMax" data-expected-width="30" data-expected-height="15">
    4343        <div class="firstRowFirstColumn">XXX</div>
    4444        <div class="firstRowFirstColumn">XX XX</div>

  • trunk/LayoutTests/fast/css-grid-layout/percent-of-indefinite-track-size.html

    r200510 r225776  
    4040</div>
    4141<div style="position: relative;">
    42     <div class="grid min-content gridWithPercentInMinOfMinMax" data-expected-width="30" data-expected-height="20">
     42    <div class="grid min-content gridWithPercentInMinOfMinMax" data-expected-width="30" data-expected-height="15">
    4343        <div class="firstRowFirstColumn">XXX</div>
    4444        <div class="firstRowFirstColumn">XX XX</div>

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r225741 r225776  
     12017-12-11  Manuel Rego Casasnovas  <rego@igalia.com>
     2
     3        [css-grid] Automatic minimum size is not clamped if min track sizing function is auto
     4        https://bugs.webkit.org/show_bug.cgi?id=180283
     5
     6        Reviewed by Darin Adler.
     7
     8        Update expected result in the following WPT tests, as now everything
     9        is passing there.
     10
     11        * web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-022-expected.txt:
     12        * web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-023-expected.txt:
     13
    1142017-12-11  Manuel Rego Casasnovas  <rego@igalia.com>
    215

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-022-expected.txt

    r225561 r225776  
    1010PASS .grid 9
    1111PASS .grid 10
    12 FAIL .grid 11 assert_equals:
    13 <div class="grid" style="grid-template-columns: minmax(auto, 25px);">
    14   <div data-expected-width="25">XXXXXXXXXX</div>
    15   <div data-expected-width="25"></div>
    16 </div>
    17 width expected 25 but got 100
    18 FAIL .grid 12 assert_equals:
    19 <div class="grid" style="grid-template-columns: minmax(auto, 0px);">
    20   <div data-expected-width="0">XXXXXXXXXX</div>
    21   <div data-expected-width="0"></div>
    22 </div>
    23 width expected 0 but got 100
     12PASS .grid 11
     13PASS .grid 12
    2414PASS .grid 13
    2515PASS .grid 14

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-023-expected.txt

    r225561 r225776  
    1010PASS .grid 9
    1111PASS .grid 10
    12 FAIL .grid 11 assert_equals:
    13 <div class="grid" style="grid-template-rows: minmax(auto, 25px);">
    14   <div class="verticalRL" data-expected-height="25">XXXXXXXXXX</div>
    15   <div data-expected-height="25"></div>
    16 </div>
    17 height expected 25 but got 100
    18 FAIL .grid 12 assert_equals:
    19 <div class="grid" style="grid-template-rows: minmax(auto, 0px);">
    20   <div class="verticalRL" data-expected-height="0">XXXXXXXXXX</div>
    21   <div data-expected-height="0"></div>
    22 </div>
    23 height expected 0 but got 100
     12PASS .grid 11
     13PASS .grid 12
    2414PASS .grid 13
    2515PASS .grid 14
    … …  
    3525PASS .grid 24
    3626PASS .grid 25
    37 FAIL .grid 26 assert_equals:
    38 <div class="grid" style="grid-template-rows: minmax(auto, 25px);">
    39   <div class="verticalLR" data-expected-height="25">XXXXXXXXXX</div>
    40   <div data-expected-height="25"></div>
    41 </div>
    42 height expected 25 but got 100
    43 FAIL .grid 27 assert_equals:
    44 <div class="grid" style="grid-template-rows: minmax(auto, 0px);">
    45   <div class="verticalLR" data-expected-height="0">XXXXXXXXXX</div>
    46   <div data-expected-height="0"></div>
    47 </div>
    48 height expected 0 but got 100
     27PASS .grid 26
     28PASS .grid 27
    4929PASS .grid 28
    5030PASS .grid 29

  • trunk/Source/WebCore/ChangeLog

    r225775 r225776  
     12017-12-11  Manuel Rego Casasnovas  <rego@igalia.com>
     2
     3        [css-grid] Automatic minimum size is not clamped if min track sizing function is auto
     4        https://bugs.webkit.org/show_bug.cgi?id=180283
     5
     6        Reviewed by Darin Adler.
     7
     8        We were not clamping the automatic minimum size when
     9        the min track sizing function was intrinsic (e.g. minmax(auto, 0px)).
     10        However the spec (https://drafts.csswg.org/css-grid/#min-size-auto)
     11        is very clear regarding that.
     12
     13        This patch modifies
     14        GridTrackSizingAlgorithm::sizeTrackToFitNonSpanningItem(),
     15        so in the case of a fixed max track sizing function it clamps
     16        the automatic minimum size of the item to the stretch fit
     17        of the grid area's size.
     18        It needs to take into account if the item has fixed size, margin, border
     19        and/or padding as those cannot be clamped.
     20
     21        Using WPT tests to verify this behavior,
     22        and corrected a bunch of other tests that were wrong.
     23
     24        Test: imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-017.html
     25              imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-022.html
     26              imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-minimum-size-grid-items-023.html
     27
     28        * rendering/GridTrackSizingAlgorithm.cpp:
     29        (WebCore::GridTrackSizingAlgorithm::sizeTrackToFitNonSpanningItem):
     30        * rendering/GridTrackSizingAlgorithm.h:
     31        (WebCore::GridTrack::growthLimitIsInfinite const):
     32        * rendering/style/GridTrackSize.h:
     33        (WebCore::GridTrackSize::cacheMinMaxTrackBreadthTypes):
     34        (WebCore::GridTrackSize::hasFixedMaxTrackBreadth const):
     35
    1362017-12-11  Zan Dobersek  <zdobersek@igalia.com>
    237

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

    r225741 r225776  
    236236    GridTrackSize trackSize = gridTrackSize(m_direction, trackPosition);
    237237
    238     if (trackSize.hasMinContentMinTrackBreadth())
     238    if (trackSize.hasMinContentMinTrackBreadth()) {
    239239        track.setBaseSize(std::max(track.baseSize(), m_strategy->minContentForChild(gridItem)));
    240     else if (trackSize.hasMaxContentMinTrackBreadth())
     240    } else if (trackSize.hasMaxContentMinTrackBreadth()) {
    241241        track.setBaseSize(std::max(track.baseSize(), m_strategy->maxContentForChild(gridItem)));
    242     else if (trackSize.hasAutoMinTrackBreadth())
    243         track.setBaseSize(std::max(track.baseSize(), m_strategy->minSizeForChild(gridItem)));
     242    } else if (trackSize.hasAutoMinTrackBreadth()) {
     243        auto minSize = m_strategy->minSizeForChild(gridItem);
     244        bool isRowAxis = m_direction == GridLayoutFunctions::flowAwareDirectionForChild(*m_renderGrid, gridItem, ForColumns);
     245        Length gridItemSize = isRowAxis ? gridItem.style().logicalWidth() : gridItem.style().logicalHeight();
     246        Length gridItemMinSize = isRowAxis ? gridItem.style().logicalMinWidth() : gridItem.style().logicalMinHeight();
     247        bool overflowIsVisible = isRowAxis ? gridItem.style().overflowInlineDirection() == OVISIBLE : gridItem.style().overflowBlockDirection() == OVISIBLE;
     248
     249        if (gridItemSize.isAuto() && gridItemMinSize.isAuto() && overflowIsVisible && trackSize.hasFixedMaxTrackBreadth()) {
     250            auto maxTrackBreadth = valueForLength(trackSize.maxTrackBreadth().length(), availableSpace().value_or(LayoutUnit()));
     251            if (minSize > maxTrackBreadth) {
     252                auto marginAndBorderAndPadding = GridLayoutFunctions::marginLogicalSizeForChild(*m_renderGrid, m_direction, gridItem);
     253                marginAndBorderAndPadding += isRowAxis ? gridItem.borderAndPaddingLogicalWidth() : gridItem.borderAndPaddingLogicalHeight();
     254                minSize = std::max(maxTrackBreadth, marginAndBorderAndPadding);
     255            }
     256        }
     257
     258        track.setBaseSize(std::max(track.baseSize(), minSize));
     259    }
    244260
    245261    if (trackSize.hasMinContentMaxTrackBreadth()) {

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

    r225741 r225776  
    5454
    5555    const LayoutUnit& growthLimit() const;
     56    bool growthLimitIsInfinite() const { return m_growthLimit == infinity; }
    5657    void setGrowthLimit(LayoutUnit);
    5758
    … …  
    7374
    7475private:
    75     bool growthLimitIsInfinite() const { return m_growthLimit == infinity; }
    7676    bool isGrowthLimitBiggerThanBaseSize() const { return growthLimitIsInfinite() || m_growthLimit >= m_baseSize; }
    7777

  • trunk/Source/WebCore/rendering/style/GridTrackSize.h

    r212629 r225776  
    103103        m_maxTrackBreadthIsMinContent = maxTrackBreadth().isLength() && maxTrackBreadth().length().isMinContent();
    104104        m_maxTrackBreadthIsAuto = maxTrackBreadth().isLength() && maxTrackBreadth().length().isAuto();
     105        m_maxTrackBreadthIsFixed = maxTrackBreadth().isLength() && maxTrackBreadth().length().isSpecified();
    105106
    106107        // These values depend on the above ones so keep them here.
    … …  
    124125    bool hasMaxContentMinTrackBreadthAndMaxContentMaxTrackBreadth() const { return m_minTrackBreadthIsMaxContent && m_maxTrackBreadthIsMaxContent; }
    125126    bool hasAutoOrMinContentMinTrackBreadthAndIntrinsicMaxTrackBreadth() const { return (m_minTrackBreadthIsMinContent || m_minTrackBreadthIsAuto) && m_maxTrackBreadthIsIntrinsic; }
     127    bool hasFixedMaxTrackBreadth() const { return m_maxTrackBreadthIsFixed; }
    126128
    127129private:
    … …  
    139141    bool m_minTrackBreadthIsIntrinsic : 1;
    140142    bool m_maxTrackBreadthIsIntrinsic : 1;
     143    bool m_maxTrackBreadthIsFixed : 1;
    141144};
    142145
Note: See TracChangeset for help on using the changeset viewer.