Title: [288003] trunk
Revision
288003
Author
hey...@apple.com
Date
2022-01-13 18:53:22 -0800 (Thu, 13 Jan 2022)

Log Message

Only apply automatic minimum block-size aspect-ratio rules to non-replaced elements
https://bugs.webkit.org/show_bug.cgi?id=235058
<rdar://85505101>

Reviewed by Alan Bujtas.

Source/WebCore:

The rules in https://www.w3.org/TR/css-sizing-4/#aspect-ratio-minimum
that define the automatic minimum size of an element subject to an
aspect-ratio only apply if the element is non-replaced.

In constrainLogicalHeightByMinMax specifically, when min-height is
auto, after applying the aspect-ratio to produce an automatic minimum
height, we bump it up to the content height if the element has
children. This, presumably, is to account for the way the height of a
block is computed in CSS 2.2. (As CSS 2.2 doesn't have an auto value
for min-height, and a CSS Block Layout module has not been written in
terms of CSS Sizing concepts, there is no clear definition of
automatic minimum sizing for blocks at the moment.) If we erroneously
apply this to a replaced element, such as an image or video, it can
get a computed min-height equal to the intrinsic height of the image,
which may be much larger than expected.

This commit adds an is<RenderReplaced>() check for both automatic
minimum logical width and height calculations, although I was unable to
produce a test to exercise the logical width case (which does not take
the intrinsic size into account when computing the automatic minimum).

Test: fast/css/aspect-ratio-min-height-replaced.html

* rendering/RenderBox.cpp:
(WebCore::RenderBox::constrainLogicalWidthInFragmentByMinMax const):
(WebCore::RenderBox::constrainLogicalHeightByMinMax const):

LayoutTests:

Test that replaced elements with child content (in this case, a
<video> element's controls in the UA shadow tree) don't influence the
automatic minimum size calculation.

* fast/css/aspect-ratio-min-height-replaced-expected.html: Added.
* fast/css/aspect-ratio-min-height-replaced.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (288002 => 288003)


--- trunk/LayoutTests/ChangeLog	2022-01-14 02:40:08 UTC (rev 288002)
+++ trunk/LayoutTests/ChangeLog	2022-01-14 02:53:22 UTC (rev 288003)
@@ -1,3 +1,18 @@
+2022-01-13  Cameron McCormack  <hey...@apple.com>
+
+        Only apply automatic minimum block-size aspect-ratio rules to non-replaced elements
+        https://bugs.webkit.org/show_bug.cgi?id=235058
+        <rdar://85505101>
+
+        Reviewed by Alan Bujtas.
+
+        Test that replaced elements with child content (in this case, a
+        <video> element's controls in the UA shadow tree) don't influence the
+        automatic minimum size calculation.
+
+        * fast/css/aspect-ratio-min-height-replaced-expected.html: Added.
+        * fast/css/aspect-ratio-min-height-replaced.html: Added.
+
 2022-01-13  Chris Dumez  <cdu...@apple.com>
 
         Implement HTMLScriptElement.supports(type) method

Added: trunk/LayoutTests/fast/css/aspect-ratio-min-height-replaced-expected.html (0 => 288003)


--- trunk/LayoutTests/fast/css/aspect-ratio-min-height-replaced-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css/aspect-ratio-min-height-replaced-expected.html	2022-01-14 02:53:22 UTC (rev 288003)
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<div style="display: flex;">
+  <video controls width="100" height="100" style="background-color: yellow;"></video>
+</div>

Added: trunk/LayoutTests/fast/css/aspect-ratio-min-height-replaced.html (0 => 288003)


--- trunk/LayoutTests/fast/css/aspect-ratio-min-height-replaced.html	                        (rev 0)
+++ trunk/LayoutTests/fast/css/aspect-ratio-min-height-replaced.html	2022-01-14 02:53:22 UTC (rev 288003)
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<div style="display: flex; flex-direction: column; align-items: start;">
+  <video controls width="300" height="300" style="width: auto; max-height: 100px; aspect-ratio: 1 / 1; background-color: yellow;"></video>
+</div>

Modified: trunk/Source/WebCore/ChangeLog (288002 => 288003)


--- trunk/Source/WebCore/ChangeLog	2022-01-14 02:40:08 UTC (rev 288002)
+++ trunk/Source/WebCore/ChangeLog	2022-01-14 02:53:22 UTC (rev 288003)
@@ -1,3 +1,38 @@
+2022-01-13  Cameron McCormack  <hey...@apple.com>
+
+        Only apply automatic minimum block-size aspect-ratio rules to non-replaced elements
+        https://bugs.webkit.org/show_bug.cgi?id=235058
+        <rdar://85505101>
+
+        Reviewed by Alan Bujtas.
+
+        The rules in https://www.w3.org/TR/css-sizing-4/#aspect-ratio-minimum
+        that define the automatic minimum size of an element subject to an
+        aspect-ratio only apply if the element is non-replaced.
+
+        In constrainLogicalHeightByMinMax specifically, when min-height is
+        auto, after applying the aspect-ratio to produce an automatic minimum
+        height, we bump it up to the content height if the element has
+        children. This, presumably, is to account for the way the height of a
+        block is computed in CSS 2.2. (As CSS 2.2 doesn't have an auto value
+        for min-height, and a CSS Block Layout module has not been written in
+        terms of CSS Sizing concepts, there is no clear definition of
+        automatic minimum sizing for blocks at the moment.) If we erroneously
+        apply this to a replaced element, such as an image or video, it can
+        get a computed min-height equal to the intrinsic height of the image,
+        which may be much larger than expected.
+
+        This commit adds an is<RenderReplaced>() check for both automatic
+        minimum logical width and height calculations, although I was unable to
+        produce a test to exercise the logical width case (which does not take
+        the intrinsic size into account when computing the automatic minimum).
+
+        Test: fast/css/aspect-ratio-min-height-replaced.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::constrainLogicalWidthInFragmentByMinMax const):
+        (WebCore::RenderBox::constrainLogicalHeightByMinMax const):
+
 2022-01-13  Simon Fraser  <simon.fra...@apple.com>
 
         Move the code that computes layer content visibility into its own function

Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (288002 => 288003)


--- trunk/Source/WebCore/rendering/RenderBox.cpp	2022-01-14 02:40:08 UTC (rev 288002)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp	2022-01-14 02:53:22 UTC (rev 288003)
@@ -673,13 +673,12 @@
         logicalWidth = std::min(logicalWidth, computeLogicalWidthInFragmentUsing(MaxSize, styleToUse.logicalMaxWidth(), availableWidth, cb, fragment));
     if (allowIntrinsic == AllowIntrinsic::No && styleToUse.logicalMinWidth().isIntrinsic())
         return logicalWidth;
-    auto minLength = styleToUse.logicalMinWidth();
-    if (styleToUse.hasAspectRatio() && minLength.isAuto() && (styleToUse.logicalWidth().isAuto() || styleToUse.logicalWidth().isMinContent() || styleToUse.logicalWidth().isMaxContent()) && effectiveOverflowInlineDirection() == Overflow::Visible) {
+    auto logicalMinWidth = styleToUse.logicalMinWidth();
+    if (logicalMinWidth.isAuto() && shouldComputeLogicalWidthFromAspectRatio() && (styleToUse.logicalWidth().isAuto() || styleToUse.logicalWidth().isMinContent() || styleToUse.logicalWidth().isMaxContent()) && !is<RenderReplaced>(*this) && effectiveOverflowInlineDirection() == Overflow::Visible) {
         // Make sure we actually used the aspect ratio.
-        if (shouldComputeLogicalWidthFromAspectRatio())
-            minLength = Length(LengthType::MinContent);
+        logicalMinWidth = Length(LengthType::MinContent);
     }
-    return std::max(logicalWidth, computeLogicalWidthInFragmentUsing(MinSize, minLength, availableWidth, cb, fragment));
+    return std::max(logicalWidth, computeLogicalWidthInFragmentUsing(MinSize, logicalMinWidth, availableWidth, cb, fragment));
 }
 
 LayoutUnit RenderBox::constrainLogicalHeightByMinMax(LayoutUnit logicalHeight, std::optional<LayoutUnit> intrinsicContentHeight) const
@@ -690,7 +689,7 @@
             logicalHeight = std::min(logicalHeight, maxH.value());
     }
     auto logicalMinHeight = styleToUse.logicalMinHeight();
-    if (logicalMinHeight.isAuto() && shouldComputeLogicalHeightFromAspectRatio() && intrinsicContentHeight && effectiveOverflowBlockDirection() == Overflow::Visible) {
+    if (logicalMinHeight.isAuto() && shouldComputeLogicalHeightFromAspectRatio() && intrinsicContentHeight && !is<RenderReplaced>(*this) && effectiveOverflowBlockDirection() == Overflow::Visible) {
         auto heightFromAspectRatio = blockSizeFromAspectRatio(horizontalBorderAndPaddingExtent(), verticalBorderAndPaddingExtent(), style().logicalAspectRatio(), style().boxSizingForAspectRatio(), logicalWidth()) - borderAndPaddingLogicalHeight();
         if (firstChild())
             heightFromAspectRatio = std::max(heightFromAspectRatio, *intrinsicContentHeight);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to