Changeset 280022 in webkit

Timestamp:

Jul 19, 2021 1:39:01 AM (12 months ago)

Author:

Ziran Sun

Message:

[CSS-grid] Ignore the aspect-ratio of a replaced element if stretch alignments are applied to both axes
​https://bugs.webkit.org/show_bug.cgi?id=227573

Reviewed by Javier Fernandez.

Source/WebCore:

As discussed in ​https://github.com/w3c/csswg-drafts/issues/5713, for the replaced element as a grid item,
when both axes have stretch alignments applied and there is no auto margin(s) presented, the aspect ratio
should be ignored if there is any.

Part of this patch is an import of Chromium CL at
​https://chromium-review.googlesource.com/c/chromium/src/+/2651651

• rendering/RenderBox.cpp:

(WebCore::RenderBox::hasStretchedLogicalHeight const):
(WebCore::RenderBox::shouldComputeLogicalWidthFromAspectRatio const):

• rendering/RenderBox.h:

LayoutTests:

Two grid WPT tests are now passing.

• TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderBox.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-07-19  Ziran Sun  <zsun@igalia.com>
+
+        [CSS-grid] Ignore the aspect-ratio of a replaced element if stretch alignments are applied to both axes
+        https://bugs.webkit.org/show_bug.cgi?id=227573
+
+        Reviewed by Javier Fernandez.
+
+        Two grid WPT tests are now passing.
+
+        * TestExpectations:
+
 2021-07-16  Simon Fraser  <simon.fraser@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -1435 +1435 @@
 imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-001.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-002.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-004.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-005.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-006.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-007.html [ ImageOnlyFailure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-07-19  Ziran Sun  <zsun@igalia.com>
+
+        [CSS-grid] Ignore the aspect-ratio of a replaced element if stretch alignments are applied to both axes
+        https://bugs.webkit.org/show_bug.cgi?id=227573
+
+        Reviewed by Javier Fernandez.
+
+        As discussed in https://github.com/w3c/csswg-drafts/issues/5713, for the replaced element as a grid item,
+        when both axes have stretch alignments applied and there is no auto margin(s) presented, the aspect ratio
+        should be ignored if there is any. 
+
+        Part of this patch is an import of Chromium CL at
+        https://chromium-review.googlesource.com/c/chromium/src/+/2651651
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::hasStretchedLogicalHeight const):
+        (WebCore::RenderBox::shouldComputeLogicalWidthFromAspectRatio const):
+        * rendering/RenderBox.h:
+
 2021-07-18  Sam Weinig  <weinig@apple.com>
 

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -2761 +2761 @@
 
 // FIXME: Can/Should we move this inside specific layout classes (flex. grid)? Can we refactor columnFlexItemHasStretchAlignment logic?
+bool RenderBox::hasStretchedLogicalHeight() const
+{
+    auto& style = this->style();
+    if (!style.logicalHeight().isAuto() || style.marginBefore().isAuto() || style.marginAfter().isAuto())
+        return false;
+    RenderBlock* containingBlock = this->containingBlock();
+    if (!containingBlock) {
+        // We are evaluating align-self/justify-self, which default to 'normal' for the root element.
+        // The 'normal' value behaves like 'start' except for Flexbox Items, which obviously should have a container.
+        return false;
+    }
+    if (containingBlock->isHorizontalWritingMode() != isHorizontalWritingMode())
+        return style.resolvedJustifySelf(&containingBlock->style(), containingBlock->selfAlignmentNormalBehavior(this)).position() == ItemPosition::Stretch;
+    return style.resolvedAlignSelf(&containingBlock->style(), containingBlock->selfAlignmentNormalBehavior(this)).position() == ItemPosition::Stretch;
+}
+
+// FIXME: Can/Should we move this inside specific layout classes (flex. grid)? Can we refactor columnFlexItemHasStretchAlignment logic?
 bool RenderBox::hasStretchedLogicalWidth() const
 {
@@ -5252 +5269 @@
         return false;
 
+    if (isGridItem() && shouldComputeSizeAsReplaced() && hasStretchedLogicalWidth() && hasStretchedLogicalHeight())
+        return false;
+
     auto isResolvablePercentageHeight = [&] {
         return style().logicalHeight().isPercentOrCalculated() && (isOutOfFlowPositioned() || percentageLogicalHeightIsResolvable());

trunk/Source/WebCore/rendering/RenderBox.h

@@ -437 +437 @@
     bool sizesLogicalWidthToFitContent(SizeType) const;
 
+    bool hasStretchedLogicalHeight() const;
     bool hasStretchedLogicalWidth() const;
     bool isStretchingColumnFlexItem() const;