Attachment 430467 Details for Bug 222357 – Patch

[patch] Patch

bug-222357-20210603160857.patch (text/plain), 19.16 KB, created by Rob Buis on 2021-06-03 07:08:58 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Rob Buis

Created: 2021-06-03 07:08:58 PDT

Size: 19.16 KB

patch

obsolete

>Subversion Revision: 278401
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 62bc0ebabad4f8c1d023247f460c86f6267c7f5d..0cb3988b9125790c075327dad07bfad34681d916 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2021-06-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!).
>+
>+        Make computeMainSizeFromAspectRatioUsing take into account
>+        box-sizing for the case where CSS aspect-ratio is applied.
>+
>+        * rendering/RenderBox.cpp:
>+        (WebCore::RenderBox::computeIntrinsicLogicalWidthUsing const):
>+        * rendering/RenderFlexibleBox.cpp:
>+        (WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const):
>+        (WebCore::RenderFlexibleBox::adjustChildSizeForMinAndMax):
>+
> 2021-06-03  Julian Gonzalez  <julian_a_gonzalez@apple.com>
> 
>         Crash in HTMLConverter::_addLinkForElement()
>diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
>index 6834f238c4e627400165eeb3a918829f207ea7b4..39def86e596a1884c1b3405e3fe615e248085234 100644
>--- a/Source/WebCore/rendering/RenderBox.cpp
>+++ b/Source/WebCore/rendering/RenderBox.cpp
>@@ -2672,9 +2672,10 @@ LayoutUnit RenderBox::computeIntrinsicLogicalWidthUsing(Length logicalWidthLengt
> 
>     LayoutUnit minLogicalWidth;
>     LayoutUnit maxLogicalWidth;
>-    if (!logicalWidthLength.isMinIntrinsic() && shouldComputeLogicalWidthFromAspectRatio())
>+    if (!logicalWidthLength.isMinIntrinsic() && shouldComputeLogicalWidthFromAspectRatio()) {
>         minLogicalWidth = maxLogicalWidth = computeLogicalWidthFromAspectRatio();
>-    else
>+        borderAndPadding = LayoutUnit();
>+    } else
>         computeIntrinsicKeywordLogicalWidths(minLogicalWidth, maxLogicalWidth);
> 
>     if (logicalWidthLength.isMinContent() || logicalWidthLength.isMinIntrinsic())
>diff --git a/Source/WebCore/rendering/RenderFlexibleBox.cpp b/Source/WebCore/rendering/RenderFlexibleBox.cpp
>index 87593d686c1fd3adf9e4888a24bb9e9b2021ced6..8a4ed074bbf8a320bbba8c126f9f5861e7583c1b 100644
>--- a/Source/WebCore/rendering/RenderFlexibleBox.cpp
>+++ b/Source/WebCore/rendering/RenderFlexibleBox.cpp
>@@ -808,9 +808,7 @@ LayoutUnit RenderFlexibleBox::computeMainSizeFromAspectRatioUsing(const RenderBo
> {
>     ASSERT(childHasAspectRatio(child));
> 
>-    auto adjustForBoxSizing = [this] (const RenderBox& box, Length length) -> LayoutUnit {
>-        ASSERT(length.isFixed());
>-        auto value = LayoutUnit(length.value());
>+    auto adjustForBoxSizing = [this] (const RenderBox& box, LayoutUnit value) -> LayoutUnit {
>         // 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.
>@@ -819,9 +817,11 @@ LayoutUnit RenderFlexibleBox::computeMainSizeFromAspectRatioUsing(const RenderBo
>         return value;
>     };
> 
>-    std::optional<LayoutUnit> crossSize;
>+    // crossSize is border-box size if box-sizing is border-box, and
>+    // content-box otherwise.
>+    LayoutUnit crossSize;
>     if (crossSizeLength.isFixed())
>-        crossSize = adjustForBoxSizing(child, crossSizeLength);
>+        crossSize = LayoutUnit(crossSizeLength.value());
>     else if (crossSizeLength.isAuto()) {
>         ASSERT(childCrossSizeShouldUseContainerCrossSize(child));
>         auto containerCrossSizeLength = isHorizontalFlow() ? style().height() : style().width();
>@@ -830,22 +830,29 @@ LayoutUnit RenderFlexibleBox::computeMainSizeFromAspectRatioUsing(const RenderBo
>         crossSize = std::max(0_lu, valueForLength(containerCrossSizeLength, -1_lu) - crossAxisMarginExtentForChild(child));
>     } else {
>         ASSERT(crossSizeLength.isPercentOrCalculated());
>-        crossSize = mainAxisIsChildInlineAxis(child) ? child.computePercentageLogicalHeight(crossSizeLength) : adjustBorderBoxLogicalWidthForBoxSizing(valueForLength(crossSizeLength, contentWidth()), crossSizeLength.type());
>-        if (!crossSize)
>+        std::optional<LayoutUnit> childSize = mainAxisIsChildInlineAxis(child) ? child.computePercentageLogicalHeight(crossSizeLength) : adjustBorderBoxLogicalWidthForBoxSizing(valueForLength(crossSizeLength, contentWidth()), crossSizeLength.type());
>+        if (!childSize)
>             return 0_lu;
>+        crossSize = childSize.value();
>     }
> 
>     const LayoutSize& childIntrinsicSize = child.intrinsicSize();
>     double ratio;
>-    if (child.style().aspectRatioType() == AspectRatioType::Ratio || (child.style().aspectRatioType() == AspectRatioType::AutoAndRatio && childIntrinsicSize.isEmpty()))
>+    LayoutUnit borderAndPadding;
>+    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::ContentBox)
>+            crossSize -= isHorizontalFlow() ? child.verticalBorderAndPaddingExtent() : child.horizontalBorderAndPaddingExtent();
>+        else
>+            borderAndPadding = isHorizontalFlow() ? child.horizontalBorderAndPaddingExtent() : child.verticalBorderAndPaddingExtent();
>+    } else {
>         ASSERT(childIntrinsicSize.height());
>         ratio = childIntrinsicSize.width().toFloat() / childIntrinsicSize.height().toFloat();
>+        crossSize = adjustForBoxSizing(child, crossSize);
>     }
>     if (isHorizontalFlow())
>-        return LayoutUnit(crossSize.value() * ratio);
>-    return LayoutUnit(crossSize.value() / ratio);
>+        return LayoutUnit(crossSize * ratio) - borderAndPadding;
>+    return LayoutUnit(crossSize / ratio) - borderAndPadding;
> }
> 
> void RenderFlexibleBox::setFlowAwareLocationForChild(RenderBox& child, const LayoutPoint& location)
>@@ -1213,6 +1220,12 @@ LayoutUnit RenderFlexibleBox::adjustChildSizeForMinAndMax(RenderBox& child, Layo
>     if (max.isSpecifiedOrIntrinsic()) {
>         maxExtent = computeMainAxisExtentForChild(child, MaxSize, max);
>         childSize = std::min(childSize, maxExtent.value_or(childSize));
>+    } else if (child.style().hasAspectRatio()) {
>+        max = crossSizeLengthForChild(MaxSize, child);
>+        if (max.isSpecifiedOrIntrinsic()) {
>+            maxExtent = computeMainSizeFromAspectRatioUsing(child, max);
>+            childSize = std::min(childSize, maxExtent.value_or(childSize));
>+        }
>     }
> 
>     Length min = mainSizeLengthForChild(MinSize, child);
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 2936962364fbe197345224e13a2541a5e326f744..20a04573058d2f8dce810f9eedc5178acd236589 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2021-06-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!).
>+
>+        * TestExpectations:
>+
> 2021-06-03  Julian Gonzalez  <julian_a_gonzalez@apple.com>
> 
>         Crash in HTMLConverter::_addLinkForElement()
>diff --git a/LayoutTests/imported/w3c/ChangeLog b/LayoutTests/imported/w3c/ChangeLog
>index b13e25b6cdb01f5fe6e0a4ebe16fdab49d47df82..44efa45abf30414d1394af332af04fdbabc1f5b8 100644
>--- a/LayoutTests/imported/w3c/ChangeLog
>+++ b/LayoutTests/imported/w3c/ChangeLog
>@@ -1,3 +1,15 @@
>+2021-06-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!).
>+
>+        Import updated WPT tests.
>+
>+        * 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-06-02  Alex Christensen  <achristensen@webkit.org>
> 
>         Fix WPT test resource-timing/cross-origin-redirects.html
>diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
>index ff7cf8f7cb8967d898ea706fbf3c0319a28a5957..735789e6a138c8aa17d21098152f49b15294468b 100644
>--- a/LayoutTests/TestExpectations
>+++ b/LayoutTests/TestExpectations
>@@ -4460,8 +4460,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-018.html [ ImageOnlyFailure ]
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/grid-aspect-ratio-019.html [ ImageOnlyFailure ]
> webkit.org/b/214463 imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/grid-aspect-ratio-020.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 c4ff8110d7bed7079c864ddf192f8242932e588d..2399c103ecd2393c029b9b78be754b721eccfa5c 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
>@@ -4,6 +4,8 @@
>   <link rel="author" title="Ting-Yu Lin" href="mailto:tlin@mozilla.com">
>   <link rel="author" title="Mozilla" href="https://www.mozilla.org/">
>   <link rel="help" href="https://drafts.csswg.org/css-sizing-4/#aspect-ratio">
>+  <link rel="help" href="https://drafts.csswg.org/css-sizing-4/#aspect-ratio-minimum">
>+  <link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#min-size-auto">
>   <link rel="match" href="../../reference/ref-filled-green-200px-square.html" />
>   <style>
>   .flexContainer {
>@@ -21,54 +23,20 @@
> 
>   <p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
> 
>-  
>-  <div class="flexContainer" style="width: auto;">
>-    
>-    <div class="item" style="min-width:0; height: 25px; aspect-ratio: 8/1;"></div>
>-  </div>
>-
>-  <div class="flexContainer">
>-    
>-    <div class="item" style="height: 25px; aspect-ratio: 8/1;"></div>
>-  </div>
>+  
> 
>+  
>   <div class="flexContainer">
>-    
>+    
>     <div class="item" style="min-height: 25px; aspect-ratio: 8/1;"></div>
>   </div>
> 
>-  <div class="flexContainer">
>-    
>-    <div class="item" style="max-height: 25px; height: 100px; aspect-ratio: 8/1;"></div>
>-  </div>
>-
>-  
>-  <div class="flexContainer" style="width: auto;">
>-    
>-    <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 19/1;"></div>
>-  </div>
>-
>-  <div class="flexContainer">
>-    
>-    <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 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 c4a250818bb7786e2f395922d28e3c7519b4ea3a..ff6bf2aa5690551808b98e7117ec65d9178a3425 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
>@@ -4,6 +4,8 @@
>   <link rel="author" title="Ting-Yu Lin" href="mailto:tlin@mozilla.com">
>   <link rel="author" title="Mozilla" href="https://www.mozilla.org/">
>   <link rel="help" href="https://drafts.csswg.org/css-sizing-4/#aspect-ratio">
>+  <link rel="help" href="https://drafts.csswg.org/css-sizing-4/#aspect-ratio-minimum">
>+  <link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#min-size-auto">
>   <link rel="match" href="../../reference/ref-filled-green-200px-square.html" />
>   <style>
>   .flexContainer {
>@@ -19,60 +21,74 @@
>     box-sizing: border-box;
>   }
>   .item > div {
>-    height: 500px; /* Set a large content size suggestion for flex item. */
>+    height: 190px; /* Set the content size suggestion for flex item. */
>   }
>   </style>
> 
>   <p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
> 
>+  
>+
>   
>   <div class="flexContainer" style="height: auto;">
>     
>+         yielding a resolved flex base size (border-box) of 200px. -->
>     <div class="item" style="min-height:0; width: 25px; aspect-ratio: 1/8;"><div></div></div>
>   </div>
> 
>   <div class="flexContainer">
>-    
>-    <div class="item" style="width: 25px; aspect-ratio: 1/8;"><div></div></div>
>+    
>+    <div class="item" style="width: 25px; aspect-ratio: 1/1;"><div></div></div>
>   </div>
> 
>   <div class="flexContainer">
>-    
>-    <div class="item" style="min-width: 25px; aspect-ratio: 1/8;"><div></div></div>
>+    
>+    <div class="item" style="min-width: 25px; aspect-ratio: 1/1;"><div></div></div>
>   </div>
> 
>   <div class="flexContainer">
>-    
>-    <div class="item" style="max-width: 25px; width: 100px; aspect-ratio: 1/8;"><div></div></div>
>+    
>+    <div class="item" style="max-width: 25px; width: 100px; aspect-ratio: 1/8;">
>+      <div style="height: 500px"></div>
>+    </div>
>   </div>
> 
>   
>   <div class="flexContainer" style="height: auto;">
>     
>+         yielding a resolved flex base size (content-box) 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/19;"><div></div></div>
>+    
>+    <div class="item" style="width: 25px; aspect-ratio: auto 1/1;"><div></div></div>
>   </div>
> 
>   <div class="flexContainer">
>-    
>-    <div class="item" style="min-width: 25px; aspect-ratio: auto 1/19;"><div></div></div>
>+    
>+    <div class="item" style="min-width: 25px; aspect-ratio: auto 1/1;"><div></div></div>
>   </div>
> 
>   <div class="flexContainer">
>-    
>-    <div class="item" style="max-width: 25px; width: 100px; aspect-ratio: auto 1/19;"><div></div></div>
>+    
>+    <div class="item" style="max-width: 25px; width: 100px; aspect-ratio: auto 1/19;">
>+      <div style="height: 500px"></div>
>+    </div>
>   </div>
> </html>

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