[css-flexbox][css-grid] Intrinsic sizing with ∑flex < 1 is broken

The Intrinsic Main Sizes algorithm in Flexbox fails to handle cases where the sum of the flex factors is less than one. It creates a container that fits too snugly, so when the flex algorithm is run to set the sizes of the items, they are too small: essentially the multipliers are multiplied twice.

We should fix this, and for Grid, too.

I think the correct fix to Flexbox is to add

The flex container’s max-content size is the largest sum of the afore-calculated sizes of all items within a single line. If the sum of the flex factors on that line is less than one, however, additionally divide this sum by the sum of the flex factors.

Here's a well-formatted testcase showing off the behavior in Flexbox and Grid: https://jsfiddle.net/Lbvz57zj/4/

Chrome's behavior:

Grid, sum < 1: double-applies the fractions. Each items' max-content size is 200px, so when the floated grid container is sizing itself, they report 40% (80px) and 50% (100px), so the grid ends up 180px wide. Then the columns are 40% and 50% of 180px, resulting in values smaller than originally requested.
Grid, sum >= 1: correct behavior. The items report 50% (100px) and 100% (200px), so the grid ends up 300px wide. The columns then take 1/3 and 2/3 of that size (proper re-scaled shares), resulting in the exact sizes they requested earlier.
Flexbox, either case: The items just report their max-content size (200px each), so the flexbox is 400px wide. The items then size correctly within that size.

Firefox's behavior:

Everything: acts the same as Chrome/Flexbox; the container is 400px wide, the items size correctly within that size.

Ideal behavior for both Grid and Flexbox:

Sum < 1: Items report 40% (80px) and 50% (100px). The sum is < 1, so this sum gets rescaled to 180px / .9 = 200px, and the container ends up 200px wide. The items then claim 40% and 50% of that size, resulting in them being 80px and 100px.
Sum > 1: Items report 50% (100px) and 100% (200px). The sum is > 1, so the container sizes itself to that exactly and ends up 300px wide. The items then claim 1/3 and 2/3 of that space, resulting in them being 100px and 200px.

Actually, that fix is only correct if the flex basis is zero. We need to do something a little more complicated here.

OK, new fix:

For each flex item, subtract its outer flex base size from its max-content contribution size. If that result is not zero, divide it by (if the result was positive) its flex grow factor floored at 1, or (if the result was negative) by its scaled flex shrink factor, having floored the flex shrink factor at 1. This is the item’s max-content flex fraction.

Place all flex items into lines of infinite length.

Within each line, find the largest max-content flex fraction among all the flex items. If the sum of the flex grow factors (flex shrink factors, if the largest max-content flex fraction was negative) on that line is less than one, divide each flex grow factor by that sum to rescale their total to one.

Add each item’s flex base size to the product of its rescaled flex grow factor (or rescaled scaled flex shrink factor, if the chosen max-content flex fraction was negative) and the chosen max-content flex fraction, then clamp that result by the max main size property floored by the min main size property.

The flex container’s max-content size is the largest sum of the afore-calculated sizes of all items within a single line.

(Yes, “rescaled scaled” sounds a bit silly.)

Okay, so example problem showing off non-zero base sizes:

Two flex items, both have a max-content size of 200px, both have flex: .4. First item has flex-basis: 100px, second has flex-basis: 300px.

Ideal outcome: First item wants to grow, second wants to shrink, so per standard flex rules, we're growing. It wants to get 40% of the way toward its ideal (max-content) width, so should land at 140px. Second item wants to grow the same amount, so it should land at 340px.

("Idealness" is defined as "gives reasonable continuous behavior as the flex values change from 0 to >=1" and vice-versa. A max-content flexbox with all inflexible items should definitely just sum the flex basises of its items, and not pay attention to their max-content size at all.)

Algo walkthrough:

