Changeset 275754 in webkit


Ignore:
Timestamp:
Apr 9, 2021 2:17:59 AM (16 months ago)
Author:
commit-queue@webkit.org
Message:

[css-grid] Fix min/max widths of grid affected by ancestor
​https://bugs.webkit.org/show_bug.cgi?id=222100

Patch by Ziran Sun <Ziran Sun> on 2021-04-09
LayoutTests/imported/w3c:

Reviewed by Javier Fernandez.

  • web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-002-expected.txt:
  • web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-lr-002-expected.txt:
  • web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-rl-002-expected.txt:

Source/WebCore:

Reviewed by Reviewed by Javier Fernandez.

It's a reland of r273435, which got reverted because it broke
imported/w3c/web-platform-tests/css/css-flexbox/percentage-max-height-003.html. This change
narrows down the fix specificallly to the case when logical-width is recomputed.
We need to recalculate min/max widths of child that depend on the ancestor.
Before update logical-width, for element that needs preferredWidth recalcution,
it is necessary to make sure that min/max widths are set dirty.

This change is an import of chromium CL at
​https://chromium-review.googlesource.com/c/chromium/src/+/527640/
Only the parts that apply to this issue are imported.

Tests were already imported in WPT.

  • rendering/RenderBlock.cpp:

(WebCore::shouldRecalculateMinMaxWidthsAffectedByAncestor):
(WebCore::RenderBlock::recomputeLogicalWidth):

Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r275667 r275754  
     12021-04-09  Ziran Sun  <zsun@igalia.com>
     2
     3        [css-grid] Fix min/max widths of grid affected by ancestor
     4        https://bugs.webkit.org/show_bug.cgi?id=222100
     5
     6        Reviewed by Javier Fernandez.
     7
     8        * web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-002-expected.txt:
     9        * web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-lr-002-expected.txt:
     10        * web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-rl-002-expected.txt:
     11
    1122021-04-08  Youenn Fablet  <youenn@apple.com>
    213

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-002-expected.txt

    r273606 r275754  
    11
    2 FAIL .grid 1 assert_equals:
    3 <div class="grid">
    4   <div class="paddingLeft50Percent" data-expected-padding-left="50" data-expected-width="60" data-expected-height="10">X</div>
    5   <div data-offset-x="0" data-offset-y="10" data-expected-width="100" data-expected-height="10"></div>
    6 </div>
    7 width expected 60 but got 50
    8 FAIL .grid 2 assert_equals:
    9 <div class="grid">
    10   <div class="paddingRight50Percent" data-expected-padding-right="50" data-expected-width="60" data-expected-height="10">X</div>
    11   <div data-offset-x="0" data-offset-y="10" data-expected-width="100" data-expected-height="10"></div>
    12 </div>
    13 width expected 60 but got 50
     2PASS .grid 1
     3PASS .grid 2
    144PASS .grid 3
    155PASS .grid 4
    16 FAIL .grid 5 assert_equals:
    17 <div class="grid directionRTL">
    18   <div class="paddingLeft50Percent" data-expected-padding-left="50" data-expected-width="60" data-expected-height="10">X</div>
    19   <div data-offset-x="400" data-offset-y="10" data-expected-width="100" data-expected-height="10"></div>
    20 </div>
    21 width expected 60 but got 50
    22 FAIL .grid 6 assert_equals:
    23 <div class="grid directionRTL">
    24   <div class="paddingRight50Percent" data-expected-padding-right="50" data-expected-width="60" data-expected-height="10">X</div>
    25   <div data-offset-x="400" data-offset-y="10" data-expected-width="100" data-expected-height="10"></div>
    26 </div>
    27 width expected 60 but got 50
     6PASS .grid 5
     7PASS .grid 6
    288PASS .grid 7
    299PASS .grid 8

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-lr-002-expected.txt

    r273606 r275754  
    22PASS .grid 1
    33PASS .grid 2
    4 FAIL .grid 3 assert_equals:
    5 <div class="grid">
    6   <div class="paddingTop50Percent" data-expected-padding-top="50" data-expected-width="10" data-expected-height="60">X</div>
    7   <div data-offset-x="10" data-offset-y="0" data-expected-width="10" data-expected-height="100"></div>
    8 </div>
    9 height expected 60 but got 50
    10 FAIL .grid 4 assert_equals:
    11 <div class="grid">
    12   <div class="paddingBottom50Percent" data-expected-padding-bottom="50" data-expected-width="10" data-expected-height="60">X</div>
    13   <div data-offset-x="10" data-offset-y="0" data-expected-width="10" data-expected-height="100"></div>
    14 </div>
    15 height expected 60 but got 50
     4PASS .grid 3
     5PASS .grid 4
    166PASS .grid 5
    177PASS .grid 6
    18 FAIL .grid 7 assert_equals:
    19 <div class="grid directionRTL">
    20   <div class="paddingTop50Percent" data-expected-padding-top="50" data-expected-width="10" data-expected-height="60">X</div>
    21   <div data-offset-x="10" data-offset-y="400" data-expected-width="10" data-expected-height="100"></div>
    22 </div>
    23 height expected 60 but got 50
    24 FAIL .grid 8 assert_equals:
    25 <div class="grid directionRTL">
    26   <div class="paddingBottom50Percent" data-expected-padding-bottom="50" data-expected-width="10" data-expected-height="60">X</div>
    27   <div data-offset-x="10" data-offset-y="400" data-expected-width="10" data-expected-height="100"></div>
    28 </div>
    29 height expected 60 but got 50
     8PASS .grid 7
     9PASS .grid 8
    3010Direction LTR
    3111

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-items/grid-items-percentage-paddings-vertical-rl-002-expected.txt

    r273606 r275754  
    22PASS .grid 1
    33PASS .grid 2
    4 FAIL .grid 3 assert_equals:
    5 <div class="grid">
    6   <div class="paddingTop50Percent" data-expected-padding-top="50" data-expected-width="10" data-expected-height="60">X</div>
    7   <div data-offset-x="0" data-offset-y="0" data-expected-width="10" data-expected-height="100"></div>
    8 </div>
    9 height expected 60 but got 50
    10 FAIL .grid 4 assert_equals:
    11 <div class="grid">
    12   <div class="paddingBottom50Percent" data-expected-padding-bottom="50" data-expected-width="10" data-expected-height="60">X</div>
    13   <div data-offset-x="0" data-offset-y="0" data-expected-width="10" data-expected-height="100"></div>
    14 </div>
    15 height expected 60 but got 50
     4PASS .grid 3
     5PASS .grid 4
    166PASS .grid 5
    177PASS .grid 6
    18 FAIL .grid 7 assert_equals:
    19 <div class="grid directionRTL">
    20   <div class="paddingTop50Percent" data-expected-padding-top="50" data-expected-width="10" data-expected-height="60">X</div>
    21   <div data-offset-x="0" data-offset-y="400" data-expected-width="10" data-expected-height="100"></div>
    22 </div>
    23 height expected 60 but got 50
    24 FAIL .grid 8 assert_equals:
    25 <div class="grid directionRTL">
    26   <div class="paddingBottom50Percent" data-expected-padding-bottom="50" data-expected-width="10" data-expected-height="60">X</div>
    27   <div data-offset-x="0" data-offset-y="400" data-expected-width="10" data-expected-height="100"></div>
    28 </div>
    29 height expected 60 but got 50
     8PASS .grid 7
     9PASS .grid 8
    3010Direction LTR
    3111

  • trunk/Source/WebCore/ChangeLog

    r275751 r275754  
     12021-04-09  Ziran Sun  <zsun@igalia.com>
     2
     3        [css-grid] Fix min/max widths of grid affected by ancestor
     4        https://bugs.webkit.org/show_bug.cgi?id=222100
     5
     6        Reviewed by Reviewed by Javier Fernandez.
     7
     8        It's a reland of r273435, which got reverted because it broke
     9        imported/w3c/web-platform-tests/css/css-flexbox/percentage-max-height-003.html. This change
     10        narrows down the fix specificallly to the case when logical-width is recomputed.
     11        We need to recalculate min/max widths of child that depend on the ancestor.
     12        Before update logical-width, for element that needs preferredWidth recalcution,
     13        it is necessary to make sure that min/max widths are set dirty.
     14
     15        This change is an import of chromium CL at
     16        https://chromium-review.googlesource.com/c/chromium/src/+/527640/
     17        Only the parts that apply to this issue are imported.
     18
     19        Tests were already imported in WPT.       
     20
     21        * rendering/RenderBlock.cpp:
     22        (WebCore::shouldRecalculateMinMaxWidthsAffectedByAncestor):
     23        (WebCore::RenderBlock::recomputeLogicalWidth):
     24
    1252021-04-09  Yusuke Suzuki  <ysuzuki@apple.com>
    226

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

    r275413 r275754  
    632632}
    633633
     634static bool shouldRecalculateMinMaxWidthsAffectedByAncestor(const RenderBox* box)
     635{
     636    // If the preferred widths are already dirty at this point (during layout), it actually means that we never need to calculate them, since that should
     637    // have been carried out by an ancestor that's sized based on preferred widths (a shrink-to-fit container, for instance). In such cases the
     638    // object will be left as dirty indefinitely, and it would just be a waste of time to calculate the preferred withs when nobody needs them.
     639    if (box->preferredLogicalWidthsDirty())
     640        return false;
     641    // If our containing block also has min/max widths that are affected by the ancestry, we have already dealt with this object as well. Avoid
     642    // unnecessary work and O(n^2) time complexity.
     643    if (const RenderBox* cb = box->containingBlock()) {
     644        if (cb->needsPreferredWidthsRecalculation() && !cb->preferredLogicalWidthsDirty())
     645            return false;
     646    }
     647    return true;
     648}
     649
    634650bool RenderBlock::recomputeLogicalWidth()
    635651{
    636652    LayoutUnit oldWidth = logicalWidth();
    637    
     653
     654    // Laying out this object means that its containing block is also being laid out. This object is special, in that its min/max widths depend on
     655    // the ancestry (min/max width calculation should ideally be strictly bottom-up, but that's not always the case), so since the containing
     656    // block size may have changed, we need to recalculate the min/max widths of this object, and every child that has the same issue, recursively.
     657    if (needsPreferredWidthsRecalculation() && shouldRecalculateMinMaxWidthsAffectedByAncestor(this))
     658        setPreferredLogicalWidthsDirty(true, MarkOnlyThis);
    638659    updateLogicalWidth();
    639660   
Note: See TracChangeset for help on using the changeset viewer.