New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-grid] Intrinsic size of grid containers #2303

Open
mrego opened this Issue Feb 12, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@mrego
Member

mrego commented Feb 12, 2018

The spec text is very clear about this and has been around for a long time:

The max-content size (min-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 (min-content constraint).

But it seems not all the browsers are doing it, or at least not all
are doing it properly.

Firefox is following the spec, and it runs the track sizing algorithm twice
(one under min-content constraint and another under max-content constraint)
to compute the min|max-content widths of the grid container.

However Blink and WebKit follow a different approach.
In this case they just run the track sizing algorithm once
with an available space of zero and use the sum of base sizes
as min-content size and the sum of the growth limits as max-content size.
The reason why these browsers do it that way is because
this was suggested back in 2013 by Julien Chaffraix in www-style
and never modified later.

Regarding Edge I don't know what's their implementation, but I'm finding
the same issues than in Blink and WebKit. So I guess Edge is either
not following the spec or not implementing it properly.

Let's go to the examples and why I'm bringing up this issue.

The key part of the track sizing algorithm that is affected by this
is when we resolve intrinsic track sizes
(for both non-spanning and spanning items we have a similar text):

  • For auto minimums:

    If the track has an auto min track sizing function and the grid container
    is being sized under a min/max-content constraint,
    set the track's base size to the maximum
    of its items' min/max-content contributions, respectively.

    Otherwise, set its base size to the maximum
    of its items' min-size contributions.

Imagine the following example:

<div style="width: 20px; border: thick dotted red;">
  <div style="display: inline-grid; border: solid thick;">
    <div style="min-width: 0px; background: lime;">Lorem ipsum.</div>
  </div>
</div>

The intrinsic sizes should be:

  • min-content size: The width of Lorem, let's say 50px.
  • max-content size: The width of Lorem ipsum., let's say 100px.

As the grid container is inline-grid, its sized as fit-content:
min(max-content size, max(min-content size, stretch-fit size)).
In this example, min(100px, max(50px, 20px)) = 50px.

Note: This also matches what happens in a similar case using inline-block
instead of inline-grid.

Output in Firefox vs other browsers of the previous example

In Blink and WebKit we always go through the Otherwise paragraph,
so the min-content size is 0px and the final width is 20px.
The same result happens on Edge, though I don't know how is that implemented.
In any case it seems quite clear that this is a bug in Blink, WebKit and Edge
and their behavior should be modified.

Note that to fix this, we'll need to follow a similar approach than Firefox,
which means that the track sizing algorithm will be run 3 times:

  • 2 times to calculate the min and max-content sizes
    (one under min-content constraint and another under max-content constraint).
  • And one extra time during layout considering no constraints.

This is not all, now let's go to a different example:

<div style="display: inline-grid; grid-template-columns: minmax(auto, 0px); border: solid thick;">
  <div style="background: lime;">Lorem ipsum.</div>
</div>

To calculate the min-content size, all the browsers seem to apply the clamping
described in Automatic Minimum Size of Grid Items section
of the spec. So the min-content size is 0px.
However to calculate the max-content size, Firefox uses
the max-content contribution as requested by the spec, that's 100px.
This makes that the intrinsic size of the grid container is 100px.
However during layout the track sizing algorithm is run again,
and as during layout it doesn't know if it's under a min-content
or max-content constraint, it goes through the Otherwise sentence
and apply the clamping, so the track has 0px width.
This causes a weird effect as the track is 0px
but the grid container is 100px.

Output in Firefox vs other browsers of the previous example

In Blink and WebKit the result is better, the grid container is 0px
as we always go through the Otherwise sentence and apply the clamping.
Edge has the same output too.

In a Firefox bug
@MatsPalmgren suggests that we apply the automatic minimum size clamping
when we are under a min-content constraint, max-content constraint
or no constraint.
That would make Firefox has the same output than the other browsers
in this last example.

It'd be nice to get the feedback from @atanassov about this issue,
as we don't know how the intrinsic size is being calculated in Edge.

Check the following codepen to see the examples live and play with them:
https://codepen.io/mrego/pen/xYdvbZ

@mrego mrego added the css-grid-1 label Feb 12, 2018

@fantasai

This comment has been minimized.

Contributor

fantasai commented Feb 14, 2018

The quoted spec text was added in this thread, here: https://lists.w3.org/Archives/Public/www-style/2016Jan/0163.html

The principle it's trying (and maybe failing?) to uphold is this one:

auto as a size generally passes down any min/max-content constraints, and passes up through it the min/max-content contribution. The min-content contribution of the grid item is still 50px, even if its min-width is zero.

The idea is that if all our sizes are auto and we degenerate down to one grid track/one item, a shirnkwrapped (e.g. floated or abspos'd) grid should behave similar to a shrinkwrapped block or flexbox.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 28, 2018

Bug 1427608 - [css-grid] Fix span=1 'auto' min-sizing for intrinsic s…
…izing. r=dholbert

When sizing the container under a min- or max-content constraint,
the item's min/max-content contribution needs to be clamped (when
Automatic Minimum Size / clamping applies) if its size is 'auto'.
That'll give the container the right intrinsic size. In Reflow,
we'll size the track initially to the clamped min-content
contribution again (in the Resolve Intrinsic Track Sizes step),
but since the container now has a definite size we'll grow
the track in the Maximize Tracks step up to its limit
(i.e. the clamp size).

For more details on the underlying issue, see:
w3c/csswg-drafts#2303
@MatsPalmgren

This comment has been minimized.

MatsPalmgren commented Mar 3, 2018

FYI, this is now fixed in the latest Firefox Nightly.
The algorithm we use in "For auto minimums" is:

if 'width' doesn't behave as auto {
  return the item's min-content contribution
}
if 'min-width' is 'auto' {
  if 'overflow' is 'visible' {
    if sizing under a max-content constraint {
      return the item's max-content contribution, clamped*
    }
    return the item's min-content contribution, clamped*
  }
  set the used value of 'min-width' to zero, fall-through
}
set the used value of 'width' to the used value of 'min-width'
return the item's margin-box size

