Title: [275908] trunk
Revision
275908
Author
[email protected]
Date
2021-04-13 13:52:00 -0700 (Tue, 13 Apr 2021)

Log Message

[css-grid] Incorrect track sizing when using relative sized items in 'auto' column tracks
https://bugs.webkit.org/show_bug.cgi?id=191627

Patch by Ziran Sun <[email protected]> on 2021-04-13
Reviewed by Javier Fernandez.

LayoutTests/imported/w3c:

* web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001-expected.txt:

Source/WebCore:

This is a reland of r274596. The original patch was reverted because Google logo is not visible
on google.com.

Containing block for a grid item is the grid area it's located in. During the GridTrackSizingAlgorithm,
we determine that this item has a relative size (percent in this case) and that the grid area is still
indefinite. Hence, we set nullopt for the grid area width and mark the item for layout. During the item's
layout, we call this RenderBox::containingBlockLogicalWidthForContent to determine the available height.
However, this method checks the containing block's width if the overridingContentLogicalWidth is nullopt,
which is incorrect for grid items. We should never use the grid container for resolving grid item's size.

This change is to return 0_lu for grid item when overridingContainingBlockContentLogicalWidth() returns
nullopt rather than use containing block's width. It also comments that there might be an issue with
RenderBox::containingBlockLogicalHeightForContent for grid item. It should return whatever value we get from
overridingContainingBlockContentLogicalHeight(), including WTF::nullopt.

* rendering/RenderBox.cpp:
(WebCore::RenderBox::containingBlockLogicalWidthForContent const):
(WebCore::RenderBox::containingBlockLogicalHeightForContent const):

LayoutTests:

* TestExpectations:
* platform/ios-simulator-wk2/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001-expected.txt:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (275907 => 275908)


--- trunk/LayoutTests/ChangeLog	2021-04-13 20:38:03 UTC (rev 275907)
+++ trunk/LayoutTests/ChangeLog	2021-04-13 20:52:00 UTC (rev 275908)
@@ -1,3 +1,13 @@
+2021-04-13  Ziran Sun  <[email protected]>
+
+        [css-grid] Incorrect track sizing when using relative sized items in 'auto' column tracks
+        https://bugs.webkit.org/show_bug.cgi?id=191627
+
+        Reviewed by Javier Fernandez.
+
+        * TestExpectations:
+        * platform/ios-simulator-wk2/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001-expected.txt:
+
 2021-04-13  Kate Cheney  <[email protected]>
 
         http/.../contentSecurityPolicy/report-document-uri-after-blocked-redirect.html is a flaky failure

Modified: trunk/LayoutTests/TestExpectations (275907 => 275908)


--- trunk/LayoutTests/TestExpectations	2021-04-13 20:38:03 UTC (rev 275907)
+++ trunk/LayoutTests/TestExpectations	2021-04-13 20:52:00 UTC (rev 275908)
@@ -1211,7 +1211,6 @@
 webkit.org/b/216145 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-8.html [ ImageOnlyFailure ]
 webkit.org/b/216145 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-9.html [ ImageOnlyFailure ]
 webkit.org/b/216145 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-10.html [ ImageOnlyFailure ]
-webkit.org/b/191627 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001.html [ Failure ]
 webkit.org/b/149890 fast/css-grid-layout/grid-shorthands-style-format.html [ Failure ]
 webkit.org/b/191508 fast/css-grid-layout/crash-large-positions.html [ Skip ]
 

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (275907 => 275908)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-04-13 20:38:03 UTC (rev 275907)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-04-13 20:52:00 UTC (rev 275908)
@@ -1,3 +1,12 @@
+2021-04-13  Ziran Sun  <[email protected]>
+
+        [css-grid] Incorrect track sizing when using relative sized items in 'auto' column tracks
+        https://bugs.webkit.org/show_bug.cgi?id=191627
+
+        Reviewed by Javier Fernandez.
+
+        * web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001-expected.txt:
+
 2021-04-13  Antoine Quint  <[email protected]>
 
         Unreviewed. Upstream a test.

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001-expected.txt (275907 => 275908)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001-expected.txt	2021-04-13 20:38:03 UTC (rev 275907)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001-expected.txt	2021-04-13 20:52:00 UTC (rev 275908)
@@ -24,46 +24,10 @@
 PASS .inline-grid 4
 PASS .inline-grid 5
 PASS .inline-grid 6
-FAIL .inline-grid 7 assert_equals:
-<div class="inline-grid justifyItemsBaseline rows">
-        <div class="firstRowFirstColumn verticalRL width50" data-offset-x="0" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
-        <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="0" data-offset-y="100" data-expected-width="100" data-expected-height="100"></div>
-        <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="50" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
-    </div>
-width expected 100 but got 700
-FAIL .inline-grid 8 assert_equals:
-<div class="inline-grid justifyItemsBaseline rows min-content-columns">
-        <div class="firstRowFirstColumn verticalRL" data-offset-x="0" data-offset-y="0" data-expected-width="80" data-expected-height="100">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
-        <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="0" data-offset-y="100" data-expected-width="160" data-expected-height="100"></div>
-        <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="80" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
-    </div>
-width expected 160 but got 940
-FAIL .inline-grid 9 assert_equals:
-<div class="inline-grid justifyItemsBaseline rows max-content-columns">
-        <div class="firstRowFirstColumn verticalRL" data-offset-x="0" data-offset-y="0" data-expected-width="80" data-expected-height="100">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
-        <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="0" data-offset-y="100" data-expected-width="160" data-expected-height="100"></div>
-        <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="80" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
-    </div>
-width expected 160 but got 940
-FAIL .inline-grid 10 assert_equals:
-<div class="inline-grid justifyItemsBaseline rows fit-content-columns">
-        <div class="firstRowFirstColumn verticalRL" data-offset-x="0" data-offset-y="0" data-expected-width="80" data-expected-height="100">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
-        <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="0" data-offset-y="100" data-expected-width="160" data-expected-height="100"></div>
-        <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="80" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
-    </div>
-width expected 160 but got 940
-FAIL .inline-grid 11 assert_equals:
-<div class="inline-grid justifyItemsBaseline rows flex-columns">
-        <div class="firstRowFirstColumn verticalRL width50" data-offset-x="0" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
-        <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="0" data-offset-y="100" data-expected-width="100" data-expected-height="100"></div>
-        <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="50" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
-    </div>
-width expected 100 but got 700
-FAIL .inline-grid 12 assert_equals:
-<div class="inline-grid justifyItemsBaseline rows max-flex-columns">
-        <div class="firstRowFirstColumn verticalRL width50" data-offset-x="0" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
-        <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="-50" data-offset-y="100" data-expected-width="100" data-expected-height="100"></div>
-        <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="50" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
-    </div>
-offsetLeft expected 0 but got 100
+PASS .inline-grid 7
+PASS .inline-grid 8
+PASS .inline-grid 9
+PASS .inline-grid 10
+PASS .inline-grid 11
+PASS .inline-grid 12
 

Modified: trunk/LayoutTests/platform/ios-simulator-wk2/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001-expected.txt (275907 => 275908)


--- trunk/LayoutTests/platform/ios-simulator-wk2/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001-expected.txt	2021-04-13 20:38:03 UTC (rev 275907)
+++ trunk/LayoutTests/platform/ios-simulator-wk2/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-001-expected.txt	2021-04-13 20:52:00 UTC (rev 275908)
@@ -18,28 +18,39 @@
  flex max-function columns - items with relative width
 baseline is not applied initially, but orthogonal items force repeating the track sizing and height is not indefinite in that phase.
 
-FAIL .inline-grid 1 assert_equals:
-<div class="inline-grid justifyItemsBaseline rows max-flex-columns">
-        <div class="firstRowFirstColumn verticalRL width50" data-offset-x="0" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
-        <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="-50" data-offset-y="100" data-expected-width="100" data-expected-height="100"></div>
-        <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="50" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
-    </div>
-offsetLeft expected -50 but got 0
+PASS .inline-grid 1
 FAIL .inline-grid 2 assert_equals:
-<div class="inline-grid justifyItemsBaseline rows flex-columns">
-        <div class="firstRowFirstColumn verticalRL width50" data-offset-x="0" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
-        <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="0" data-offset-y="100" data-expected-width="100" data-expected-height="100"></div>
-        <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="50" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
+<div class="inline-grid alignItemsBaseline columns min-content-rows">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="80">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
+        <div class="firstRowSecondColumn height200Percent" data-offset-x="100" data-offset-y="0" data-expected-width="100" data-expected-height="160"></div>
+        <div class="autoRowAutoColumnSpanning2 height25" data-offset-x="0" data-offset-y="80" data-expected-width="200" data-expected-height="25"></div>
     </div>
-width expected 100 but got 300
+height expected 80 but got 85
 FAIL .inline-grid 3 assert_equals:
-<div class="inline-grid justifyItemsBaseline rows fit-content-columns">
+<div class="inline-grid alignItemsBaseline columns max-content-rows">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="80">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
+        <div class="firstRowSecondColumn height200Percent" data-offset-x="100" data-offset-y="0" data-expected-width="100" data-expected-height="160"></div>
+        <div class="autoRowAutoColumnSpanning2 height25" data-offset-x="0" data-offset-y="80" data-expected-width="200" data-expected-height="25"></div>
+    </div>
+height expected 80 but got 85
+FAIL .inline-grid 4 assert_equals:
+<div class="inline-grid alignItemsBaseline columns fit-content-rows">
+        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="80">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
+        <div class="firstRowSecondColumn height200Percent" data-offset-x="100" data-offset-y="0" data-expected-width="100" data-expected-height="160"></div>
+        <div class="autoRowAutoColumnSpanning2 height25" data-offset-x="0" data-offset-y="80" data-expected-width="200" data-expected-height="25"></div>
+    </div>
+height expected 80 but got 85
+PASS .inline-grid 5
+PASS .inline-grid 6
+PASS .inline-grid 7
+FAIL .inline-grid 8 assert_equals:
+<div class="inline-grid justifyItemsBaseline rows min-content-columns">
         <div class="firstRowFirstColumn verticalRL" data-offset-x="0" data-offset-y="0" data-expected-width="80" data-expected-height="100">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
         <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="0" data-offset-y="100" data-expected-width="160" data-expected-height="100"></div>
         <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="80" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
     </div>
 width expected 80 but got 85
-FAIL .inline-grid 4 assert_equals:
+FAIL .inline-grid 9 assert_equals:
 <div class="inline-grid justifyItemsBaseline rows max-content-columns">
         <div class="firstRowFirstColumn verticalRL" data-offset-x="0" data-offset-y="0" data-expected-width="80" data-expected-height="100">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
         <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="0" data-offset-y="100" data-expected-width="160" data-expected-height="100"></div>
@@ -46,42 +57,13 @@
         <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="80" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
     </div>
 width expected 80 but got 85
-FAIL .inline-grid 5 assert_equals:
-<div class="inline-grid justifyItemsBaseline rows min-content-columns">
+FAIL .inline-grid 10 assert_equals:
+<div class="inline-grid justifyItemsBaseline rows fit-content-columns">
         <div class="firstRowFirstColumn verticalRL" data-offset-x="0" data-offset-y="0" data-expected-width="80" data-expected-height="100">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
         <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="0" data-offset-y="100" data-expected-width="160" data-expected-height="100"></div>
         <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="80" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
     </div>
 width expected 80 but got 85
-FAIL .inline-grid 6 assert_equals:
-<div class="inline-grid justifyItemsBaseline rows">
-        <div class="firstRowFirstColumn verticalRL width50" data-offset-x="0" data-offset-y="0" data-expected-width="50" data-expected-height="100"></div>
-        <div class="secondRowFirstColumn verticalRL width200Percent" data-offset-x="0" data-offset-y="100" data-expected-width="100" data-expected-height="100"></div>
-        <div class="firstRowSpanning2AutoColumn verticalRL width25" data-offset-x="50" data-offset-y="0" data-expected-width="25" data-expected-height="200"></div>
-    </div>
-width expected 100 but got 300
-PASS .inline-grid 7
-PASS .inline-grid 8
-FAIL .inline-grid 9 assert_equals:
-<div class="inline-grid alignItemsBaseline columns fit-content-rows">
-        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="80">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
-        <div class="firstRowSecondColumn height200Percent" data-offset-x="100" data-offset-y="0" data-expected-width="100" data-expected-height="160"></div>
-        <div class="autoRowAutoColumnSpanning2 height25" data-offset-x="0" data-offset-y="80" data-expected-width="200" data-expected-height="25"></div>
-    </div>
-height expected 80 but got 85
-FAIL .inline-grid 10 assert_equals:
-<div class="inline-grid alignItemsBaseline columns max-content-rows">
-        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="80">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
-        <div class="firstRowSecondColumn height200Percent" data-offset-x="100" data-offset-y="0" data-expected-width="100" data-expected-height="160"></div>
-        <div class="autoRowAutoColumnSpanning2 height25" data-offset-x="0" data-offset-y="80" data-expected-width="200" data-expected-height="25"></div>
-    </div>
-height expected 80 but got 85
-FAIL .inline-grid 11 assert_equals:
-<div class="inline-grid alignItemsBaseline columns min-content-rows">
-        <div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="100" data-expected-height="80">ÉÉ É ÉÉ ÉÉÉÉ É ÉÉ ÉÉÉ ÉÉ É</div>
-        <div class="firstRowSecondColumn height200Percent" data-offset-x="100" data-offset-y="0" data-expected-width="100" data-expected-height="160"></div>
-        <div class="autoRowAutoColumnSpanning2 height25" data-offset-x="0" data-offset-y="80" data-expected-width="200" data-expected-height="25"></div>
-    </div>
-height expected 80 but got 85
+PASS .inline-grid 11
 PASS .inline-grid 12
 

Modified: trunk/Source/WebCore/ChangeLog (275907 => 275908)


--- trunk/Source/WebCore/ChangeLog	2021-04-13 20:38:03 UTC (rev 275907)
+++ trunk/Source/WebCore/ChangeLog	2021-04-13 20:52:00 UTC (rev 275908)
@@ -1,3 +1,29 @@
+2021-04-13  Ziran Sun  <[email protected]>
+
+        [css-grid] Incorrect track sizing when using relative sized items in 'auto' column tracks
+        https://bugs.webkit.org/show_bug.cgi?id=191627
+
+        Reviewed by Javier Fernandez.
+
+        This is a reland of r274596. The original patch was reverted because Google logo is not visible
+        on google.com.
+
+        Containing block for a grid item is the grid area it's located in. During the GridTrackSizingAlgorithm,
+        we determine that this item has a relative size (percent in this case) and that the grid area is still
+        indefinite. Hence, we set nullopt for the grid area width and mark the item for layout. During the item's
+        layout, we call this RenderBox::containingBlockLogicalWidthForContent to determine the available height.
+        However, this method checks the containing block's width if the overridingContentLogicalWidth is nullopt,
+        which is incorrect for grid items. We should never use the grid container for resolving grid item's size.
+
+        This change is to return 0_lu for grid item when overridingContainingBlockContentLogicalWidth() returns
+        nullopt rather than use containing block's width. It also comments that there might be an issue with 
+        RenderBox::containingBlockLogicalHeightForContent for grid item. It should return whatever value we get from
+        overridingContainingBlockContentLogicalHeight(), including WTF::nullopt.
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::containingBlockLogicalWidthForContent const):
+        (WebCore::RenderBox::containingBlockLogicalHeightForContent const):
+
 2021-04-13  Devin Rousso  <[email protected]>
 
         [WebIDL] includes for `Conditional` dictionary members should also be guarded

Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (275907 => 275908)


--- trunk/Source/WebCore/rendering/RenderBox.cpp	2021-04-13 20:38:03 UTC (rev 275907)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp	2021-04-13 20:52:00 UTC (rev 275908)
@@ -2044,10 +2044,8 @@
 
 LayoutUnit RenderBox::containingBlockLogicalWidthForContent() const
 {
-    if (hasOverridingContainingBlockContentLogicalWidth()) {
-        if (auto width = overridingContainingBlockContentLogicalWidth())
-            return width.value();
-    }
+    if (hasOverridingContainingBlockContentLogicalWidth())
+        return overridingContainingBlockContentLogicalWidth().valueOr(0_lu);
 
     if (RenderBlock* cb = containingBlock())
         return cb->availableLogicalWidth();
@@ -2057,6 +2055,8 @@
 LayoutUnit RenderBox::containingBlockLogicalHeightForContent(AvailableLogicalHeightType heightType) const
 {
     if (hasOverridingContainingBlockContentLogicalHeight()) {
+        // FIXME: Containing block for a grid item is the grid area it's located in. We need to return whatever
+        // height value we get from overridingContainingBlockContentLogicalHeight() here, including WTF::nullopt.
         if (auto height = overridingContainingBlockContentLogicalHeight())
             return height.value();
     }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to