Title: [286100] trunk
Revision
286100
Author
z...@igalia.com
Date
2021-11-21 10:22:01 -0800 (Sun, 21 Nov 2021)

Log 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.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (286099 => 286100)


--- trunk/LayoutTests/ChangeLog	2021-11-21 16:51:11 UTC (rev 286099)
+++ trunk/LayoutTests/ChangeLog	2021-11-21 18:22:01 UTC (rev 286100)
@@ -1,3 +1,12 @@
+2021-11-21  Ziran Sun  <z...@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  <cgar...@igalia.com>
 
         Report the initiating url instead of the redirected one

Modified: trunk/LayoutTests/TestExpectations (286099 => 286100)


--- trunk/LayoutTests/TestExpectations	2021-11-21 16:51:11 UTC (rev 286099)
+++ trunk/LayoutTests/TestExpectations	2021-11-21 18:22:01 UTC (rev 286100)
@@ -4322,8 +4322,6 @@
 webkit.org/b/209460 imported/w3c/web-platform-tests/css/css-grid/abspos/descendant-static-position-002.html [ ImageOnlyFailure ]
 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 ]
 webkit.org/b/212246 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-item-content-baseline-001.html [ ImageOnlyFailure ]

Modified: trunk/Source/WebCore/ChangeLog (286099 => 286100)


--- trunk/Source/WebCore/ChangeLog	2021-11-21 16:51:11 UTC (rev 286099)
+++ trunk/Source/WebCore/ChangeLog	2021-11-21 18:22:01 UTC (rev 286100)
@@ -1,3 +1,28 @@
+2021-11-21  Ziran Sun  <z...@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  <za...@apple.com>
 
         [LFC][IFC] Introduce canCacheMeasuredWidthOnInlineTextItem

Modified: trunk/Source/WebCore/rendering/RenderReplaced.cpp (286099 => 286100)


--- trunk/Source/WebCore/rendering/RenderReplaced.cpp	2021-11-21 16:51:11 UTC (rev 286099)
+++ trunk/Source/WebCore/rendering/RenderReplaced.cpp	2021-11-21 18:22:01 UTC (rev 286100)
@@ -531,6 +531,15 @@
     return LayoutUnit(round(logicalHeight * aspectRatio));
 }
 
+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
 {
     if (style().logicalWidth().isSpecified() || style().logicalWidth().isIntrinsic())
@@ -553,7 +562,7 @@
         // For flex or grid items where the logical height has been overriden then we should use that size to compute the replaced width as long as the flex or
         // 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);
 
         // If 'height' and 'width' both have computed values of 'auto' and the element also has an intrinsic width, then that intrinsic width is the used value of 'width'.
@@ -623,7 +632,7 @@
     bool hasIntrinsicWidth = constrainedSize.hasIntrinsicWidth || constrainedSize.width() > 0;
 
     // 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)));
 
     // If 'height' and 'width' both have computed values of 'auto' and the element also has an intrinsic height, then that intrinsic height is the used value of 'height'.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to