Changeset 217705 in webkit


Ignore:
Timestamp:
Jun 2, 2017 2:08:15 AM (7 years ago)
Author:
jfernandez@igalia.com
Message:

[css-grid] Margin wrong applied when stretching an orthogonal item in fixed size track
​https://bugs.webkit.org/show_bug.cgi?id=172590

Reviewed by Sergio Villar Senin.

LayoutTests/imported/w3c:

All the test cases of these tests pass with this change, so updating their expectations accordingly.

  • web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-006-expected.txt:
  • web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-lr-006-expected.txt:
  • web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-rl-006-expected.txt:

Source/WebCore:

We need to consider orthogonality when using the item's logical margin to
compute the available space for stretching.

The issue this patch fixes is only reproducible when the grid layout logic
is executed several times, since probably the item doesn't need to be
laid out again. In such cases, we just get the cached logical margins
but we were not taking orthogonality into account.

Test: fast/css-grid-layout/grid-item-stretching-must-not-depend-on-previous-layouts.html

  • rendering/RenderGrid.cpp:

(WebCore::RenderGrid::layoutBlock):
(WebCore::RenderGrid::marginLogicalSizeForChild):
(WebCore::RenderGrid::availableAlignmentSpaceForChildBeforeStretching):

  • rendering/RenderGrid.h:

LayoutTests:

  • TestExpectations: 2 tests pass now but 3 more fail because of bug #172836
  • fast/css-grid-layout/grid-item-stretching-must-not-depend-on-previous-layouts-expected.html: Added.
  • fast/css-grid-layout/grid-item-stretching-must-not-depend-on-previous-layouts.html: Added.
Location:
trunk
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r217704 r217705  
     12017-06-02  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        [css-grid] Margin wrong applied when stretching an orthogonal item in fixed size track
     4        https://bugs.webkit.org/show_bug.cgi?id=172590
     5
     6        Reviewed by Sergio Villar Senin.
     7
     8        * TestExpectations: 2 tests pass now but 3 more fail because of bug #172836
     9        * fast/css-grid-layout/grid-item-stretching-must-not-depend-on-previous-layouts-expected.html: Added.
     10        * fast/css-grid-layout/grid-item-stretching-must-not-depend-on-previous-layouts.html: Added.
     11
    1122017-06-02  Zan Dobersek  <zdobersek@igalia.com>
    213

  • trunk/LayoutTests/TestExpectations

    r217670 r217705  
    285285webkit.org/b/165062 fast/css-grid-layout/grid-baseline.html [ ImageOnlyFailure ]
    286286webkit.org/b/165062 fast/css-grid-layout/grid-baseline-margins.html [ ImageOnlyFailure ]
    287 webkit.org/b/172590 imported/w3c/web-platform-tests/css/css-grid-1/abspos/orthogonal-positioned-grid-items-013.html [ ImageOnlyFailure ]
    288 webkit.org/b/172590 imported/w3c/web-platform-tests/css/css-grid-1/abspos/orthogonal-positioned-grid-items-017.html [ ImageOnlyFailure ]
    289287webkit.org/b/149891 imported/w3c/web-platform-tests/css/css-grid-1/grid-layout-properties.html [ Failure ]
    290288webkit.org/b/169271 imported/w3c/web-platform-tests/css/css-grid-1/grid-items/grid-items-sizing-alignment-001.html [ ImageOnlyFailure ]
     289webkit.org/b/172836 imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-014.html [ Failure ]
     290webkit.org/b/172836 imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-rl-014.html [ Failure ]
     291webkit.org/b/172836 imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-lr-014.html [ Failure ]
    291292
    292293# selectors4

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r217652 r217705  
     12017-06-02  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        [css-grid] Margin wrong applied when stretching an orthogonal item in fixed size track
     4        https://bugs.webkit.org/show_bug.cgi?id=172590
     5
     6        Reviewed by Sergio Villar Senin.
     7
     8        All the test cases of these tests pass with this change, so updating their expectations accordingly.
     9
     10        * web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-006-expected.txt:
     11        * web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-lr-006-expected.txt:
     12        * web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-rl-006-expected.txt:
     13
    1142017-06-01  Javier Fernandez  <jfernandez@igalia.com>
    215

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-006-expected.txt

    r217652 r217705  
    1010XX XXX
    1111
    12 FAIL .grid 1 assert_equals:
    13 <div class="grid">
    14   <div data-offset-x="0" data-offset-y="0" data-expected-width="90" data-expected-height="60" class="firstRowFirstColumn">X XX X</div>
    15   <div data-offset-x="100" data-offset-y="0" data-expected-width="40" data-expected-height="130" class="firstRowSecondColumn">XX X<br>X XXX<br>X<br>XX XXX</div>
    16   <div data-offset-x="0" data-offset-y="150" data-expected-width="10" data-expected-height="60" class="secondRowFirstColumn">X XX X</div>
    17   <div data-offset-x="100" data-offset-y="150" data-expected-width="130" data-expected-height="90" class="secondRowSecondColumn">XX X<br>X XXX<br>X<br>XX XXX</div>
    18 </div>
    19 width expected 90 but got 100
     12PASS .grid 1
    2013

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-lr-006-expected.txt

    r217652 r217705  
    1010XX XXX
    1111
    12 FAIL .grid 1 assert_equals:
    13 <div class="grid">
    14   <div data-offset-x="0" data-offset-y="0" data-expected-width="60" data-expected-height="90" class="firstRowFirstColumn">X XX X</div>
    15   <div data-offset-x="0" data-offset-y="100" data-expected-width="130" data-expected-height="40" class="firstRowSecondColumn">XX X<br>X XXX<br>X<br>XX XXX</div>
    16   <div data-offset-x="150" data-offset-y="0" data-expected-width="60" data-expected-height="10" class="secondRowFirstColumn">X XX X</div>
    17   <div data-offset-x="150" data-offset-y="100" data-expected-width="90" data-expected-height="130" class="secondRowSecondColumn">XX X<br>X XXX<br>X<br>XX XXX</div>
    18 </div>
    19 height expected 90 but got 100
     12PASS .grid 1
    2013

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid-1/alignment/grid-self-alignment-stretch-vertical-rl-006-expected.txt

    r217652 r217705  
    1010XX XXX
    1111
    12 FAIL .grid 1 assert_equals:
    13 <div class="grid">
    14   <div data-offset-x="190" data-offset-y="0" data-expected-width="60" data-expected-height="90" class="firstRowFirstColumn">X XX X</div>
    15   <div data-offset-x="100" data-offset-y="100" data-expected-width="130" data-expected-height="40" class="firstRowSecondColumn">XX X<br>X XXX<br>X<br>XX XXX</div>
    16   <div data-offset-x="40" data-offset-y="0" data-expected-width="60" data-expected-height="10" class="secondRowFirstColumn">X XX X</div>
    17   <div data-offset-x="0" data-offset-y="100" data-expected-width="90" data-expected-height="130" class="secondRowSecondColumn">XX X<br>X XXX<br>X<br>XX XXX</div>
    18 </div>
    19 height expected 90 but got 100
     12PASS .grid 1
    2013

  • trunk/Source/WebCore/ChangeLog

    r217702 r217705  
     12017-06-02  Javier Fernandez  <jfernandez@igalia.com>
     2
     3        [css-grid] Margin wrong applied when stretching an orthogonal item in fixed size track
     4        https://bugs.webkit.org/show_bug.cgi?id=172590
     5
     6        Reviewed by Sergio Villar Senin.
     7
     8        We need to consider orthogonality when using the item's logical margin to
     9        compute the available space for stretching.
     10
     11        The issue this patch fixes is only reproducible when the grid layout logic
     12        is executed several times, since probably the item doesn't need to be
     13        laid out again. In such cases, we just get the cached logical margins
     14        but we were not taking orthogonality into account.
     15
     16        Test: fast/css-grid-layout/grid-item-stretching-must-not-depend-on-previous-layouts.html
     17
     18        * rendering/RenderGrid.cpp:
     19        (WebCore::RenderGrid::layoutBlock):
     20        (WebCore::RenderGrid::marginLogicalSizeForChild):
     21        (WebCore::RenderGrid::availableAlignmentSpaceForChildBeforeStretching):
     22        * rendering/RenderGrid.h:
     23
    1242017-06-01  Carlos Garcia Campos  <cgarcia@igalia.com>
    225

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

    r217486 r217705  
    11361136}
    11371137
    1138 // FIXME: This logic is shared by RenderFlexibleBox, so it should be moved to RenderBox.
    1139 LayoutUnit RenderGrid::marginLogicalHeightForChild(const RenderBox& child) const
    1140 {
    1141     return isHorizontalWritingMode() ? child.verticalMarginExtent() : child.horizontalMarginExtent();
     1138LayoutUnit RenderGrid::marginLogicalSizeForChild(GridTrackSizingDirection direction, const RenderBox& child) const
     1139{
     1140    return flowAwareDirectionForChild(child, direction) == ForColumns ? child.marginLogicalWidth() : child.marginLogicalHeight();
    11421141}
    11431142
    … …  
    11621161    // children are laid out, so we can't use the child cached values. Hence, we need to
    11631162    // compute margins in order to determine the available height before stretching.
    1164     return gridAreaBreadthForChild - (child.needsLayout() ? computeMarginLogicalSizeForChild(ForRows, child) : marginLogicalHeightForChild(child));
     1163    GridTrackSizingDirection childBlockFlowDirection = flowAwareDirectionForChild(child, ForRows);
     1164    return gridAreaBreadthForChild - (child.needsLayout() ? computeMarginLogicalSizeForChild(childBlockFlowDirection, child) : marginLogicalSizeForChild(childBlockFlowDirection, child));
    11651165}
    11661166

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

    r215800 r217705  
    131131
    132132    void paintChildren(PaintInfo& forSelf, const LayoutPoint& paintOffset, PaintInfo& forChild, bool usePrintRect) override;
    133     LayoutUnit marginLogicalHeightForChild(const RenderBox&) const;
     133    LayoutUnit marginLogicalSizeForChild(GridTrackSizingDirection, const RenderBox&) const;
    134134    LayoutUnit computeMarginLogicalSizeForChild(GridTrackSizingDirection, const RenderBox&) const;
    135135    LayoutUnit availableAlignmentSpaceForChildBeforeStretching(LayoutUnit gridAreaBreadthForChild, const RenderBox&) const;
Note: See TracChangeset for help on using the changeset viewer.