Bug 216142

Summary: [css-grid] Growth limits not increased with min-content contributions of spanning items
Product: WebKit Reporter: Oriol Brufau <obrufau>
Component: CSSAssignee: Oriol Brufau <obrufau>
Status: RESOLVED FIXED    
Severity: Normal CC: changseok, clopez, darin, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, pdr, rego, svillar, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.chromium.org/p/chromium/issues/detail?id=1122084
Bug Depends on:    
Bug Blocks: 163283    
Attachments:
Description Flags
Patch
none
Patch none

Description Oriol Brufau 2020-09-03 14:16:06 PDT 
By mistake the specification used to say that, for items spanning multiple tracks, the growth limits of the tracks with an intrinsic max track sizing function should grow to accommodate the minimum contribution of the item.

But this was a mistake, because an intrinsic max track sizing function can only be min-content or max-content. So instead of distributing the minimum contribution, it should be the min-content contribution.

The spec has been fixed and there is a CSSWG resolution in https://github.com/w3c/csswg-drafts/issues/4790

Sadly r424527 implemented the old text, it should be reverted. The change will likely be web compatible, since it only affects an edge case with 'minmax()' where the min track sizing function is a fixed value smaller than the min-content contribution, the max track sizing function is 'min-content', and an item whose minimum contribution is forced to be different than the min-content contribution, and spans multiple tracks. Should rarely happen.

Firefox is also affected: https://bugzilla.mozilla.org/show_bug.cgi?id=1655581
Chromium was fixed in https://bugs.chromium.org/p/chromium/issues/detail?id=1122084
Comment 1 Oriol Brufau 2020-09-03 14:48:56 PDT 
Created attachment 407917 [details]
Patch
Comment 2 Darin Adler 2020-09-03 17:08:55 PDT 
Comment on attachment 407917 [details]
Patch

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

> LayoutTests/fast/css-grid-layout/grid-intrinsic-maximums-expected.html:55
> +<!-- Fails due to https://bugs.webkit.org/show_bug.cgi?id=216144 -->

Commenting out tests that will fail is not the usual process in WebKit. Instead we have an expected result that includes the expected failures. Is there a reason to take a different approach here?
Comment 3 Oriol Brufau 2020-09-03 17:41:41 PDT 
Created attachment 407937 [details]
Patch
Comment 4 Oriol Brufau 2020-09-03 17:45:53 PDT 
(In reply to Oriol Brufau from comment #0)
> Sadly r424527 implemented the old text
Actually it was r207290.

(In reply to Darin Adler from comment #2)
> Commenting out tests that will fail is not the usual process in WebKit.
> Instead we have an expected result that includes the expected failures. Is
> there a reason to take a different approach here?
I did it like this because before the change that I'm reverting (r207290), some cases were also commented out.

But sure, I have uncommented the failures and changed the expectations with a comment to bug 216144.
Comment 5 EWS 2020-09-05 17:26:27 PDT 
Committed r266675: <https://trac.webkit.org/changeset/266675>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407937 [details].
Comment 6 Radar WebKit Bug Importer 2020-09-05 17:27:23 PDT 
<rdar://problem/68407848>