Skip to content

Commit

Permalink
[css-grid] Correctly stretch a replaced element in both axes
Browse files Browse the repository at this point in the history
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 <cbiesinger@chromium.org>
Reviewed-by: Oriol Brufau <obrufau@igalia.com>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#848107}
GitOrigin-RevId: 2ba07b2385593e296bb69271b4c27402a6d929c3
  • Loading branch information
cbiesinger authored and copybara-github committed Jan 28, 2021
1 parent 049a80c commit c4359c5
Show file tree
Hide file tree
Showing 12 changed files with 102 additions and 1 deletion.
31 changes: 30 additions & 1 deletion blink/renderer/core/layout/layout_box.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions blink/renderer/core/layout/layout_box.h
Original file line number Diff line number Diff line change
Expand Up @@ -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&,
Expand Down
8 changes: 8 additions & 0 deletions blink/web_tests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
Expand Down Expand Up @@ -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 ]
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions blink/web_tests/external/wpt/lint.ignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c4359c5

Please sign in to comment.