Changeset 286206 in webkit

Timestamp:

Nov 29, 2021 2:00:51 AM (8 months ago)

Author:

svillar@igalia.com

Message:

[css-flexbox] Add support for replaced elements with intrinsic ratio and no intrinsic size
​https://bugs.webkit.org/show_bug.cgi?id=231952
<rdar://problem/84662305>

Reviewed by Manuel Rego Casasnovas.

Source/WebCore:

It might be the case that replaced elements have intrinsic aspect ratio but no
intrinsic dimensions. Actually that's quite common for SVGs for example. We're
incorrectly assuming that flex items with no intrinsic dimensions didn't have
intrinsic aspect ratios which is wrong.

Added new checks that are able to detect those cases were we have an intrinsic
aspect ratio even though there are no intrinsic dimensions.

• rendering/RenderFlexibleBox.cpp:

(WebCore::isRenderReplacedWithIntrinsicAspectRatio):
(WebCore::childHasAspectRatio):
Consider replaced elements with intrinsic ratios and no intrinsic dimensions.
(WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const):
Let replaced elements compute the aspect ratio by themselves.
(WebCore::RenderFlexibleBox::childHasComputableAspectRatio const):

• rendering/RenderReplaced.cpp:

(WebCore::RenderReplaced::computeIntrinsicAspectRatio const): New method that
computes the aspect ratio for replaced elements.

• rendering/RenderReplaced.h:

LayoutTests:

• TestExpectations: Unskipped a test case that is now passing.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderFlexibleBox.cpp (modified) (4 diffs)
• Source/WebCore/rendering/RenderReplaced.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderReplaced.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-10-27  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flexbox] Add support for replaced elements with intrinsic ratio and no intrinsic size
+        https://bugs.webkit.org/show_bug.cgi?id=231952
+        <rdar://problem/84662305>
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        * TestExpectations: Unskipped a test case that is now passing.
+
 2021-11-28  Matt Woodrow  <matt.woodrow@gmail.com>
 

trunk/LayoutTests/TestExpectations

@@ -4243 +4243 @@
 # SVGs as flex items.
 webkit.org/b/221474 imported/w3c/web-platform-tests/css/css-flexbox/svg-root-as-flex-item-002.html [ ImageOnlyFailure ]
-webkit.org/b/221474 imported/w3c/web-platform-tests/css/css-flexbox/svg-root-as-flex-item-003.html [ ImageOnlyFailure ]
 
 # The test works fine but the expected result fails due to a missing layout.

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-10-27  Sergio Villar Senin  <svillar@igalia.com>
+
+        [css-flexbox] Add support for replaced elements with intrinsic ratio and no intrinsic size
+        https://bugs.webkit.org/show_bug.cgi?id=231952
+        <rdar://problem/84662305>
+
+        Reviewed by Manuel Rego Casasnovas.
+
+        It might be the case that replaced elements have intrinsic aspect ratio but no
+        intrinsic dimensions. Actually that's quite common for SVGs for example. We're
+        incorrectly assuming that flex items with no intrinsic dimensions didn't have
+        intrinsic aspect ratios which is wrong.
+
+        Added new checks that are able to detect those cases were we have an intrinsic
+        aspect ratio even though there are no intrinsic dimensions.
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::isRenderReplacedWithIntrinsicAspectRatio):
+        (WebCore::childHasAspectRatio):
+        Consider replaced elements with intrinsic ratios and no intrinsic dimensions.
+        (WebCore::RenderFlexibleBox::computeMainSizeFromAspectRatioUsing const):
+        Let replaced elements compute the aspect ratio by themselves.
+        (WebCore::RenderFlexibleBox::childHasComputableAspectRatio const):
+        * rendering/RenderReplaced.cpp:
+        (WebCore::RenderReplaced::computeIntrinsicAspectRatio const): New method that
+        computes the aspect ratio for replaced elements.
+        * rendering/RenderReplaced.h:
+
 2021-11-29  Kimmo Kinnunen  <kkinnunen@apple.com>
 

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -38 +38 @@
 #include "RenderLayer.h"
 #include "RenderLayoutState.h"
+#include "RenderObjectEnums.h"
+#include "RenderReplaced.h"
 #include "RenderStyleConstants.h"
+#include "RenderSVGRoot.h"
 #include "RenderView.h"
 #include "WritingMode.h"
@@ -615 +618 @@
 }
 
+// FIXME: consider adding this check to RenderBox::hasIntrinsicAspectRatio(). We could even make it
+// virtual returning false by default. RenderReplaced will overwrite it with the current implementation
+// plus this extra check. See wkb.ug/231955.
+static bool isRenderReplacedWithIntrinsicAspectRatio(const RenderBox& child)
+{
+    if (!is<RenderReplaced>(child))
+        return false;
+    // It's common for some replaced elements, such as SVGs, to have intrinsic aspect ratios but no intrinsic sizes.
+    // That's why it isn't enough just to check for intrinsic sizes in those cases.
+    return downcast<RenderReplaced>(child).computeIntrinsicAspectRatio() > 0;
+};
+
 static bool childHasAspectRatio(const RenderBox& child)
 {
-    return child.hasIntrinsicAspectRatio() || child.style().hasAspectRatio();
+    return child.hasIntrinsicAspectRatio() || child.style().hasAspectRatio() || isRenderReplacedWithIntrinsicAspectRatio(child);
 }
 
@@ -884 +899 @@
     }
 
-    const LayoutSize& childIntrinsicSize = child.intrinsicSize();
     double ratio;
-    if (child.style().aspectRatioType() == AspectRatioType::Ratio || (child.style().aspectRatioType() == AspectRatioType::AutoAndRatio && childIntrinsicSize.isEmpty()))
-        ratio = child.style().aspectRatioWidth() / child.style().aspectRatioHeight();
+    if (is<RenderReplaced>(child))
+        ratio = downcast<RenderReplaced>(child).computeIntrinsicAspectRatio();
     else {
-        ASSERT(childIntrinsicSize.height());
-        ratio = childIntrinsicSize.width().toFloat() / childIntrinsicSize.height().toFloat();
+        auto childIntrinsicSize = child.intrinsicSize();
+        if (child.style().aspectRatioType() == AspectRatioType::Ratio || (child.style().aspectRatioType() == AspectRatioType::AutoAndRatio && childIntrinsicSize.isEmpty()))
+            ratio = child.style().aspectRatioWidth() / child.style().aspectRatioHeight();
+        else {
+            ASSERT(childIntrinsicSize.height());
+            ratio = childIntrinsicSize.width().toFloat() / childIntrinsicSize.height().toFloat();
+        }
     }
     if (isHorizontalFlow())
@@ -935 +954 @@
     if (!childHasAspectRatio(child))
         return false;
-    return child.intrinsicSize().height() || child.style().hasAspectRatio();
+    return child.intrinsicSize().height() || child.style().hasAspectRatio() || isRenderReplacedWithIntrinsicAspectRatio(child);
 }
 

trunk/Source/WebCore/rendering/RenderReplaced.cpp

@@ -476 +476 @@
 }
 
+double RenderReplaced::computeIntrinsicAspectRatio() const
+{
+    double intrinsicRatio;
+    FloatSize intrinsicSize;
+    computeAspectRatioInformationForRenderBox(embeddedContentBox(), intrinsicSize, intrinsicRatio);
+    return intrinsicRatio;
+}
+
 RoundedRect RenderReplaced::roundedContentBoxRect() const
 {

trunk/Source/WebCore/rendering/RenderReplaced.h

@@ -52 +52 @@
     bool isContentLikelyVisibleInViewport();
     bool needsPreferredWidthsRecalculation() const override;
+
+    double computeIntrinsicAspectRatio() const;
 
 protected: