Bug 237628

Summary:

[css-grid] Subgrid items should always be stretched

Product:

WebKit

Reporter:

Matt Woodrow <mattwoodrow>

Component:

Layout and Rendering

Assignee:

Matt Woodrow <mattwoodrow>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, changseok, clopez, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, ntim, obrufau, pdr, rego, simon.fraser, svillar, webkit-bug-importer, youennf, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://github.com/web-platform-tests/wpt/pull/33121

Attachments:

Description	Flags
Patch	none
Patch	zalan: review+
Patch for landing	mattwoodrow: commit-queue+
Patch for landing	none

Description Matt Woodrow 2022-03-08 16:29:00 PST

9.j: The subgrid is always stretched in its subgridded dimension(s): the align-self/justify-self properties on it are ignored, as are any specified width/height constraints.

We need to make sure that we always stretch subgrid items, even if it means overriding a specified width/height.

Comment 1 Radar WebKit Bug Importer 2022-03-08 16:29:13 PST

<rdar://problem/89996514>

Comment 2 Matt Woodrow 2022-03-08 19:29:42 PST

Created attachment 454186 [details]
Patch

Comment 3 Matt Woodrow 2022-03-08 19:54:30 PST

Created attachment 454188 [details]
Patch

Comment 4 EWS Watchlist 2022-03-08 19:56:55 PST

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

Comment 5 Tim Nguyen (:ntim) 2022-03-11 01:26:23 PST

Comment on attachment 454188 [details]
Patch

Upstream WPT doesn't seem happy about some trailing whitespace in your test, can you remove them?

See: https://community-tc.services.mozilla.com/tasks/fKANgyH9StmuqxCzp54LiA/runs/0/logs/public/logs/live.log#L108-110

Comment 6 zalan 2022-03-16 20:45:47 PDT

Comment on attachment 454188 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=454188&action=review

> Source/WebCore/rendering/RenderBox.cpp:2885
> -        return style.resolvedJustifySelf(&containingBlock->style(), containingBlock->selfAlignmentNormalBehavior(this)).position() == ItemPosition::Stretch;
> -    return style.resolvedAlignSelf(&containingBlock->style(), containingBlock->selfAlignmentNormalBehavior(this)).position() == ItemPosition::Stretch;
> +        return resolvedJustifySelf(&containingBlock->style(), containingBlock->selfAlignmentNormalBehavior(this)).position() == ItemPosition::Stretch;
> +    return resolvedAlignSelf(&containingBlock->style(), containingBlock->selfAlignmentNormalBehavior(this)).position() == ItemPosition::Stretch;

I think it's ok to just check/downcast to RenderGrid and run the grid specific code in here. While I personally dislike the way we structure all these different layout systems, I think that's what the current pattern here is (same for width)

> Source/WebCore/rendering/RenderGrid.cpp:1208
> +    GridTrackSizingDirection childFlowDirection = GridLayoutFunctions::flowAwareDirectionForChild(*this, child, direction);

auto?

> Source/WebCore/rendering/RenderGrid.cpp:1292
> +{

I'd just early return on a non-grid child.

> Source/WebCore/rendering/RenderGrid.cpp:1295
> +        GridTrackSizingDirection childBlockDirection = GridLayoutFunctions::flowAwareDirectionForChild(*this, child, ForRows);
> +        LayoutUnit stretchedLogicalHeight = availableAlignmentSpaceForChildBeforeStretching(GridLayoutFunctions::overridingContainingBlockContentSizeForChild(child, childBlockDirection).value(), child, ForRows);

autos

Comment 7 Matt Woodrow 2022-03-17 13:51:23 PDT

Created attachment 455023 [details]
Patch for landing

Comment 8 Matt Woodrow 2022-03-17 15:42:14 PDT

Created attachment 455039 [details]
Patch for landing

Comment 9 EWS 2022-03-17 19:34:49 PDT

Committed r291464 (248580@main): <https://commits.webkit.org/248580@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455039 [details].