Bug 216142 - [css-grid] Growth limits not increased with min-content contributions of spanning items
Summary: [css-grid] Growth limits not increased with min-content contributions of span...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oriol Brufau
URL:
Keywords: InRadar
Depends on:
Blocks: 163283
  Show dependency treegraph
 
Reported: 2020-09-03 14:16 PDT by Oriol Brufau
Modified: 2020-09-05 17:27 PDT (History)
13 users (show)

See Also:


Attachments
Patch (18.52 KB, patch)
2020-09-03 14:48 PDT, Oriol Brufau
no flags Details | Formatted Diff | Diff
Patch (16.84 KB, patch)
2020-09-03 17:41 PDT, Oriol Brufau
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>