Changeset 286100 in webkit

Timestamp:

Nov 21, 2021 10:22:01 AM (8 months ago)

Author:

Ziran Sun

Message:

[css-grid] svg image as grid items should use the overriding logical width/height when defined to compute the logical height/width
​https://bugs.webkit.org/show_bug.cgi?id=228105

Reviewed by Javier Fernandez.

Source/WebCore:

This is a reland of r280290. It's got reverted because another patch r280078 caused crash and had been
reverted. r280290 is a dependent patch for r280078.

As discussed at ​https://github.com/w3c/csswg-drafts/issues/6286#issuecomment-866986544, degenerate
aspect ratios derived from SVG width/height attributes fall back to viewbox aspect ratio
(whether due to negative values or zero values).

When computing the logical height/width using an intrinsic aspect ratio, RenderReplaced uses the
overridingLogicalWidth/overridingLogicalHeight whenever defined as long as the flex or
grid item has an intrinsic size. For an SVG graphic though, it's common to have an intrinsic aspect
ratio but not to have an intrinsic width or height. For this special case, we still should use
overridingLogicalWidth/overridingLogicalHeight for logical height/width calculations.

• rendering/RenderReplaced.cpp:

(WebCore::hasIntrinsicSize):
(WebCore::RenderReplaced::computeReplacedLogicalWidth const):
(WebCore::RenderReplaced::computeReplacedLogicalHeight const):

LayoutTests:

• TestExpectations: unskip two tests that are now passing.

Location:

trunk

Files:

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

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-11-21  Ziran Sun  <zsun@igalia.com>
+
+        [css-grid] svg image as grid items should use the overriding logical width/height when defined to compute the logical height/width
+        https://bugs.webkit.org/show_bug.cgi?id=228105
+
+        Reviewed by Javier Fernandez.
+
+        * TestExpectations: unskip two tests that are now passing.
+
 2021-11-20  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/LayoutTests/TestExpectations

@@ -4323 +4323 @@
 webkit.org/b/209460 imported/w3c/web-platform-tests/css/css-grid/abspos/descendant-static-position-003.html [ ImageOnlyFailure ]
 webkit.org/b/212201 imported/w3c/web-platform-tests/css/css-grid/grid-definition/grid-limits-001.html [ Skip ]
-webkit.org/b/227900 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-aspect-ratio-stretch-1.html [ ImageOnlyFailure ]
-webkit.org/b/227900 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-aspect-ratio-stretch-2.html [ ImageOnlyFailure ]
 webkit.org/b/212246 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-baseline-align-cycles-001.html [ ImageOnlyFailure ]
 webkit.org/b/231021 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-inline-baseline.html [ ImageOnlyFailure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-11-21  Ziran Sun  <zsun@igalia.com>
+
+        [css-grid] svg image as grid items should use the overriding logical width/height when defined to compute the logical height/width
+        https://bugs.webkit.org/show_bug.cgi?id=228105
+
+        Reviewed by Javier Fernandez.
+
+        This is a reland of r280290. It's got reverted because another patch r280078 caused crash and had been
+        reverted. r280290 is a dependent patch for r280078.
+
+        As discussed at https://github.com/w3c/csswg-drafts/issues/6286#issuecomment-866986544, degenerate
+        aspect ratios derived from SVG width/height attributes fall back to viewbox aspect ratio 
+        (whether due to negative values or zero values).
+
+        When computing the logical height/width using an intrinsic aspect ratio, RenderReplaced uses the
+        overridingLogicalWidth/overridingLogicalHeight whenever defined as long as the flex or
+        grid item has an intrinsic size. For an SVG graphic though, it's common to have an intrinsic aspect
+        ratio but not to have an intrinsic width or height. For this special case, we still should use
+        overridingLogicalWidth/overridingLogicalHeight for logical height/width calculations.        
+
+        * rendering/RenderReplaced.cpp:
+        (WebCore::hasIntrinsicSize):
+        (WebCore::RenderReplaced::computeReplacedLogicalWidth const):
+        (WebCore::RenderReplaced::computeReplacedLogicalHeight const):
+
 2021-11-21  Alan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/rendering/RenderReplaced.cpp

@@ -532 +532 @@
 }
 
+static inline bool hasIntrinsicSize(RenderBox*contentRenderer, bool hasIntrinsicWidth, bool hasIntrinsicHeight )
+{
+    if (hasIntrinsicWidth && hasIntrinsicHeight)
+        return true;
+    if (hasIntrinsicWidth || hasIntrinsicHeight)
+        return contentRenderer && contentRenderer->isSVGRoot();
+    return false;
+}
+
 LayoutUnit RenderReplaced::computeReplacedLogicalWidth(ShouldComputePreferred shouldComputePreferred) const
 {
@@ -554 +563 @@
         // grid item has an intrinsic size. It is possible (indeed, common) for an SVG graphic to have an intrinsic aspect ratio but not to have an intrinsic
         // width or height. There are also elements with intrinsic sizes but without intrinsic ratio (like an iframe).
-        if (intrinsicRatio && (isFlexItem() || isGridItem()) && hasOverridingLogicalHeight() && hasIntrinsicWidth && hasIntrinsicHeight)
+        if (intrinsicRatio && (isFlexItem() || isGridItem()) && hasOverridingLogicalHeight() && hasIntrinsicSize(contentRenderer, hasIntrinsicWidth, hasIntrinsicHeight))
             return computeReplacedLogicalWidthRespectingMinMaxWidth(roundToInt(round(overridingContentLogicalHeight() * intrinsicRatio)), shouldComputePreferred);
 
@@ -624 +633 @@
 
     // See computeReplacedLogicalHeight() for a similar check for heights.
-    if (intrinsicRatio && (isFlexItem() || isGridItem()) && hasOverridingLogicalWidth() && hasIntrinsicHeight && hasIntrinsicWidth)
+    if (intrinsicRatio && (isFlexItem() || isGridItem()) && hasOverridingLogicalWidth() && hasIntrinsicSize(contentRenderer, hasIntrinsicWidth, hasIntrinsicHeight))
         return computeReplacedLogicalHeightRespectingMinMaxHeight(roundToInt(round(overridingContentLogicalWidth() / intrinsicRatio)));