Bug 237628

Summary: [css-grid] Subgrid items should always be stretched
Product: WebKit Reporter: Matt Woodrow <mattwoodrow>
Component: Layout and RenderingAssignee: 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: P2 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].