First item's max-content flex fraction is (200px - 100px) / max(.4, 1), or 100px. (.4 from its flex-grow.) Second item's max-content flex fraction is (200px - 300px) / max(1, 1), or -100px. (1 from its flex-shrink.)
Both items are on the same line.
Largest mcff is 100px, so we use that. Sum of flex grow factors (since it's positive) is .8, so we rescale each item's flex grow factors to sum to 1 - they both become .5.
First item becomes 100px + (100px * .5) = 150px. Second item becomes 300px + (100px * .5) = 350px. No additional clamping for this example.
Flex container is thus 150px + 350px = 500px wide.

Then we actually run flex layout inside that flex container. Container is 500px wide, sum of base sizes is 400px, free space is 100px. Items thus grow. Both have a flex-grow factor of .4, so they each get 40px of space added to their base size, resulting in 140px and 340px. Success!

Just ran the above example for differing flex-grow factors as well; specifically, factors of .4 and .5. End result is that step 4 yields a size of 144.444px for the first item and 355.555px for the second, which still sums to 500px. Then layout runs, yielding a size of 140px and 350px, which is again exactly the ideal size we calculate beforehand.

Wording proposal for Grid:

The max-content size of a grid container is the sum of the grid container’s track sizes (including gutters) in the appropriate axis, when the grid is sized under a max-content constraint. However, if the sum of the flex factors of the flexible tracks is less than one, then instead calculate the max-content size of the grid container with those tracks using rescaled flex factors--each flex factor divided by the sum of the flex factors--in place of their actual flex factors.

(And I've run this Grid text thru the same examples - as far as I can tell, Chrome's current behavior is exactly correct per spec, and this edit will make the sum<1 case match our ideal behavior.)

The Working Group just discussed Intrinsic sizing with ∑flex < 1 is broken, and agreed to the following resolutions:

RESOLVED: take this edit as soon as dholbert gives approval

The full IRC log of that discussion

<dael> Topic: Intrinsic sizing with ∑flex < 1 is broken
<dael> Github: https://github.com//issues/1735
<dael> fantasai: There was an error in the algo for flexing for intrinisic size. We look at flexing algo in compo with min/max sizes so we give continer enough space so when flexing algo takes place min/max are respected. The error was when flex values add up to less than one.
<dael> fantasai: TabAtkins and I proposed fixes, we think are correct. We're happy to edit them in. If anyone wants to review, great. We're pretty confident.
<dael> dbaron: Are impl signed up to make thesE? WEll, does change match existing impl?
<dael> fantasai: Current impl don't account for the flexing in calc of intrinisc. For grid we have impl that do that. THis change is fixing a behavior that doesn't seem right. I think chrome is the one that does that...I don't remember other impl.
<bkardell_> present +bkardell_
<dael> dbaron: My worry is if somebody tries to impl do websites depend on old
<dael> fantasai: There's 2 steps of impl. 1 is if you have the algo tweak it. That would be to grid where I don't think we have any dependency. I think for sure we should fix that. For flexbox the algo overall isn't impl so changing the algo won't affect impl.
<dael> fantasai: dholbert has volunteered to impl the algo, but he hasn't gotten to it.
<dael> TabAtkins: They impl a simplier version of the algo and they are consistant in that, but it's wrong and dholbert is offering to fix that.
<dael> fantasai: And that will make it more consistant with grid
<dael> dbaron: And if dholbert succeeds will other impl follow?
<dael> TabAtkins: Yes, we will
<dael> astearns: Has dholbert reviewed this proposed change?
<dael> TabAtkins: We pinged him, he hasn't yet been able to do a full review.
<dael> fantasai: It's not difficult to do, it's a tweek to some math. Largely it's a q of did we get the math right.
<TabAtkins> (Re: compat, we implement an *almost* correct version of this for Grid already, the tweak to fix it only changes grids with a sum of fr < 1.)
<dael> astearns: I'd be okay with getting it edited in, but I'm a little concerned no one has checked the math. I'd be happier if someone looked and said yay or nay
<dael> fantasai: We can resolve and say it takes effect once verified.
<dael> astearns: BUt that doesn't get us closer to pub
<dael> fantasai: It does b/c if we replies today or tomorrow we can get it into pipeline
<TabAtkins> (Our Flexbox impl is currently the "simple and dumb" version, which matches what Firefox currently does for both Grid and Flexbox; if dholbert fixes in Gecko and it sticks, we're happy to change ours to match.)
<dael> astearns: Does it make sense to publish even if this doesn't make it?
<dael> fantasai: Yeah, there's a ton of changes. THis is a CR so changes pub takes 6 weeks. Might as well start.
<dael> Chris: If it takes 6 weeks is dependant on how good DoC and changes are and availability.
<dael> fantasai: DoC is compiled.
<fantasai> s/availability/availability of Ralph and plh/
<dael> astearns: Any other comments on resolving or resolve pending dholbert?
<dael> fantasai: I'd prefer to get resolutions in place b/c that will get things rolling
<dael> Chris: I agree.
<dael> astearns: You propose to resolve we take this edit as soon as dholbert gives a thumbs up. Obj?
<dael> RESOLVED: take this edit as soon as dholbert gives approval

The relevant text for Grid was reverted and the result of the final text looks good to me.

All right, nits fixed. Thanks, y'all!

[css-flexbox] Address nits by dholbert in #1735, and fix minor linkin…

[css-flexbox-1] Clean up wording for dividing flex factor in finding …

Navigation Menu

Repository navigation

[css-flexbox][css-grid] Intrinsic sizing with ∑flex < 1 is broken #1735

35 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[css-flexbox][css-grid] Intrinsic sizing with ∑flex < 1 is broken #1735

Description

Activity

fantasai commented on Sep 11, 2017

tabatkins commented on Sep 11, 2017

fantasai commented on Sep 11, 2017

fantasai commented on Sep 11, 2017

tabatkins commented on Sep 11, 2017

tabatkins commented on Sep 11, 2017

fantasai commented on Sep 11, 2017

tabatkins commented on Sep 11, 2017

css-meeting-bot commented on Sep 13, 2017

35 remaining items

mrego commented on Oct 9, 2017

tabatkins commented on Oct 13, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions