[css-grid] What if base sizes of both auto/min-content and max-content minimums are being handled?

[Loirooriol]

From https://drafts.csswg.org/css-grid/#algo-spanning-items,

For intrinsic minimums: First increase the base size of tracks with an intrinsic min track sizing function by distributing extra space as needed to accommodate these items’ minimum contributions.

But then https://drafts.csswg.org/css-grid/#extra-space says

If space remains after all tracks are frozen, unfreeze and continue to distribute space to the item-incurred increase of…

• when handling base sizes of tracks with min-content or auto minimums: [...]
• when handling base sizes of tracks with max-content minimums: [...]

What if the item spans a track with a min-content or auto minimum, and another with max-content, and the base sizes of both tracks are being simultaneously handled because both are intrinsic? To which tracks do you distribute space? The union of both cases? The intersection (all affected tracks if empty)? Something else?

[Loirooriol]

Firefox, Chromium and Edge are not interoperable: https://jsfiddle.net/mafd8s3o/

.grid {
  display: grid;
  grid-template-columns: 200px 300px;
  grid-template-rows: max-content fit-content(0);
}
.foo {
  background: yellow;
}
.bar {
  background: cyan;
}
.baz {
  grid-area: 1 / 2 / 3 / 3;
  background: magenta;
  height: 300px;
}

<div class="grid">
  <div class="foo">foo</div>
  <div class="bar">bar</div>
  <div class="baz">baz</div>
</div>

• Firefox distributes the 300px of .baz equally among both rows.
• Chromium distributes them only to the 2nd row because the 1st one doesn't have an auto or min-content min track sizing function.
• Edge distributes them only to the 1st row.

[MatsPalmgren]

AFAIK, Gecko has the correct layout for the given testcase. The reason is that "baz" has already grown both rows in the "For intrinsic minimums" step by its minimum contribution (300px, making each row 150px). So in the steps where the fit-content clamping occurs, "For max-content maximums" and "Distributing Extra Space Across Spanned Tracks", there is no remaining space to distribute (and fit-content only clamps the limit anyway, not the base, and the base trumps the limit).

FYI, I asked a similar question in #209. (what Tab refers to as 12.5.1 there is now 11.5 step 3.6 "For max-content maximums", and 12.5.2 step 5 and 6 are now 11.5.1 step 2 and 3.)

Chromium distributes them only to the 2nd row because the 1st one doesn't have an auto or min-content min track sizing function.

The first row has max-content min-sizing function, so the "For intrinsic minimums" step should be applied to both tracks.

As noted in #209, fit-content behaves as minmax('auto', 'max-content')) in the track sizing algorithm, with an extra clamp on the limit where it's explicitly pointed out in spec.

[Loirooriol]

@MatsPalmgren If I understand correctly, the difference is in the "For intrinsic minimums" step. At that point, the growth limit of both rows is something like 18px (the height of the content). When distributing the 300px, the base sizes have already reached the growth limits, so we need to "Distribute space beyond growth limits". The spec says,

• when handling base sizes of tracks with min-content or auto minimums: any affected track that happens to also have an intrinsic max track sizing function
• when handling base sizes of tracks with max-content minimums: any affected track that happens to also have a max-content max track sizing function

Here we are handling a track with an auto minimum (the 2nd row) and another one with a max-content minimum. Both conditions apply, so it's not clear among which tracks the 300px should be distributed.

In fact this is not completely related to fit-content, you can replace it with auto and you will get the same non-interoperability between Firefox and Chromium. And about fit-content it seems neither Firefox nor Chromium are obeying this:

For this purpose, fit-content() tracks are treated as max-content until they reach the limit specified as the fit-content() argument, after which they are treated as having a fixed sizing function of that argument.

[MatsPalmgren]

When distributing the 300px, the base sizes have already reached the growth limits

Hmm, yeah, that's a good point. I'll have to run this in a debugger later to see exactly what we're doing...

Here we are handling a track with an auto minimum (the 2nd row) and another one with a max-content minimum. Both conditions apply, so it's not clear among which tracks the 300px should be distributed.

"For intrinsic minimums" says "First increase the base size of tracks with an intrinsic min track sizing function". Since both rows have an intrinsic min sizing function, both are included in the set of tracks we should run "distributing extra space" over. The second bullet applies to row 1, and the first bullet applies to row 2 (since the "For intrinsic minimums" step handles base sizes). So it seems to me we should distribute to both rows in this section:

2.1. extra-space should be 300 - 18 - 18
2.2. "Distribute space to base sizes up to growth limits" should do nothing, since as you pointed out, base is equal to limit at this point
2.3. Then we're distributing space beyond growth limits. I don't think any limit applies here and thus not any clamp on the limit either. So this should set the item-incurred increase to half the extra-space for each track.
2.4. ... which then becomes the planned increase
3. ... which then updates the affected sizes (i.e. is added to the base sizes)

And about fit-content it seems neither Firefox nor Chromium are obeying this:

For this purpose, fit-content() tracks are treated as max-content until they reach the limit specified as the fit-content() argument, after which they are treated as having a fixed sizing function of that argument.

Yeah, I'm not sure what that paragraph is trying to say really. Maybe that any fit-content() track that reached its limit in step 2.2 should be excluded from 2.3? (In which case you'd get Chrome's layout I guess)

In fact this is not completely related to fit-content, you can replace it with auto and you will get the same non-interoperability between Firefox and Chromium.

For the auto case it seems clear to me that Gecko is correct. I'm less sure about the fit-content case now...

[Loirooriol]

The second bullet applies to row 1, and the first bullet applies to row 2 (since the "For intrinsic minimums" step handles base sizes). So it seems to me we should distribute to both rows

So I guess you are saying that when multiple bullets apply, we should consider the union? With the "if there are no such tracks, then all affected tracks" happening at the end after doing the union. Like

• when handling base sizes: any affected track that happens to have either

  • a max-content min and max track sizing functions, or
  • a min-content or auto min track sizing function, and an intrinsic max track sizing function

  If there are no such tracks, then all affected tracks.

• when handling growth limits: all affected tracks.

That would be clearer (and easy to implement in Chromium).

Yeah, I'm not sure what that paragraph is trying to say really. Maybe that any fit-content() track that reached its limit in step 2.2 should be excluded from 2.3? (In which case you'd get Chrome's layout I guess)

Yes, that's my understanding too. It would match Edge instead of Chrome. When distributing "For intrinsic minimums", Chromium only applies the first bullet, i.e. only tracks matching HasAutoOrMinContentMinTrackBreadthAndIntrinsicMaxTrackBreadth() are allowed to grow beyond growth limits. This is the case for fit-content(0) or auto but not for max-content.

[MatsPalmgren]

So I guess you are saying that when multiple bullets apply, we should consider the union? With the "if there are no such tracks, then all affected tracks" happening at the end after doing the union.

Yes, given that the affected tracks may contain tracks in both these categories, it doesn't make much sense to apply the "if there are no such tracks, then all affected tracks" to each bullet separately.

When distributing "For intrinsic minimums", Chromium only applies the first bullet, i.e. only tracks matching HasAutoOrMinContentMinTrackBreadthAndIntrinsicMaxTrackBreadth() are allowed to grow beyond growth limits. This is the case for fit-content(0) or auto but not for max-content.

That seems like a bug in Chrome to me. "For intrinsic minimums" should include max-content min-sizing functions too, and some of those might match the second bullet, e.g. minmax(max-content,max-content) should be allowed to grow beyond limits in the "For intrinsic minimums" step.

Regarding the meaning of the "For this purpose, fit-content() tracks..." paragraph, I think the intention is that fit-content() counts as max-content in the conditions in the bullets, then while distributing space to the selected tracks we apply the clamping - if we find that a fit-content() track has reached its limit, then we remove it from the selected tracks and continue with the rest. However, if the set becomes empty, then the "if there are no such tracks, then all affected tracks" rule kicks in and at that point we include all the affected tracks again (also those that didn't match the bullets originally, but this time around we distribute without clamping, since now fit-content(50px) behaves as 50px which can grow beyond limits!).

I think that's the intention of the spec as it's written and it makes sense to me.
For example, changing your example to grid-template-rows: max-content fit-content(50px), we should grow both rows equally in this step, but the second row will be clamped at 50px, then the first row will continue to grow and soak up the remaining space.

However, for grid-template-rows: fit-content(40px) fit-content(50px), the first grows to 40px and the second to 50px, but then, since the set of affected tracks is now empty, we start to distribute equally to both rows again, which results in the second row being 10px larger. (This makes sense to me since it behaves as grid-template-rows: minmax(auto,40px) minmax(auto,50px) but without the max-content max-sizing part. (In your example it would give the same result.)) In general, this has the effect that we'll prioritize the tracks that has an intrinsic max-sizing function (per the conditions in the bullets) but only up to their fit-content() limit. If all intrinsic max-sizing functions are fit-content(), then non-intrinsic max-sizing tracks also gets a chance after the fit-content() limits are reached. E.g. grid-template-rows: minmax(auto, 40px) fit-content(50px) starts to distribute beyond limits to both tracks after the second has reached 50px, but this only happens if there are no other (unlimited) intrinsic max-sizing tracks since they will always soak up all space.

[MatsPalmgren]

Fwiw, I did a quick hack to handle fit-content tracks in the "distribute beyond limits" phase as described above just to see what breaks... If you click on the orange jobs there then the "Reftest analyzer" tool (fifth icon from the left in the panel that pops up) will show you the before/after rendering.
The patch makes your testcase (and variations) work as I described above, and the few WPT failures that I looked at so far feels like reasonable changes to me. It'd be interesting to see of you get the same failures in Chromium. :-)

[fantasai]

OK, I reviewed the old version of the Grid sizing algorithm... afaict the two branches are correct, they're just poorly labelled. I've clarified the conditions for each one, I think this is now correct.

It doesn't handle fit-content() though, so I'll need to follow up on that point.

[tabatkins]

However, for grid-template-rows: fit-content(40px) fit-content(50px)...

@fantasai and I walked thru some examples including this specific one, and yes, we agree with Mats's interpretation in all aspects. In this case, the two rows would end up being 145px and 155px tall.

To be clear: when fit-content() tracks are involved, the set of tracks that are growing when you're distributing space past limits can change over time due to fit-content() tracks freezing, and then potentially causing a "grow all tracks" switchover due to all the fit-content() tracks freezing.

@Loirooriol Do you need anything clarified about the algorithm in that case? It read pretty clearly to us, even with fresh eyes that don't remember typing it out originally, but we're happy to make it clearer if necessary.

[Loirooriol]

Yes I think it's clear enough now