[css-grid] Correctly stretch a replaced element in both axes

As discussed in w3c/csswg-drafts#5713 Change-Id: I90c8bd9c5516d11cae6798d7d7cb6c679e166994 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2651651 Commit-Queue: Christian Biesinger <[email protected]> Reviewed-by: Oriol Brufau <[email protected]> Reviewed-by: Javier Fernandez <[email protected]> Reviewed-by: Ian Kilpatrick <[email protected]> Auto-Submit: Christian Biesinger <[email protected]> Cr-Commit-Position: refs/heads/master@{#848107} GitOrigin-RevId: 2ba07b2385593e296bb69271b4c27402a6d929c3
mjfroman · Jan 28, 2021 · c4359c5 · c4359c5
1 parent 049a80c
commit c4359c5
Show file tree

Hide file tree

Showing 12 changed files with 102 additions and 1 deletion.
diff --git a/blink/renderer/core/layout/layout_box.cc b/blink/renderer/core/layout/layout_box.cc
@@ -4118,7 +4118,9 @@ void LayoutBox::ComputeLogicalWidth(
     computed_values.extent_ =
         ComputeReplacedLogicalWidth() + BorderAndPaddingLogicalWidth();
   } else if (StyleRef().LogicalWidth().IsAuto() &&
-      ComputeLogicalWidthFromAspectRatio(&computed_values.extent_)) {
+             (!IsGridItem() || !HasStretchedLogicalWidth() ||
+              !HasStretchedLogicalHeight()) &&
+             ComputeLogicalWidthFromAspectRatio(&computed_values.extent_)) {
     /* we're good */
   } else {
     LayoutUnit preferred_width = ComputeLogicalWidthUsing(
@@ -4356,6 +4358,33 @@ bool LayoutBox::HasStretchedLogicalWidth() const {
              .GetPosition() == ItemPosition::kStretch;
 }
 
+// TODO (lajava) Can/Should we move this inside specific layout classes (flex.
+// grid)? Can we refactor columnFlexItemHasStretchAlignment logic?
+bool LayoutBox::HasStretchedLogicalHeight() const {
+  NOT_DESTROYED();
+  const ComputedStyle& style = StyleRef();
+  if (!style.LogicalHeight().IsAuto() || style.MarginBefore().IsAuto() ||
+      style.MarginAfter().IsAuto())
+    return false;
+  LayoutBlock* cb = ContainingBlock();
+  if (!cb) {
+    // 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 (cb->IsHorizontalWritingMode() != IsHorizontalWritingMode()) {
+    return style
+               .ResolvedJustifySelf(cb->SelfAlignmentNormalBehavior(this),
+                                    cb->Style())
+               .GetPosition() == ItemPosition::kStretch;
+  }
+  return style
+             .ResolvedAlignSelf(cb->SelfAlignmentNormalBehavior(this),
+                                cb->Style())
+             .GetPosition() == ItemPosition::kStretch;
+}
+
 bool LayoutBox::SizesLogicalWidthToFitContent(
     const Length& logical_width) const {
   NOT_DESTROYED();

diff --git a/blink/renderer/core/layout/layout_box.h b/blink/renderer/core/layout/layout_box.h
@@ -2117,6 +2117,7 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject {
   bool ColumnFlexItemHasStretchAlignment() const;
   bool IsStretchingColumnFlexItem() const;
   bool HasStretchedLogicalWidth() const;
+  bool HasStretchedLogicalHeight() const;
 
   void ExcludeScrollbars(
       PhysicalRect&,

diff --git a/blink/web_tests/TestExpectations b/blink/web_tests/TestExpectations
@@ -3329,6 +3329,10 @@ crbug.com/1045599 external/wpt/css/css-grid/alignment/grid-alignment-implies-siz
 crbug.com/941987 external/wpt/css/css-grid/alignment/grid-baseline-align-cycles-001.html [ Failure ]
 crbug.com/1045599 external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-001.html [ Failure ]
 crbug.com/1045599 external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-002.html [ Failure ]
+crbug.com/1171262 external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-006.html [ Failure ]
+crbug.com/1171262 external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-007.html [ Failure ]
+crbug.com/1171262 external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-008.html [ Failure ]
+crbug.com/1171262 external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-009.html [ Failure ]
 crbug.com/759665 external/wpt/css/css-grid/animation/grid-template-columns-001.html [ Failure ]
 crbug.com/759665 external/wpt/css/css-grid/animation/grid-template-rows-001.html [ Failure ]
 crbug.com/935102 external/wpt/css/css-grid/layout-algorithm/grid-flex-track-intrinsic-sizes-002.html [ Failure ]
@@ -3382,6 +3386,10 @@ crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/alignment/gri
 crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/alignment/grid-alignment-implies-size-change-036.html [ Pass ]
 crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-001.html [ Pass ]
 crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-002.html [ Pass ]
+crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-006.html [ Pass ]
+crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-007.html [ Pass ]
+crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-008.html [ Pass ]
+crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-009.html [ Pass ]
 
 # Tests that pass with layout-ng-grid enabled but fail in legacy grid:
 virtual/layout-ng-grid/external/wpt/css/css-grid/abspos/descendant-static-position-003.html [ Pass ]

diff --git a/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-004.html b/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-004.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
+<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/5713">
+<meta name="assert" content="If stretch alignment is applied to both axis the aspect-ratio of a replaced element is ignored.">
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+
+<div style="display: grid; grid-template: 100px / 100px; width: 100px; height: 100px; background: red;">
+  <img src="support/25x50-green.png" width=25 height=50 style="background: green; align-self: stretch; justify-self: stretch; width: auto; height: auto;"></img>
+</div>
diff --git a/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-005.html b/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-005.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
+<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/5713">
+<meta name="assert" content="If stretch alignment is applied to both axis the aspect-ratio of a replaced element is ignored.">
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+
+<div style="display: grid; grid-template: 100px / 100px; width: 100px; height: 100px; background: red;">
+  <img src="support/25x50-green.png" width=25 height=50 style="background: green; align-self: stretch; justify-self: stretch; width: auto; height: auto; writing-mode: vertical-lr;"></img>
+</div>
diff --git a/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-006.html b/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-006.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
+<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/5713">
+<meta name="assert" content="If stretch alignment is applied to both axis the aspect-ratio of a replaced element is ignored, but not when auto margins are present.">
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+
+<div style="display: grid; grid-template: 100px / 500px; width: 100px; height: 100px; background: red;">
+  <img src="support/50x50-green.png" width=50 height=50 style="background: green; align-self: stretch; justify-self: stretch; width: auto; height: auto; margin-right: auto;"></img>
+</div>
diff --git a/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-007.html b/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-007.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
+<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/5713">
+<meta name="assert" content="If stretch alignment is applied to both axis the aspect-ratio of a replaced element is ignored, but not when auto margins are present.">
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+
+<div style="display: grid; grid-template: 500px / 100px; width: 100px; height: 100px; background: red;">
+  <img src="support/50x50-green.png" width=50 height=50 style="background: green; align-self: stretch; justify-self: stretch; width: auto; height: auto; margin-bottom: auto;"></img>
+</div>
diff --git a/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-008.html b/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-008.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
+<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/5713">
+<meta name="assert" content="Stretch alignment in one axis should apply aspect-ratio in the other">
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+
+<div style="display: grid; grid-template: 100px / 100px; width: 100px; height: 100px; background: red;">
+  <img src="support/50x50-green.png" width=50 height=50 style="background: green; justify-self: stretch; width: auto; height: auto;"></img>
+</div>
diff --git a/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-009.html b/...b_tests/external/wpt/css/css-grid/alignment/replaced-alignment-with-aspect-ratio-009.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
+<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/5713">
+<meta name="assert" content="Stretch alignment in one axis should apply aspect-ratio in the other">
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+
+<div style="display: grid; grid-template: 100px / 100px; width: 100px; height: 100px; background: red;">
+  <img src="support/50x50-green.png" width=50 height=50 style="background: green; align-self: stretch; width: auto; height: auto;"></img>
+</div>
diff --git a/blink/web_tests/external/wpt/css/css-grid/alignment/support/25x50-green.png b/blink/web_tests/external/wpt/css/css-grid/alignment/support/25x50-green.png
diff --git a/blink/web_tests/external/wpt/css/css-grid/alignment/support/50x50-green.png b/blink/web_tests/external/wpt/css/css-grid/alignment/support/50x50-green.png
diff --git a/blink/web_tests/external/wpt/lint.ignore b/blink/web_tests/external/wpt/lint.ignore
@@ -613,6 +613,9 @@ CSS-COLLIDING-SUPPORT-NAME: css/css-backgrounds/support/50x50-green.png
 CSS-COLLIDING-SUPPORT-NAME: css/css-grid/grid-items/support/50x50-green.png
 CSS-COLLIDING-SUPPORT-NAME: css/CSS2/support/50x50-green.png
 CSS-COLLIDING-SUPPORT-NAME: css/CSS2/ui/support/animated.gif
+CSS-COLLIDING-SUPPORT-NAME: css/css-grid/alignment/support/50x50-green.png
+CSS-COLLIDING-SUPPORT-NAME: css/css-grid/alignment/support/25x50-green.png
+CSS-COLLIDING-SUPPORT-NAME: css/css-grid/grid-items/support/25x50-green.png
 CSS-COLLIDING-SUPPORT-NAME: css/CSS2/backgrounds/support/animated.gif
 CSS-COLLIDING-SUPPORT-NAME: css/css-shapes/shape-outside/shape-image/support/animated.gif
 CSS-COLLIDING-SUPPORT-NAME: css/css-display/support/util.js