New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 654904 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 79180



Sign in to add a comment

[css-grid] Stretching flexible tracks doesn't work correctly when the grid container size is indefinite

Project Member Reported by mpalmg...@mozilla.com, Oct 11 2016

Issue description

Version: 55.0.2873.0 (Official Build) dev (64-bit)

What steps will reproduce the problem?
(1) load the attached testcase
(2)
(3)

What is the expected output?
The content box height of the grid container should be 83px,
first row: 10px, grid gap: 33px, last row: 40px.

What do you see instead?
63px

The 'containing block' to use for track sizing is the grid container's
content box.  The 'height' of the grid container isn't specified in
the testcase to it's indefinite and thus we should use the "If the free
space is an indefinite length" part of this algorithm:
https://drafts.csswg.org/css-grid/#algo-flex-tracks

Note that stretching flexible columns in a grid container with width:auto
in a block container with a specified width is different.  The grid containers
width *is* definite in this case because the grid container stretches to fill
its container in that axis.  It does *not* stretch-to-fill in the block axis
though and is therefore indefinite in that axis unless explicitly specified.
Grid containers are defined to behave as block boxes in that respect:
"As a block-level box in a block formatting context"
https://drafts.csswg.org/css-grid/#intrinsic-sizes
 
chrome-bug-row-flex-sizing.html
660 bytes View Download
The same problem occurs with flexible columns in a display:inline-grid.
Here, the grid container width should be 83px, but is 63px.

Firefox is handling both these cases correctly.
chrome-bug-inline-grid-column-flex-sizing.html
674 bytes View Download
Components: Blink>Layout>Grid

Comment 3 by svil...@igalia.com, Oct 13 2016

Status: Assigned (was: Untriaged)
OK. I'll take a look

Comment 4 by svil...@igalia.com, Oct 13 2016

I've examined the example and I think that Chrome is doing the right thing here.

We're also executing the "If the free space is an indefinite length" branch of the algorithm so the problem is not there. I'll try to explain our result so you could compare with FF.

Basically for flex tracks we have to compute the flex fraction and then multiply it by the flex factor of each track. The result of computing the flex fraction is 10px (due to tracks' min sizing function of 10px).

Then we just need to multiply it by the flex factors. Just doing it would result in a grid with 5px and 20px rows. But the specs clearly say "For each flexible track, if the product of the used flex fraction and the track’s flex factor is greater than the track’s base size, set its base size to that product." For the first row, as 5px < 10px then we need to leave the track unchanged. That leads to the final 10px 20px.

OK, good that we agree on using the "indefinite length" part of
the algorithm in this case.

So, what is the value that you calculate for 'flex fraction' in that algo?

The value I get is 20, which I got like this:
in "Each flexible track’s base size divided by its flex factor." we have:
  10px / 0.5 = 20
  10px / 2.0 = 5
in "The result of finding the size of an fr for each grid item..." we have:
https://drafts.csswg.org/css-grid/#algo-find-fr-size
  max-content contribution = 0px => "left over space" = 0 => 
  "hypothetical fr size" = 0 => restart algo with this track as inflexible =>
  0 - 10px = -10px => "hypothetical fr size" -10px / 2.5 = -4
(and something similar for track 2)

Then, back in 12.7, "The used 'flex fraction' is the maximum of:" results in
'flex fraction' = 20

Then we do "For each flexible track, if the product of the used 'flex fraction'
and the track’s 'flex factor' is greater than the track’s base size, set its
base size to that product."
  20 * 0.5 = 10px which is equal to it's current size => final result: 10px
  20 * 2.0 = 40px which is greater than its current size (10px) => final result: 40px

10px + 33px (grid-gap) + 40px = 83px

> (and something similar for track 2)
Sorry, I meant "item 2" there.

Comment 7 by r...@igalia.com, Oct 14 2016

Blocking: 79180
Cc: jfernan...@igalia.com r...@igalia.com
Owner: svil...@igalia.com

Comment 8 by svil...@igalia.com, Oct 25 2016

Status: WontFix (was: Assigned)
So the main difference is that we're correcting the flex factor when it's < 1 when computing the "Each flexible track’s base size divided by its flex factor." step. I was not able to find it in the specs but I found the email from Tab which explains why we should do that (perhaps they just forgot to add it to the spec).

The relevant email is this one https://lists.w3.org/Archives/Public/www-style/2015Sep/0032.html. As you could see, the math in the example is totally wrong so just skip it. I'm quoting here the interesting part:

    * First track yields an fr size of 40px.
    * Second track yields an fr size of 9px.
    * First item yields an fr size of 40px.
    * Second item yields an fr size of 0px.

    So, the size of an fr is 40px.

    * 40px*.5 is 20px, less than the first track's base size, so no change
    - it remains 40px.
    * 40px*2 is 80px, greater than the second track's base size, so its
    base size is set to 80px.

    This does change the ratio, from 4x to 2x.  But that's okay!  If you'd
    used 5fr and 20fr, that would maintain a 4x ratio, but we're dealing
    with something below 1.  We don't want a .01fr item to make the entire
    thing blow up; in particular, a 0fr item doesn't influence the size of
    the fr *at all*, and we should make sure that the behavior approaches
    that in the limit.  That's what we get here; .5fr has a lesser effect
    on the size of the fr than it would naively appear.

As you could see the idea is not to keep ratios for fr < 1 in order to provide an smooth transition to 0fr values (dividing by values close to 0 will rocket the value of the flex fraction artificially). That's why we "normalize" the flex fraction to 1 when the value is < 1. So for the very first computation the algo spits:

10px/ max(1,0.5) = 10
10px/ 2 = 5

So 10. It looks like FF is not applying any kind of protection and thus transitions from 1fr to 0fr in those cases become totally crazy. Check the attached file which is a slightly modified version of your original example. In the case of Chrome the result is quite compact but in FF the second item becomes really huge.
ff-bug-row-flex-sizing.html
649 bytes View Download
Status: Available (was: WontFix)
> 10px/ max(1,0.5) = 10

No, you're misreading the spec!

The 0.5 above is the 'flex factor' in "Each flexible track’s base size divided
by its flex factor" (which is what we're doing here) says nothing about using
1 for flex factors less than 1:
https://drafts.csswg.org/css-grid/#algo-flex-tracks

The part of the spec where that applies is the 'flex factor sum' in
"Let flex factor sum be the sum of the flex factors of the flexible tracks.
If this value is less than 1, set it to 1 instead."
https://drafts.csswg.org/css-grid/#algo-find-fr-size

The 'flex factor sum' in the attached example is 0.5 + 2 = 2.5 .
It's only when the *sum* of all the tracks' flex factors are less
than 1 that you should use 1 instead, so this rule doesn't affect
the attached example at all.
Please also note that the result in FF maintains the proportions
the author specified.  The result 10px 40px makes the first row 1/5
and the second 4/5, which corresponds to 0.5/2.5 and 2/2.5 .
(any result that doesn't maintain the proportions is clearly wrong)

Comment 11 by r...@igalia.com, Oct 25 2016

Cc: mpalmg...@mozilla.com tabatkins@chromium.org
It seems there's a disagreement here. The mail from Tab explicitly says:
"This does change the ratio, from 4x to 2x.  But that's okay!"

And Mats is suggesting that Firefox implementation is right because
it keeps the ratio.

Another thing is that the spec might still need to be updated,
Tab wrote at the end of the mail:
"So: I'll edit the spec to make the "indefinite" clause also clamp to 1."

@tabatkins, could you please take a look and clarify this issue? Thanks!

Comment 12 by svil...@igalia.com, Oct 26 2016

Mats I am not misreading it, note that I mentioned that "I was not able to find it in the specs but I found the email from Tab"

If you check Tab's answer, we don't want to keep the ratio in this case. The idea is that fractions in the 0..1 range should be less and less important for the final computation as they reach 0. Note that FF keeps increasing the size of the flex fraction as the fr value decreases, that's what the editors were trying to avoid.
Ugh, sorry I missed making that edit last year. Thanks so much for digging up the email, y'all; I've pushed the edit now, so the "indefinite" case is consistent with the "definite" one. Now, both have the proper limit behavior as you approach 0fr, and match the informal descriptions of the <1 behavior elsewhere in the spec.
Hmm, I really don't like that the flex tracks lose their relative proportions.
I also don't like that having indefinite/definite available space behaves
differently in this respect (as far as I'm aware, definite available space
always maintains the proportions).

Can someone please explain why we can't just scale up the factors so
that they all are >= 1 in the indefinite available size case?  That is,
if there are factors < 1, multiply all factors by "1 / (minimum flex factor)"
and proceed as if those scaled factors were the ones specified.

That should maintain the proportions and avoid blowing things up
for factors close to zero.  E.g. the attached testcase would
render identically to "minmax(10px, 1fr) minmax(10px, 4fr)".
> (as far as I'm aware, definite available space
> always maintains the proportions).

It doesn't - it just jumps straight into "finding the size of an fr", which has clamped <1 flex factors for a long time.  This actually *harmonizes* the two paths.

> Can someone please explain why we can't just scale up the factors so
> that they all are >= 1 in the indefinite available size case?

Because then the limit case as we approach 0fr is inconsistent. We had this same issue with Flexbox, and fixed it in the same way - animating the sole flexible flex item from "flex-grow: 1" to "flex-grow: 0" wasn't smooth and continuous - all non-zero flex-grow values make it fill the whole box, then it suddenly becomes non-flexible at zero. The special <1 behavior instead makes this work smoothly.

Similarly, animating a track from 1fr to 0fr, before the fix, would do the same thing as flexbox if it's the sole flexible track, and if there are multiple, it makes the "size of an fr" approach infinity - until it hit 0fr, at which point it's non-flexible and doesn't contribute at all. This fixes that: at <1fr, you just contribute your base size to the "size of an fr" calculation.

Your proposed fix avoids the "blow-up" problem, but doesn't fix the "single flexible track always claims the entire free space, until it hits 0" problem that Flexbox had until we fixed it.
> It doesn't - it just jumps straight into "finding the size of an fr",
> which has clamped <1 flex factors for a long time.
> This actually *harmonizes* the two paths.

I think you're wrong about that.  "finding the size of an fr" clamps the "flex factor sum", not the individual flex factors.
https://drafts.csswg.org/css-grid/#algo-find-fr-size

Example:

<style>
.grid {
  display: grid;
  grid: 0.1fr 0.4fr / 100px;
  height: 100px;
}
x:nth-child(1) { background: blue; }
x:nth-child(2) { background: grey; }
</style>
<div class="grid"><x></x><x></x></div>

Which results in the row sizes 10px 40px, thus maintaining the relative
proportions the author specified (FF and Chrome agrees on that).
I can't think of a case where a definite available space would result
in distorted proportions.

My gut feeling is that there must be a way to maintain the proportions
also in the indefinite case.  I'll think about this some more and get
back to you when I find it. ;-)

Comment 17 by r...@igalia.com, Oct 28 2016

Status: WontFix (was: Available)
I guess we can close this now that the issue has been clarified.

If the spec is changed again in the future we should probably report a different one.

Sign in to add a comment