Attachment 422056 Details for Bug 222357 – Patch

[patch] Patch

bug-222357-20210303123557.patch (text/plain), 13.78 KB, created by Rob Buis on 2021-03-03 03:35:59 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Rob Buis

Created: 2021-03-03 03:35:59 PST

Size: 13.78 KB

patch

obsolete

>Subversion Revision: 273812
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ebd61642efbe6a6323681b882859eb189620898d..199007fbc62b5742fd86d76436abddc3ee7cfc45 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,21 @@
>+2021-03-03  Rob Buis  <rbuis@igalia.com>
>+
>+        Fix flex-aspect-ratio-025+26.html
>+        https://bugs.webkit.org/show_bug.cgi?id=222357
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fix flex-aspect-ratio-025+26.html by fixing two things:
>+        - apply transferred min/max sizes for intrinsic sizing.
>+        - make computeMainSizeFromAspectRatioUsing take into account
>+          box-sizing for the case where CSS aspect-ratio is applied.
>+
>+        Behavior matches Chrome and Firefox.
>+
>+        * rendering/RenderFlexibleBox.cpp:
>+        (WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const):
>+        (WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax):
>+
> 2021-03-02  Manuel Rego Casasnovas  <rego@igalia.com>
> 
>         [selectors] :focus-visible implementation
>diff --git a/Source/WebCore/rendering/RenderFlexibleBox.cpp b/Source/WebCore/rendering/RenderFlexibleBox.cpp
>index d3cd228c56b93392cdf0dc47436a09023c1f7c24..d575714a7730601181c7795f6d007d6d93345230 100644
>--- a/Source/WebCore/rendering/RenderFlexibleBox.cpp
>+++ b/Source/WebCore/rendering/RenderFlexibleBox.cpp
>@@ -793,7 +793,12 @@ LayoutUnit RenderFlexibleBox::computeMainSizeFromAspectRatioUsing(const RenderBo
>         // We need to substract the border and padding extent from the cross axis.
>         // Furthermore, the sizing calculations that floor the content box size at zero when applying box-sizing are also ignored.
>         // https://drafts.csswg.org/css-flexbox/#algo-main-item.
>-        if (box.style().boxSizing() == BoxSizing::BorderBox)
>+        bool subtractBorderPadding = false;
>+        if (box.style().hasAspectRatio())
>+            subtractBorderPadding = box.style().boxSizingForAspectRatio() == BoxSizing::ContentBox;
>+        else if (box.style().boxSizing() == BoxSizing::BorderBox)
>+            subtractBorderPadding = true;
>+        if (subtractBorderPadding)
>             value -= isHorizontalFlow() ? box.verticalBorderAndPaddingExtent() : box.horizontalBorderAndPaddingExtent();
>         return value;
>     };
>@@ -816,15 +821,18 @@ LayoutUnit RenderFlexibleBox::computeMainSizeFromAspectRatioUsing(const RenderBo
> 
>     const LayoutSize& childIntrinsicSize = child.intrinsicSize();
>     double ratio;
>-    if (child.style().aspectRatioType() == AspectRatioType::Ratio || (child.style().aspectRatioType() == AspectRatioType::AutoAndRatio && childIntrinsicSize.isEmpty()))
>+    LayoutUnit borderPadding;
>+    if (child.style().aspectRatioType() == AspectRatioType::Ratio || (child.style().aspectRatioType() == AspectRatioType::AutoAndRatio && childIntrinsicSize.isEmpty())) {
>         ratio = child.style().aspectRatioWidth() / child.style().aspectRatioHeight();
>-    else {
>+        if (child.style().boxSizingForAspectRatio() == BoxSizing::BorderBox)
>+            borderPadding = isHorizontalFlow() ? child.horizontalBorderAndPaddingExtent() : child.verticalBorderAndPaddingExtent();
>+    } else {
>         ASSERT(childIntrinsicSize.height());
>         ratio = childIntrinsicSize.width().toFloat() / childIntrinsicSize.height().toFloat();
>     }
>     if (isHorizontalFlow())
>-        return LayoutUnit(crossSize.value() * ratio);
>-    return LayoutUnit(crossSize.value() / ratio);
>+        return LayoutUnit(crossSize.value() * ratio) - borderPadding;
>+    return LayoutUnit(crossSize.value() / ratio) - borderPadding;
> }
> 
> void RenderFlexibleBox::setFlowAwareLocationForChild(RenderBox& child, const LayoutPoint& location)
>@@ -1191,11 +1199,19 @@ LayoutUnit RenderFlexibleBox::adjustChildSizeForMinAndMax(const RenderBox& child
>         maxExtent = computeMainAxisExtentForChild(child, MaxSize, max);
>         childSize = std::min(childSize, maxExtent.valueOr(childSize));
>     }
>+    Length crossMax = crossSizeLengthForChild(MaxSize, child);
>+    if (child.style().hasAspectRatio() && !childMainSizeIsDefinite(child, flexBasisForChild(child)) && childCrossSizeIsDefinite(child, crossMax)) {
>+        maxExtent = computeMainSizeFromAspectRatioUsing(child, crossMax);
>+        childSize = std::min(childSize, maxExtent.valueOr(childSize));
>+    }
> 
>     Length min = mainSizeLengthForChild(MinSize, child);
>     if (min.isSpecifiedOrIntrinsic())
>         return std::max(childSize, std::max(0_lu, computeMainAxisExtentForChild(child, MinSize, min).valueOr(childSize)));
>-    
>+    Length crossMin = crossSizeLengthForChild(MinSize, child);
>+    if (child.style().hasAspectRatio() && !childMainSizeIsDefinite(child, flexBasisForChild(child)) && childCrossSizeIsDefinite(child, crossMin))
>+        return std::max(childSize, computeMainSizeFromAspectRatioUsing(child, crossMin));
>+
>     if (shouldApplyMinSizeAutoForChild(child)) {
>         // FIXME: If the min value is expected to be valid here, we need to come up with a non optional version of computeMainAxisExtentForChild and
>         // ensure it's valid through the virtual calls of computeIntrinsicLogicalContentHeightUsing.
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 0169b480ada56c002f8b5f8cc7bacf62749e703e..01690a709e89f697890a904ccf2fbc0da9db40af 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2021-03-03  Rob Buis  <rbuis@igalia.com>
>+
>+        Fix flex-aspect-ratio-025+26.html
>+        https://bugs.webkit.org/show_bug.cgi?id=222357
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Adapt platform test expectations.
>+
>+        * TestExpectations:
>+
> 2021-03-02  Manuel Rego Casasnovas  <rego@igalia.com>
> 
>         [selectors] :focus-visible implementation
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index 307aa6371598512b526784b4b23c60eb14580002..c3db42d4852d7d7b57842de42af834ae96390226 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,15 @@
>+2021-03-03  Rob Buis  <rbuis@igalia.com>
>+
>+        Fix flex-aspect-ratio-025+26.html
>+        https://bugs.webkit.org/show_bug.cgi?id=222357
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Sync these tests to test both main and cross size padding.
>+
>+        * web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-025.html:
>+        * web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-026.html:
>+
> 2021-03-02  Manuel Rego Casasnovas  <rego@igalia.com>
> 
>         [selectors] :focus-visible implementation
>diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
>index d5172befe552f5dfbf120493e8b83b435e541ff4..aa886504059a838d4b6bb5b602ff221f32ca0c45 100644
>--- a/LayoutTests/TestExpectations
>+++ b/LayoutTests/TestExpectations
>@@ -4554,8 +4554,6 @@ webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/
> # flex-basis:content
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-021.html [ ImageOnlyFailure ]
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-022.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-025.html [ ImageOnlyFailure ]
>-webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-026.html [ ImageOnlyFailure ]
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/grid-aspect-ratio-015.html [ ImageOnlyFailure ]
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/grid-aspect-ratio-016.html [ ImageOnlyFailure ]
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/grid-aspect-ratio-017.html [ ImageOnlyFailure ]
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-025.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-025.html
>index fa9ce8ebce69f02e6b13d9d2233d1e3b46c7f1ab..c4ff8110d7bed7079c864ddf192f8242932e588d 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-025.html
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-025.html
>@@ -14,6 +14,7 @@
>   .item {
>     background: green;
>     padding-top: 15px;
>+    padding-left: 10px;
>     box-sizing: border-box;
>   }
>   </style>
>@@ -49,25 +50,25 @@
>        because its value contains 'auto'. -->
>   <div class="flexContainer" style="width: auto;">
>     
>-    <div class="item" style="min-width:0; height: 25px; aspect-ratio: auto 20/1;"></div>
>+         yielding a resolved flex base size of 190px. -->
>+    <div class="item" style="min-width:0; height: 25px; aspect-ratio: auto 19/1;"></div>
>   </div>
> 
>   <div class="flexContainer">
>     
>-    <div class="item" style="height: 25px; aspect-ratio: auto 20/1;"></div>
>+         yielding a resolved min-width:auto of 190px. -->
>+    <div class="item" style="height: 25px; aspect-ratio: auto 19/1;"></div>
>   </div>
> 
>   <div class="flexContainer">
>     
>-    <div class="item" style="min-height: 25px; aspect-ratio: auto 20/1;"></div>
>+         yielding a resolved min-width:auto of 190px. -->
>+    <div class="item" style="min-height: 25px; aspect-ratio: auto 19/1;"></div>
>   </div>
> 
>   <div class="flexContainer">
>     
>-    <div class="item" style="max-height: 25px; height: 100px; aspect-ratio: auto 20/1;"></div>
>+         to the main axis, yielding a resolved min-width:auto of 190px. -->
>+    <div class="item" style="max-height: 25px; height: 100px; aspect-ratio: auto 19/1;"></div>
>   </div>
> </html>
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-026.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-026.html
>index 1df14468632baa8ffd03d690f7ddaa618acd7bf6..c4a250818bb7786e2f395922d28e3c7519b4ea3a 100644
>--- a/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-026.html
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-026.html
>@@ -15,6 +15,7 @@
>   .item {
>     background: green;
>     padding-left: 15px;
>+    padding-top: 10px;
>     box-sizing: border-box;
>   }
>   .item > div {
>@@ -53,25 +54,25 @@
>        because its value contains 'auto'. -->
>   <div class="flexContainer" style="height: auto;">
>     
>-    <div class="item" style="min-height:0; width: 25px; aspect-ratio: auto 1/20;"><div></div></div>
>+         yielding a resolved flex base size of 190px. -->
>+    <div class="item" style="min-height:0; width: 25px; aspect-ratio: auto 1/19;"><div></div></div>
>   </div>
> 
>   <div class="flexContainer">
>     
>-    <div class="item" style="width: 25px; aspect-ratio: auto 1/20;"><div></div></div>
>+         yielding a resolved min-height:auto of 190px. -->
>+    <div class="item" style="width: 25px; aspect-ratio: auto 1/19;"><div></div></div>
>   </div>
> 
>   <div class="flexContainer">
>     
>-    <div class="item" style="min-width: 25px; aspect-ratio: auto 1/20;"><div></div></div>
>+         yielding a resolved min-height:auto of 190px. -->
>+    <div class="item" style="min-width: 25px; aspect-ratio: auto 1/19;"><div></div></div>
>   </div>
> 
>   <div class="flexContainer">
>     
>-    <div class="item" style="max-width: 25px; width: 100px; aspect-ratio: auto 1/20;"><div></div></div>
>+         yielding a resolved min-height:auto of 190px. -->
>+    <div class="item" style="max-width: 25px; width: 100px; aspect-ratio: auto 1/19;"><div></div></div>
>   </div>
> </html>
>diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-027.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-027.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..e8fa4943925aab6420927739b237441c7e0d8c61
>--- /dev/null
>+++ b/LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-027.html
>@@ -0,0 +1,16 @@
>+<!DOCTYPE html>
>+<title>CSS aspect-ratio: Row flexbox and transferred min/max-size</title>
>+<link rel="author" title="Google LLC" href="https://www.google.com/">
>+<link rel="help" href="https://drafts.csswg.org/css-sizing-4/#aspect-ratio">
>+<link rel="match" href="../../reference/ref-filled-green-100px-square.xht" />
>+
>+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
>+
>+<div style="display: flex; flex-direction: row;">
>+  <div style="background: green; width: 60px; max-height: 50px; aspect-ratio: 1/1;"></div>
>+  <div style="background: green; width: 40px; height: 50px;" ></div>
>+</div>
>+<div style="display: flex; flex-direction: row;">
>+  <div style="background: green; width: 60px; height: 60px; max-height: 50px; aspect-ratio: 1/1;"></div>
>+  <div style="background: green; width: 40px; height: 50px;" ></div>
>+</div>

Actions: View | Formatted Diff | Diff

Attachments on bug 222357: 421407 | 421649 | 421775 | 421776 | 421915 | 422056 | 422071 | 422085 | 422206 | 422374 | 422484 | 422485 | 422492 | 422494 | 422500 | 422522 | 422523 | 422525 | 422560 | 422809 | 422811 | 429277 | 429282 | 429309 | 429875 | 430467 | 431546 | 432280 | 432480 | 432574 | 432596 | 433785 | 437454 | 437625