clamped: means shrink the margin box size to fit a fixed track max-sizing function (if there is one), without making the content-box size negative (as usual per Grid §6.6)

With this definition of "behave as auto".

(ditto for 'min-height'/'height')

So, the trick is to always clamp the size in "For auto minimums" (when AMS and clamping applies). This gives the grid container the desired intrinsic size. Then, in layout (no sizing constraint), we'll return a clamped min-content contribution there, but since the grid container now has a definite size (the intrinsic size), we'll grow the track in the "Maximize Tracks" step up to the desired size.

As far as I can tell, this gives the same layout as in Chrome for all the tests I looked at.
I found only one difference, but I tend to think that's simply a somewhat unrelated bug in Chrome.

mykmelez pushed a commit to mozilla/gecko that referenced this issue Mar 7, 2018

Bug 1427608 - [css-grid] Fix span=1 'auto' min-sizing for intrinsic s…
…izing. r=dholbert

When sizing the container under a min- or max-content constraint,
the item's min/max-content contribution needs to be clamped (when
Automatic Minimum Size / clamping applies) if its size is 'auto'.
That'll give the container the right intrinsic size. In Reflow,
we'll size the track initially to the clamped min-content
contribution again (in the Resolve Intrinsic Track Sizes step),
but since the container now has a definite size we'll grow
the track in the Maximize Tracks step up to its limit
(i.e. the clamp size).

For more details on the underlying issue, see:
w3c/csswg-drafts#2303

fantasai added a commit that referenced this issue Nov 1, 2018

[css-grid-1] Clamp 'auto' contributions by a fixed max track size eve…
…n when sizing under a min-content/max-content constraint. Part I: non-spanning items. #2303
@fantasai

This comment has been minimized.

Contributor

fantasai commented Nov 1, 2018

OK, I've made the spec changes for non-spanning items. Haven't figured out the correct edits for spanning items yet.

@fantasai

This comment has been minimized.

Contributor

fantasai commented Nov 1, 2018

[ For the archives, here's a parallel thread between me and Mats from around the time this issue was filed. I think my goal here is to make sure I catch all of the relevant edits brought up in both discussions on this issue. ]

fantasai added a commit that referenced this issue Nov 1, 2018

[css-grid-1] Clamp 'auto' contributions by a fixed max track size eve…
…n when sizing under a min-content/max-content constraint. Part I: non-spanning items, revised. #2303
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment