From 30488d91ec05b3eca4f40795372545a0f0917442 Mon Sep 17 00:00:00 2001 From: "zsun@igalia.com" Date: Mon, 19 Jul 2021 08:39:01 +0000 Subject: [PATCH] [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: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@280022 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 11 +++++++++++ LayoutTests/TestExpectations | 2 -- Source/WebCore/ChangeLog | 19 +++++++++++++++++++ Source/WebCore/rendering/RenderBox.cpp | 20 ++++++++++++++++++++ Source/WebCore/rendering/RenderBox.h | 1 + 5 files changed, 51 insertions(+), 2 deletions(-) diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 1ef3d39e45e86..2bf228fa78a96 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,14 @@ +2021-07-19 Ziran Sun + + [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 getBoundingClientRect() returns the incorrect rectangle on elements whose parent element is set -webkit-column-count diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations index 810a55ee373b6..9739083531b6d 100644 --- a/LayoutTests/TestExpectations +++ b/LayoutTests/TestExpectations @@ -1434,8 +1434,6 @@ webkit.org/b/216146 imported/w3c/web-platform-tests/css/css-grid/alignment/grid- webkit.org/b/216146 imported/w3c/web-platform-tests/css/css-grid/alignment/grid-baseline-justify-001.html [ ImageOnlyFailure ] 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 ] imported/w3c/web-platform-tests/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-008.html [ ImageOnlyFailure ] diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 48dec698125ab..3c4a1e0d97a49 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,22 @@ +2021-07-19 Ziran Sun + + [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 Fix canvas overflow checking to use CheckedArithmatic rather than adhoc floating point mechanism diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp index 0ff218fe663d5..0ac0f133b3f3f 100644 --- a/Source/WebCore/rendering/RenderBox.cpp +++ b/Source/WebCore/rendering/RenderBox.cpp @@ -2759,6 +2759,23 @@ bool RenderBox::isStretchingColumnFlexItem() const return false; } +// 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 { @@ -5251,6 +5268,9 @@ bool RenderBox::shouldComputeLogicalWidthFromAspectRatio() const if (shouldIgnoreAspectRatio()) return false; + if (isGridItem() && shouldComputeSizeAsReplaced() && hasStretchedLogicalWidth() && hasStretchedLogicalHeight()) + return false; + auto isResolvablePercentageHeight = [&] { return style().logicalHeight().isPercentOrCalculated() && (isOutOfFlowPositioned() || percentageLogicalHeightIsResolvable()); }; diff --git a/Source/WebCore/rendering/RenderBox.h b/Source/WebCore/rendering/RenderBox.h index 408ed1a3d443b..7718a4ada82c9 100644 --- a/Source/WebCore/rendering/RenderBox.h +++ b/Source/WebCore/rendering/RenderBox.h @@ -436,6 +436,7 @@ override; // of a containing block). HTML4 buttons, s, legends, and floating/compact elements do this. bool sizesLogicalWidthToFitContent(SizeType) const; + bool hasStretchedLogicalHeight() const; bool hasStretchedLogicalWidth() const; bool isStretchingColumnFlexItem() const; bool columnFlexItemHasStretchAlignment() const;