Skip to content
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-flexbox] Change content-size suggestion to min-intrinsic instead of min-content? #6794

Open
davidsgrogan opened this issue Nov 4, 2021 · 65 comments

Comments

@davidsgrogan
Copy link
Member

davidsgrogan commented Nov 4, 2021

https://drafts.csswg.org/css-flexbox/#min-size-auto

This issue stems from an interop problem discussed in web-platform-tests/wpt@21a7c47 .

In the example below, the spec dictates a final width of the flex item of 50px. But when the same element is a block box it has width 100px. Should they be different?

<div style="display: flex;">
  <!-- Chrome and spec say final width is 50px. Firefox says 100px. -->
  <div style="background: blue; height: 100px; aspect-ratio: 1/2; ">
    <div style="width: 100px;"></div>
  </div>
</div>

<!-- Everyone agrees final width is 100px -->
<div style="background: blue; height: 100px; aspect-ratio: 1/2; ">
  <div style="width: 100px;"></div>
</div>

https://jsfiddle.net/dgrogan/2734sdfr/

Of course a flex item can have a different final size than an otherwise-identical block box because the whole point of a flex container is that it flexes the items. But this item is not flexed, and it's still a different size. That seems weird.

In more detail, the difference in this case is a result of aspect-ratio's automatic minimum size being more aggressive than flexbox's. Giving flex a weaker set of constraints makes sense, because we want to give flexbox the power to flex its items. But it is puzzlement-inducing that putting a no-op flex container around an element changes the element's size.

Possible solution if we want the boxes to be the same width whether they are block boxes or unflexed flex items (i.e. Firefox's behavior today):

  1. change content-size suggestion to be min-intrinsic[1] instead of min-content (@bfgeek previously floated this idea In [css-sizing-4] Should aspect-ratio affect the intrinsic size? #5032 (comment))
  2. Then we'd also have to change specified size suggestion to only account for computed main size properties, not preferred sizes that come from the aspect-ratio. As is, specified size suggestion is 50px, so even we did (1) and only (1), the final width of the item would still be 50px.

Or, do whatever Firefox is doing, which is probably not those.

But, if giving the blue box different sizes in the two layout modes doesn't strike anyone else as weird, then maybe we just close this issue and discard the idea of making content-size suggestion = min-intrinsic. Even if we do that, we still need to determine which of Firefox and Chrome behavior is correct.

[1] or whatever the name is, vis-a-vis #5305

davidsgrogan referenced this issue in web-platform-tests/wpt Nov 4, 2021
… element.

If the flex item is a non-replaced element and its min-width/min-height
is 'auto', the spec has changed so that it has no transferred size
suggestion now. https://drafts.csswg.org/css-flexbox-1/#min-size-auto

This patch also updates WPT tests to fix
#27878

Differential Revision: https://phabricator.services.mozilla.com/D112830

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1703304
gecko-commit: fb14e6d29a44a79efc296cc727f92aa58ea4dacc
gecko-reviewers: dholbert
@bfgeek bfgeek added the css-flexbox-1 Current Work label Nov 4, 2021
@davidsgrogan
Copy link
Member Author

Looks like Gecko does something like

content-size suggestion = max(min-content, min-intrinsic)

or maybe Gecko floors the entire automatic minimum size?

min-size:auto = max(min-size:auto, min-intrinsic)

Seems reasonable and Blink can try to match if that's the behavior we end up adopting.

@dholbert @aethanyc , can you report what logic Gecko follows such that the flex item in https://jsfiddle.net/dgrogan/2734sdfr/ has automatic minimum width of 100px?

@aethanyc
Copy link

For the flex item in https://jsfiddle.net/dgrogan/2734sdfr/, Firefox only has content size suggestion, which follows the same logic described in https://drafts.csswg.org/css-sizing-4/#aspect-ratio-minimum, i.e.

max(blue background div's height 100px * (1/2), blue background's child width 100px) = 100px

Firefox currently only applies specified size suggestion only if there's a main size property. However, the current spec implies that the specified size suggestion is the item’s preferred main size, which can be computed via aspect-ratio + definite cross-size. @dholbert pointed to me that the definition was changed after the edit in 0c1a35d, and this seems changing the behavior.

@davidsgrogan
Copy link
Member Author

davidsgrogan commented Nov 11, 2021

Oh, y'all apply aspect-ratio's automatic minimum size on top of flex's. Maybe Blink should match.

Blink matches Gecko about when to apply specified side suggestion.

@davidsgrogan
Copy link
Member Author

https://drafts.csswg.org/css-flexbox-1/#min-size-auto gives one definition of automatic minimum sizing. https://drafts.csswg.org/css-sizing-4/#aspect-ratio-minimum gives another. They don't always agree.

@tabatkins @fantasai , what do we do in the case of a flex item with an aspect ratio? Apply both minimums?

Chrome only applies flexbox's. I think Firefox applies both.

@davidsgrogan
Copy link
Member Author

The underlying behavior difference may explain why firefox and chrome have different renderings for the example below.

<div style="display: flex; flex-direction: column; height: 0px;">
  <div style="aspect-ratio: 1/1; width: 100px; border: 5px solid orange;">
    <div style="height: 200%; background: blue;"></div>
  </div>
</div>

Chrome 99 canary gives 0 height to both orange item and blue child.
Firefox 95a1 gives 200px height to orange item and 400px height to blue child.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-flexbox] Change content-size suggestion to min-intrinsic instead of min-content?.

The full IRC log of that discussion <emeyer> Topic: [css-flexbox] Change content-size suggestion to min-intrinsic instead of min-content?
<emeyer> github: https://github.com//issues/6794
<emeyer> fantasai: I think I need to go back to read the flexbox specs from the very beginning to figure out what I think should happen here.
<emeyer> …I think we have to decide what behavior we want here and how to achieve that.
<emeyer> …I’m not sure I’m clear on any of that.
<emeyer> TabAtkins: Same here. This is important and VERY difficult to reason about correctly, and we need more time.
<emeyer> dgrogan: Base issue is that aspect-ratio define sizing one way and flex defines it another
<emeyer> …and they conflict when you aspect-ratio a flex item.
<emeyer> TabAtkins: That’s what I understand. I don’t know how to resolve that yet.
<emeyer> dgrogan: Before we start talking about changing the spec, should I change Chrome to match Firefox where they apply both minimums?
<emeyer> TabAtkins: Wouldn’t that answer the question?
<emeyer> …Because if you change to match Firefox, then the spec will just match whetever you do.
<emeyer> dgrogan: Okay, let’s pinch this off now.
<emeyer> astearns: Will remove Agenda+ and will put it back once fantasai and tabatkins have time.
<emeyer> iank_: Can we do this quickly? It’s a major problem.
<emeyer> TabAtkins: That’s the goal.
<emeyer> astearns: Let’s wait on this for now.

@astearns astearns removed the Agenda+ label Jan 26, 2022
@fantasai
Copy link
Collaborator

@tabatkins and I think some of the confusion here comes from missing a small nuance in this paragraph:

The content-based minimum size of a flex item is the smaller of its specified size suggestion and its content size suggestion if its specified size suggestion exists; otherwise, the smaller of its transferred size suggestion and its content size suggestion if the element is replaced and its transferred size suggestion exists; otherwise its content size suggestion.

The flex item in question does not have a specified size suggestion, and is not replaced, and therefore it falls through to the final clause and uses its content size suggestion which is its min-content size in the main axis (100px). Therefore Firefox's behavior here is correct.

@aethanyc mentioned

However, the current spec implies that the specified size suggestion is the item’s preferred main size, which can be computed via aspect-ratio + definite cross-size. @dholbert pointed to me that the definition was changed after the edit in 0c1a35d, and this seems changing the behavior.

0c1a35d was definitely not meant to change behavior, but I can see how switching the emphasis from the “computed value” would change how automatic sizes are interpreted. We need to clarify that we're only considering non-automatic definite sizes:

diff --git a/css-flexbox-1/Overview.bs b/css-flexbox-1/Overview.bs
index 2c3418beb..358740409 100644
--- a/css-flexbox-1/Overview.bs
+++ b/css-flexbox-1/Overview.bs
@@ -1001,7 +1001,7 @@ Grid doesn't have similarly meaningful shrinkability, so it doesn't need to care
 	<dl>
 		<dt><dfn>specified size suggestion</dfn>
 		<dd>
-			If the item’s [=preferred size|preferred=] [=main size=] is <a>definite</a>,
+			If the item’s [=preferred size|preferred=] [=main size=] is <a>definite</a> and not [=automatic size|automatic=],
 			then the <a>specified size suggestion</a> is that size.
 			It is otherwise undefined.

Committed in ca4dc9e .

@tabatkins
Copy link
Member

tabatkins commented Jan 31, 2022

In #6794 (comment),
@dgrogan brought up the following example:

<div style="display: flex; flex-direction: column; height: 0px;">
  <div style="aspect-ratio: 1/1; width: 100px; border: 5px solid orange;">
    <div style="height: 200%; background: blue;"></div>
  </div>
</div>

Chrome's behavior here is correct. The min-content contribution of the blue box is zero due to the cyclic percentage size rules in https://www.w3.org/TR/css-sizing-3/#cyclic-percentage-contribution. So the flex base size is 100px (from the transferred width), and the automatic minimum size is zero, resulting in a 100px hypothetical main size.

Then, since the flex container is 0px tall and the item is flexible, it shrinks down to its minimum size (0px). The blue box's percentage then resolves against that zero during layout, so the blue box is also zero.

Firefox renders this with a 200px tall orange box, and a 400px tall blue box, which means in their implementation the percentage is looping and self-applying, and just happens to be cut off on two applications. This is because they are not applying the cyclic percentage rules
while calculating the orange box's automatic minimum height: they are allowing the blue box's percentage
to resolve against the orange box's 100px automatic height, resulting in the orange box getting an automatic minimum height of 200px. The orange box then can't shrink below that, so during layout the blue box resolves its percentage against the 200px height, giving it a final height of 400px.

@fantasai
Copy link
Collaborator

Propose to close as editorial / invalid. Please let us know if we need to consider anything else!

@davidsgrogan
Copy link
Member Author

davidsgrogan commented Jan 31, 2022

Could you explicitly say which engine, if any, renders the original case correctly?

<div style="display: flex;">
  <!-- Chrome says final width is 50px. Firefox says 100px. -->
  <div style="background: blue; height: 100px; aspect-ratio: 1/2; ">
    <div style="width: 100px;"></div>
  </div>
</div>

@fantasai
Copy link
Collaborator

fantasai commented Jan 31, 2022

@davidsgrogan

The flex item in question does not have a specified size suggestion, and is not replaced, and therefore it falls through to the final clause and uses its content size suggestion which is its min-content size in the main axis (100px). Therefore Firefox's behavior here is correct.

#6794 (comment)

Unless there's something we missed?

@dholbert
Copy link
Member

dholbert commented Feb 2, 2022

@tabatkins , one thing I'm confused about in your analysis -- you say:

So the flex base size is 100px (from the transferred width), and the automatic minimum size is zero

Why do you say the automatic minimum size is zero -- shouldn't it actually be 100px?

The flex item here should use the content-size suggestion i.e. its min-content height, per https://drafts.csswg.org/css-flexbox-1/#min-size-auto (since it doesn't have a specified size suggestion and is not replaced).

And I think its min-content height is 100px, not 0. At least, this simplified example (with just a single div that has explicit height:min-content) suggests that it is:

<!DOCTYPE html>
<div style="aspect-ratio: 1/1;
            width: 100px; height: min-content;
            background: blue; border: 5px solid orange">
</div>

Browsers agree that this^ div is 100px tall, which I think (?) means its height:min-content is resolving to 100px and hence that's its min-content height? So I'd think that that's what the content size suggestion should be, when resolving its automatic minimum size in Tab's analysis above.

@dholbert
Copy link
Member

dholbert commented Feb 2, 2022

For completeness, I'll point out that browsers also agree that the div in the other example here (the row-oriented flex container that @davidsgrogan asked about) does indeed have a 100px min-content width which (as fantasai noted) agrees with Firefox's behavior in that case. Here's a simplified example with width:min-content to demonstrate that the min-content width is indeed 100px there:

<div style="aspect-ratio: 1/2;
            height: 100px; width: min-content;
            background: blue; border: 5px solid orange">
  <div style="width: 100px;"></div>
</div>

In this^ case, the min-content width does actually come from its content, i.e. its 100px-wide child; but if you edit the example to remove that child, then browsers also agree that the div ends up being 50px wide there instead (from the aspect-ratio and cross-axis size).

So, this seems to be further confirmation that the min-content size is indeed established by the aspect-ratio and the opposite-axis property (though it might be bigger the element contains content that is even larger).

@bfgeek
Copy link

bfgeek commented Feb 2, 2022

@dholbert - That example is using the min-width: auto to size up to 100px, its not strictly that "min-content" is 100px in this case. E.g.

<div style="aspect-ratio: 1/2;
            height: 100px; width: min-content; min-width: 0;
            background: blue; border: 5px solid orange">
  <div style="width: 100px;"></div>
</div>

In the above case all browsers agree that its 50px.
Here min-content is 50px, but with min-width: auto, we apply a minimum of the intrinsic min-size of 100px.

@dholbert
Copy link
Member

dholbert commented Feb 2, 2022

@bfgeek aha, thank you for clarifying that. I had forgotten that aspect-ratio had its own special min-width:auto behavior even with no flex/grid involvement.

OK - with that clarification, I think @tabatkins' and @fantasai's notes above make sense to me. EDIT: still confused, see my next comment.

@bfgeek
Copy link

bfgeek commented Feb 2, 2022

@dholbert - Given that clarification I think there is still a terminology issue here. E.g.

<div style="display: flex;">
  <!-- Chrome says final width is 50px. Firefox says 100px. -->
  <div style="background: blue; height: 100px; aspect-ratio: 1/2; ">
    <div style="width: 100px;"></div>
  </div>
</div>

The flex item in question does not have a specified size suggestion, and is not replaced, and therefore it falls through to the final clause and uses its content size suggestion which is its min-content size in the main axis (100px). Therefore Firefox's behavior here is correct.

In the given example (afaikt) the min-content size (given that it has an aspect-ratio) is actually 50px not 100px.

One thing that might mitigate the confusion here is to change the definition of min-content such that when we compute it we look to see if min-width: auto is present.

Ian

@dholbert
Copy link
Member

dholbert commented Feb 2, 2022

[retracting my "things make sense" comment]

So per @bfgeek 's example and comments, it sounds like the min-content size of an element with aspect-ratio and an opposite-axis size is defined by the aspect-ratio and the specified-size in the opposite axis (which corrects my misunderstanding in #6794 (comment) ).

So, my confusion from #6794 (comment) remains -- I'm not sure I agree with @tabatkins that the automatic minimum height of the flex item would be zero in Tab's analysis in #6794 (comment) . I would think it should be the min-content size, which (per bfgeek's notes about how width:min-content resolves) would be determined by the aspect-ratio and the opposite-axis size.

@tabatkins
Copy link
Member

tabatkins commented Feb 3, 2022

Why do you say the automatic minimum size is zero -- shouldn't it actually be 100px?

Mea culpa, you're right. When writing that down, I tripped over the fact that (a) the blue child resolves to 0 height while calculating the automatic minimum size, and (b) the specified size suggestion shouldn't take aspect-ratio into account (we had just dropped a minor edit into there to make sure that automatic sizes couldn't contribute even if they're definite), and so forgot to think about aspect-ratio when talking about the content size suggestion, which should take that into account. You're correct that the automatic minimum size in this case is 100px.

So neither Chrome nor Firefox is quite right here. Instead, the orange item starts at 100px flex basis, isn't allowed to shrink below 100px, and the blue child then resolves its % against that, resulting in the blue box being 200px tall.

@davidsgrogan
Copy link
Member Author

davidsgrogan commented Feb 3, 2022

@davidsgrogan

The flex item in question does not have a specified size suggestion, and is not replaced, and therefore it falls through to the final clause and uses its content size suggestion which is its min-content size in the main axis (100px). Therefore Firefox's behavior here is correct.

#6794 (comment)

Unless there's something we missed?

Sorry, I totally missed that paragraph.

But the content size suggestion is not 100px, it's 50px (as mentioned in #6794 (comment)).

My point from #6794 (comment) still stands. Slightly reworded:

https://drafts.csswg.org/css-flexbox-1/#min-size-auto gives a definition of automatic minimum sizing for flex items.
https://drafts.csswg.org/css-sizing-4/#aspect-ratio-minimum gives a definition of automatic minimum sizing for aspect-ratio elements.
As seen in this case, those definitions don't always agree.

So what do we do in the case of a flex item with an aspect ratio? Which minimum do we apply?

These definitions need to be modified so that they always agree (which I think is what Ian proposes in the last sentence of #6794 (comment)) OR if they don't agree, we need explicit instruction of which to honor (which is possibly both).

@tabatkins
Copy link
Member

@bfgeek

In the given example (afaikt) the min-content size (given that it has an aspect-ratio) is actually 50px not 100px.

Where are you getting that from? Are you assuming this is from the aspect-ratio affecting the specified size suggestion? We made a small tweak to prevent that, specifying it only looks at non-automatic definite sizes, specifically because it was creating an unintentional conflict with the aspect-ratio auto minimum size. (And was pre-empting the other size suggestions which were intended to handle aspect ratios.) (And it was just wrong anyway, as it would made made width: auto provide a specified size suggestion even without aspect ratio, which would have been all kinds of wrong.)

@tabatkins
Copy link
Member

OR if they don't agree, we need explicit instruction of which to honor (which is possibly both).

Yeah, we should make it clear what to do when they disagree. I suspect we want to honor the larger of the two (aka both minimums are active).

@bfgeek
Copy link

bfgeek commented Feb 3, 2022

Where are you getting that from? Are you assuming this is from the aspect-ratio affecting the specified size suggestion?

I'm describing how engines derive the min-content size of a non-replaced element with an aspect-ratio - (which feeds into the content suggestion for this specific example). E.g. for this case this rectangle has a min-content size of 50px :

<div style="aspect-ratio: 1/2;
            height: 100px; width: min-content; min-width: 0;
            background: blue; border: 5px solid orange">
  <div style="width: 100px;"></div>
</div>

The min-content size itself doesn't apply any automatic minimum sizing. (We don't read min-width when computing the min-content size). One option here is that it can! This would solve the issue.

Given that above is 50px we feed this into the content size suggestion which gives Chrome's output for the flexbox example being talked about.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Change content-size suggestion to min-intrinsic instead of min-content?, and agreed to the following:

  • RESOLVED: use the transferred size suggestion in preference to the content size suggestion, where that exists, for replaced elements
The full IRC log of that discussion <heycam> Topic: Change content-size suggestion to min-intrinsic instead of min-content?
<heycam> github: https://github.com//issues/6794
<heycam> dgrogan: this is about how the automatic minimum sizing for flex items works with items that have an aspect-ratio
<heycam> ... non-replaced items with an aspect-ratio
<heycam> ... Elika summarized it nicely
<heycam> ... ultimately it's a conflict between when an aspect-ratio item, would overflow the container, do we want to destroy the aspect-ratio and let flex's shrinking properties override it
<heycam> ... or preserve the aspect-ratio, with potentially some overflow
<dgrogan> https://jsfiddle.net/dgrogan/zcL7qs2v/5
<heycam> dgrogan: I would argue that we want to preserve the aspect-ratio, in part because authors seem to want their aspect-ratios preserved
<heycam> ... I just looked on Stack Overflow, I looked at the first 10-15 aspect ratio questions, mostly they're about flex destroying their aspect-ratio
<heycam> ... if they want it destroyed because they don't want overflow, they can specify min-width:0
<heycam> ... this issue is only about what we want in the default case
<Rossen_> ack fantasai
<heycam> fantasai: the initial value of min-width is auto, to prevent overflow when the author wasn't expecting the screen to be as narrow as it is
<heycam> ... the case here is the flex item is shrinkable, and is in a container that's smaller than its aspect-ratio derived size, but larger than its min-content width
<heycam> ... do we want it to shrink right down to the min-intrinsic size?
<heycam> ... setting min-width to 0 is more extreme, it shrinks to the point where the item no longer overlfows the container, but the item's contents overflows the item
<heycam> ... it prevents one kind of overflow but causes another
<heycam> ... if authors don't want a box to shrink below its specified size, whether it's specified through width or aspect-ratio, they shouldn't be having the box be a shrinkable flex item
<heycam> ... the purpose of min-width is a safety mechanism. preferred size is expressed through the width property
<heycam> dgrogan: I don't follow the argument about min-width no being the way to set a preferred size
<heycam> fantasai: if your preferred size is something with an aspect-ratio, and you set it width width/height, then min-width is a stopgap to prevent bad cases of overflow
<heycam> ... the aspect-ratio breaking, it might not look the way you wanted, but we're trying to optimize for the case being usable
<heycam> ... not to make your design look perfect, which is what width expresses
<heycam> dgrogan: I see what you're saying. I can take the other way symmetrically, specifying the preferred main size through the aspect-ratio and the height
<heycam> ... when we let the author set an aspect-ratio and a cross size, we're kind of promising that we'll respect that, they went out of their way to set that
<heycam> fantasai: same when the author says width:100px, but we break that when the item is shrinkable. since you've asked it to be shrinkable
<heycam> ... don't think aspect-ratio is less of a constraint than width
<heycam> dgrogan: authors seem to really hate when their aspect-ratios are broken
<heycam> ... I don't think it's obvious that aspect-ratio is the same strength as width
<heycam> ... it's not clear they're setting something as flimsy as a width, when they set aspect-ratio
<heycam> IanK: I think more opinions might be good from the author side of things
<heycam> dholbert: I'd question analagous situations. are there other cases where you set an aspect-ratio, and you stretch in both driections, like a grid cell?
<heycam> fantasai: when you set width/height on an element we ignore aspect-ratio completely
<heycam> IanK: the trick with flexbox is that it has a strong explicit stretch in the cross axis, by default
<miriam> q+
<Rossen_> ack miriam
<heycam> miriam: I can see use cases where I want it to shrink, and others where not
<heycam> ... how easy is it for the author to specify that?
<heycam> ... wondering if adding flex:0 is clear
<heycam> fantasai: that would cause it not to shrink. initial value allows shrinking but not growing
<heycam> miriam: I'm wondering if we were going with the other default, what would be the counter author solution to say, allow shrinking?
<heycam> IanK: flex:0 will still allow shrinking
<heycam> dholbert: flex:none is what you want
<heycam> fantasai: they're the same
<heycam> IanK: no
<heycam> fantasai: initial value is "1 0"
<heycam> dgrogan: I think we all agree on flex:none
<heycam> ... if we go with the opposite default, you'd have to go out of your way to say min-width:0
<heycam> dholbert: min-width:0 would cause more overflow than the opposite
<TabAtkins> Right, `flex: 0` is equivalent to `flex: 0 1 0px`
<heycam> ... it doesn't precisely undo it. that would allow things to compress to 0.
<heycam> dholbert: min-content or min-instrinsic
<heycam> IanK: min-content would do the right thing
<heycam> fantasai: it's not quite exactly that
<heycam> ... min-content will win
<heycam> ... auto minimum size, because it's trying to be a non-intrusive safety mechanism, if you have some explicit size on it, it'll take the minimum of that and the min-content size
<heycam> ... so you can't get the auto behavior by setting min-content
<heycam> miriam: I don't have an immediate sense of which one I want more often
<heycam> ... flex:none seems the simpler fix though
<heycam> fantasai: it also resolves to 0 if you're a scroll container
<heycam> ... there's a lot of magic in min-width:auto, to make it weaker than other author expressed constraints
<heycam> ... it's always trying to find what's the smallest size to prevent overflow and also not interfere with other constraints
<heycam> dgrogan: glad you brought up min-width:auto magic
<heycam> ... one proposal in the bug, from Ian in March, is redefining min-content for non-replaced aspect-ratio items to incorporate the auto min width from the aspect-ratio in css-sizing-4
<heycam> ... if we did that, we'd end up with the asepct-ratio preserving behavior in this case
<heycam> ... but it would do so in a way that would 1. a lot of magic would go away, min-width:auto becomse simpler to spec, and 2. it becomes easier to implement, and the auto min width for aspect-ratio items totally ignoring flexboxes also gets easier to implement
<heycam> ... in the 2 years this has been specced, it's been really hard to implement
<heycam> ... Ian has a big test case that shows some of this
<heycam> ... I think based on my knowledge of Blink/WebKit/Gecko, if we did Ian's proposal, implementation becomes simpler, and we'd be way more likely to get interop
<heycam> ... if we stuck with the magic in auto min-width, we're not going to get interop on a lot of this for a long time
<heycam> ... if ever
<heycam> ... we've all had a really hard time implementing this, and got lots of bug reports
<heycam> Rossen: becoming a bit more pragmatic, rather than chasing spec purity
<heycam> ... can we do that moving forward? if we can, do we have enough buy in from implementors?
<heycam> dholbert: what's Ian's proposal?
<heycam> Ian: effectively, instead of having a separate min intrinsic and min content size, you incorporate the min-width:auto behavior in min-content
<heycam> ... currently, we're the only implementation that explicitly created a separate min intrinsic size
<heycam> ... and effectively, that would disappear, and when you're calculating min-content for the box, you also apply the min-width:auto logic on top of the aspec-ratio logic
<heycam> fantasai: not following. we explicitly wanted to say min-width:auto would break aspect-ratio
<heycam> ... having min-height break but not min-width ...
<heycam> IanK: min-width:auto would still encompass this, it just wouldn't shrink
<heycam> dgrogan: I think Safari does this today
<heycam> Ian: Safari's min-content implementation basically does this already
<heycam> ... in some of the complicated test cases, Safari kind of does what I'm suggesting
<heycam> dgrogan: but we all have many bugs in this area
<heycam> Ian: engines also struggle with width:100% and width stretch being different
<heycam> Rossen: what's the proposal here?
<fantasai> https://jsfiddle.net/dgrogan/zcL7qs2v/5
<fantasai> https://github.com//issues/6794#issuecomment-1218597495
<heycam> fantasai: we walked through a bunch of cases ^
<heycam> Ian: I also wanted to note, we can't change replaced elements. they have to stay at min-content
<fantasai> https://github.com//issues/6794#issuecomment-1220038900
<heycam> ... there's too much of the web that relies on them preserving aspect-ratio
<fantasai> latest proposal from me and TabAtkins ^
<heycam> ... we accidentally broke this recently and got bug reports quickly
<heycam> ... can't shrink replaced elements below their min-content size
<heycam> dgrogan: I think your point Ian is that if we can't change replaced elements, they currently do what non-replaced elements currently do
<heycam> Ian: replaced elements will always pass through their aspect-ratio, and we can't change that
<heycam> dgrogan: so, incorporate the current min-width:auto magic into the min-content algorithm itself
<heycam> ... the min-width for these aspect-ratio items would be largest of their min intrinsic width, or their aspect-ratio derived width
<heycam> fantasai: Tab and I don't really like the idea of incorporating min-width:auto into min-content definition
<heycam> ... because it really should be the other way around
<heycam> ... if you set width:min-content, you're proposing that the resolution of that min-content value to a concrete value, looks at the min-width value, and if it is auto, incorporates some extra magic
<heycam> ... if it's not auto, does not have magic
<heycam> ... but if you set min-width:min-content, it cannot depend on the min-width property itself
<heycam> ... so the min-content keyword means two different things, depending on where you set it
<heycam> ... we think this should be more consistent, and independent of what sizing property you set it on
<heycam> ... whether it's on width or min-width, its used value is the same
<heycam> Ian: we already look at the width properties in the min-content algorithm. there's already a bunch of compressibility stuff, special table stuff
<heycam> ... this is fine from an engineering point of view
<heycam> dholbert: I'm a bit confused about that stating of the propsoal
<heycam> ... it sounds like we're talking about what should happen when you have an element with an asepct-ratio that's forced to shrink
<heycam> ... sounds like that's talking about what the magic is behind min-width:auto. not about how it gets incorporated
<heycam> fantasai: the compressibility stuff affects the min-content contribution, which is not hte same as the min-content size
<heycam> dgrogan: I think Ian's saying it's fine from an implementation PoV
<heycam> dgrogan: I think of this as how the min-content:auto magic is incorporated
<heycam> dholbert: min-width:auto resolves to come length
<heycam> s/min-content/min-width/
<heycam> ... sounds like the proposal is to resolve it to a different length
<heycam> ... that takes into account the aspect-ratio more eagerly
<heycam> dgrogan: I'm not talking about the min-content alg more eagerly taking it into account
<heycam> ... right now, it ignores min intrinsic size and only looks at aspect-ratio
<heycam> ... so if anything this proposal is to do it less eagerly
<heycam> Ian: we also look at asepct-ratio for the min intrinsci size, because we transfer through the max height and min height
<heycam> ... and that affects the min intrinsic size
<heycam> ... min-intrisc and min-content are almost always the same
<heycam> Rossen: still not seeing something we can resolve on
<heycam> ... sentiment here is clear, but I still don't see what to resolve to move forward, that satisifies implementability and the developer point
<heycam> ... Elika's pointing out another part of the issue, that articulates what we should and can do for replaced elements, and for non-replaced elements the explanation here still doesn't give enough specifiy that suggests a change
<fantasai> https://github.com//issues/6794#issuecomment-1220038900
<heycam> fantasai: we are suggesting several specific changes
<heycam> fantasai: Tab and I are saying it shouldn't be considering aspect-ratio
<heycam> ... when we originally wrote the text for min-width:auto, there wasn't aspect-ratio on anything excpet replaced elements
<heycam> ... in that context, it doesn't make sense to incorporate aspect-ratio
<heycam> ... so we really meant min-intrinsic size despite not having that term yet
<Rossen_> a/developer point/developer pain/
<heycam> ... [the other two points are there in the link above]
<heycam> fantasai: in flex/grid when you have a transferred size, it's different in grid the transferred size takes precedence
<heycam> ... but in flex it takes the min of the two
<heycam> ... it seems that would probably not be a good idea for compat, given where implementations are right now
<heycam> Ian: that's something we can resolve on. for replaced elements, for compat, we need to keep it as min-content
<heycam> fantasai: I'd phrase that as "use the transferred size suggestion in preference to the content size suggestion, where that exists, for a replaced element
<heycam> Ian: for us they're basically equivalent
<heycam> ... when do they differ?
<heycam> fantasai: because the content size suggestions do not take aspect-ratio into account
<heycam> ... if it did, then there wouldn't have been a transferred size suggestion in the first place
<heycam> Ian: this is flex box's content size suggestion, not min-content size we use elsewhere
<heycam> RESOLVED: use the transferred size suggestion in preference to the content size suggestion, where that exists, for replaced elements
<heycam> fantasai: there was a comment in the thread about how transferring the max size from the opposite axis didn't make much sense
<heycam> dgrogan: in the content size suggestion?
<heycam> fantasai: for non-replaced
<heycam> ... right now we transfer a max size constraint from the opposite axis to clamp the min in this axis
<heycam> ... that probably doesn't make sense
<heycam> dgrogan: we still have a fundamental disagreement about the main point
<heycam> ... I just want to fall back, all 3 engines tried and we're in the place where we don't have much interop
<heycam> ... the suggestion of incorporating hte auto min width into the min-content calculation, it's not great, but it's practical, pretty sure all engines can do it easily, and we'd get interop way easier
<heycam> chrishtr: you also said impl difficulty and web compat prevented you from implementing the altnerative?
<heycam> Rossen: no that's for replaced elements
<heycam> dgrogan: it's related
<heycam> ... we're talking about changing the rules. and one of them would've changed replaced, but we can't do that
<heycam> chrishtr: so we special case replaced elements, for compat
<heycam> dholbert: to be fair, non-replaced elements do have a meaningful smaller min width
<heycam> Ian: I'd still like to hear from more web devs about what they expect of the default
<heycam> ... and how they'd work around that
<heycam> ... heard from Miriam
<heycam> dgrogan: it's not just implementation. if we resolve on Ian's March proposal, it becomes easier to implement, and we think it doesn't break user expectations
<heycam> ... Adam Argyle weighed in on that
<heycam> miriam: I have use cases that I'd want to go either way, so more important to me is that I have the tools to get non-defautl behavior when I need it
<heycam> ... flex:none, to say "preserve my aspect-ratio" worked for me
<heycam> ... if there's a similar swtich from the other side, then it'd work for me too
<heycam> fantasai: but there isn't one from the other side
<heycam> ... flex:none is the thing to use to prevent it from going below its minimum size
<heycam> chrishtr: do we need something for the other case?
<heycam> fantasai: that's what min-width:auto is for
<heycam> chrishtr: is there some other way to get this use case that Miriam says is legit?
<heycam> dgrogan: I don't really understand why some fixe value of min-width wouldn't work well
<heycam> ... even if it doesn't cover all cases, I think it'd be close enough
<heycam> ... don't understand why min-width is not enough
<heycam> miriam: it might be for my cases, would have to look more
<heycam> fantasai: would point out that it's a strong constraint
<heycam> ... if you put min-width:min-content that's winning over all other size constraints
<heycam> ... min-width:auto is a weak constraint
<heycam> ... everything else will win over it
<heycam> ... so if you want to set some kind of minimum, you want it to be a weaker one than other constraints? like max-width? you can't do that with min-content, because that will win over everything else
<heycam> dgrogan: you can do min-width:10px
<heycam> fantasai: is an arbitrary length value what you want?
<heycam> ... or something based on the content?
<heycam> dgrogan: I don't know where we go from here
<heycam> miriam: would also say that the fact it's not flexing to grow ... would it maintain the aspect-ratio?
<heycam> ... I'd expected it to do the same thing growing as shrinking
<heycam> ... if flex-grow:1, it will trash the aspect-ratio
<heycam> s/...if flex/Ian: if flex/
<heycam> miriam: I'd expect them to work the same way
<heycam> Ian: the problem we get into is that flex-shrink being 1 everywhere is ...
<heycam> fantasai: being the default is what's throwing people for a loop
<heycam> ... if they also set width:100px on it and notice that shrinks but doesnt grow, that's consistent
<heycam> ... whatever width you have preferred, it will shrink but not grow, by default, in flex
<heycam> Rossen: still not hearing a concrete path forward here
<heycam> ... I think this would benefit from some whiteboarding

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 27, 2022
New expectations incorporate an item's min-intrinsic size as part of
flex's automatic minimum sizing.

The new expectations are arguably what the spec currently calls for AND
what the spec authors originally intended AND are consistent with both
relevant proposals in w3c/csswg-drafts#6794

Also discussed at
web-platform-tests/interop#139 (comment)

Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908
aarongable pushed a commit to chromium/chromium that referenced this issue Sep 27, 2022
New expectations incorporate an item's min-intrinsic size as part of
flex's automatic minimum sizing.

The new expectations are arguably what the spec currently calls for AND
what the spec authors originally intended AND are consistent with both
relevant proposals in w3c/csswg-drafts#6794

Also discussed at
web-platform-tests/interop#139 (comment)

Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1052123}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 28, 2022
New expectations incorporate an item's min-intrinsic size as part of
flex's automatic minimum sizing.

The new expectations are arguably what the spec currently calls for AND
what the spec authors originally intended AND are consistent with both
relevant proposals in w3c/csswg-drafts#6794

Also discussed at
web-platform-tests/interop#139 (comment)

Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1052123}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 28, 2022
New expectations incorporate an item's min-intrinsic size as part of
flex's automatic minimum sizing.

The new expectations are arguably what the spec currently calls for AND
what the spec authors originally intended AND are consistent with both
relevant proposals in w3c/csswg-drafts#6794

Also discussed at
web-platform-tests/interop#139 (comment)

Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1052123}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Oct 13, 2022
…pect-ratio tests, a=testonly

Automatic update from web-platform-tests
[css-flex] Change expectations of two aspect-ratio tests

New expectations incorporate an item's min-intrinsic size as part of
flex's automatic minimum sizing.

The new expectations are arguably what the spec currently calls for AND
what the spec authors originally intended AND are consistent with both
relevant proposals in w3c/csswg-drafts#6794

Also discussed at
web-platform-tests/interop#139 (comment)

Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1052123}

--

wpt-commits: 8160da618bad50684232b7865e956827efe18072
wpt-pr: 36113
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
New expectations incorporate an item's min-intrinsic size as part of
flex's automatic minimum sizing.

The new expectations are arguably what the spec currently calls for AND
what the spec authors originally intended AND are consistent with both
relevant proposals in w3c/csswg-drafts#6794

Also discussed at
web-platform-tests/interop#139 (comment)

Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1052123}
NOKEYCHECK=True
GitOrigin-RevId: cc4f488e5e9e8a496e5135e6d6fcea62eac77088
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Oct 19, 2022
…pect-ratio tests, a=testonly

Automatic update from web-platform-tests
[css-flex] Change expectations of two aspect-ratio tests

New expectations incorporate an item's min-intrinsic size as part of
flex's automatic minimum sizing.

The new expectations are arguably what the spec currently calls for AND
what the spec authors originally intended AND are consistent with both
relevant proposals in w3c/csswg-drafts#6794

Also discussed at
web-platform-tests/interop#139 (comment)

Change-Id: I3c6ecdf66fbb4ee745c88b6b2a4dbecfb9913908
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3924134
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1052123}

--

wpt-commits: 8160da618bad50684232b7865e956827efe18072
wpt-pr: 36113
webkit-early-warning-system pushed a commit to sammygill/WebKit that referenced this issue Oct 22, 2022
…lex items.

https://bugs.webkit.org/show_bug.cgi?id=246755
rdar://101346126

Reviewed by Rob Buis.

There has been a lot of conversation on computing the min-content size
for flex items when computing the content size suggestion. In
particular, there were potential issues when computing the min-content
size for non-replaced flex items that were given an aspect-ratio
property. This issue was brought up in the following CSSWG
conversation: w3c/csswg-drafts#6794

When the conversation was initially created, browsers all had different
behaviors. A patch was added to make WebKit behavior similar to the
behavior of Blink: WebKit#3025
The patch added behavior to take the aspect ratio into consideration
when computing these sizes.

This behavior ended up being incorrect by the time consensus was
reached. The final decision seems to be that we should be computing
the min-intrinsic size, which does not take into consideration the
aspect-ratio and is just based off of the content. The idea of the
min-intrinsic size was introduced here: w3c/csswg-drafts#5305
Since our initial behavior was close to the behavior that was eventually
agreed upon, this patch ends up being mostly a revert. It is not
completely a revert, however, since there are still some pieces left in
from the initial patch.

If the item is a replaced element, we will start take into consideration
the aspect-ratio and compute the size using
computeMainSizeFromAspectRatioUsing. If the item is a non-replaced
element, instead we will compute the size using
computeMainAxisExtentForChild. This will compute the min-intrinsic
size by calling either child.computeLogicalWidthInFragmentUsing or
child.computeContentLogicalHeight.

Spec reference: https://drafts.csswg.org/css-flexbox-1/#min-size-auto
Follow up discussion: web-platform-tests/interop#139

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-002.html:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/aspect-ratio/flex-aspect-ratio-004.html:
Newest version of the tests

* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::computeFlexItemMinMaxSizes):

Canonical link: https://commits.webkit.org/255858@main
@bfgeek
Copy link

bfgeek commented Apr 5, 2023

@mirisuzanne Were you able to compile any thoughts here?

@mirisuzanne
Copy link
Contributor

I'm not sure I have anything new to provide here. I'm not a big fan of the default flex values, but that ship has long sailed. I'm also not fully-versed in all the intricacies and magics involved with the different sizing algorithms discussed - which makes it hard to comment on the difference between min-width of auto vs min-content. By default, I would reach for the latter - but I do not fully understand the implications of that, and would want someone to walk me through those examples more explicitly.

At the higher level of my author expectations:

  • I do think it's fair to say that authors prioritize aspect-ratios in many/most situations where they are set. When you want an aspect ratio, there's usually a reason for it, and things often start to break when those aspect-ratios are discarded.
  • Flexible items with aspect ratios always have problems at their extremes, and almost always need guard-rails defined – either with min/max-widths, or queries to change the layout more dramatically. There's no one-size-fits-all answer.
  • In general, I would expect flex items that shrink or grow to handle their aspect-ratios in the same way.

When using aspect-ratio with flexbox, I would almost never set an explicit size on the cross-axis. The whole point would be maintaining a ratio as the item flexes. The only reason I can imagine ending up in this situation is if I'm using flexbox to lay out non-flexible items. Either way, I would want to clarify:

  • If I want the items to flex, there shouldn't be an explicit height-and-ratio, I should choose one or the other. In that case, I probably also need guard-rails at the extremes.
  • If I don't want items to flex, the height makes some sense - but then I should turn off flex-shrink.

Of course we need a default here, but I don't see a 'right' one. Both defaults look broken, because this is not a clear use-case - and I would want to provide a fix either way. Like I said in the call: what's important to me is that I have good options for clarifying my intent in either direction once this breaks.

It would be a lot easier to comment here if any of the examples were based in actual use-cases.

@bfgeek
Copy link

bfgeek commented Mar 20, 2024

As a data-point, we a starting to receive some bug reports about out behaviour (specifically for this case: https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=12476 [1] )

With people expecting WebKit's behaviour.

[1]

<!DOCTYPE html>
Both these should render the same
<div style="width: 100px; height: 100px; display: inline-flex; flex-direction: column">
	<div style="aspect-ratio: 1; background-color: red;"></div>
	<div style="min-height: 100px; background-color: blue;"></div>
</div>
<div style="width: 100px; height: 100px; display: inline-flex; flex-direction: column">
	<div style="width: 100%; aspect-ratio: 1; background-color: red;"></div>
	<div style="min-height: 100px; background-color: blue;"></div>
</div>

(in the row direction https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=12477 which only Blink is consistent).

@bfgeek
Copy link

bfgeek commented May 17, 2024

As another data-point, we've received another web developer bug report about our current (min-intrinsic) behaviour being perceived as incorrect while the min-content behaviour perceived as correct. It's more or less this case:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=12739

Agenda+ to move to the min-content behaviour.

@bfgeek bfgeek added the Agenda+ label May 17, 2024
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-flexbox] Change content-size suggestion to min-intrinsic instead of min-content?, and agreed to the following:

  • RESOLVED: change minimum size of flex items to be the larger of min-intrinsic and transferred size
The full IRC log of that discussion <emilio> iank_: this is re auto min size on flex items and aspect-ratio
<emilio> ... blink is more or less consistently dropping the aspect-ratio
<emilio> ... other browsers are sometimes dropping it
<emilio> ... blink has been on the receiving end of a bunch of web dev bug reports
<emilio> ... this situation isn't good for web devs
<emilio> ... haven't seem bug reports for the other engines
<emilio> ... as such I think we would do a change to respect aspect-ratio
<emilio> ... we can add a bunch of tentative tests about this behavior
<emilio> ... proposal would be max(aspect-ratio, min-intrinsic size)
<TabAtkins> I'm happy to take the change, I'll just have to spend some time digesting exactly what it is. But if we're getting compat issues and webdevs are preferring one behavior, let's do it.
<dholbert> s/aspect-ratio/transferred-size/
<dholbert> (in the min() expression)
<dholbert> (er max() expression)
<emilio> fantasai: I'd like to
<fantasai> s/I'd like to/I'm the same as Tab here/
<emilio> ... TabAtkins and I would like to spend some time on this issue, but if there are compat issues...
<emilio> iank_: Yeah I'm going to try changing blink and add tests
<emilio> fantasai: I think I understand the proposal now
<emilio> ... I think it makes sense?
<chrishtr> +1 to accepting the change
<emilio> astearns: should we resolve on accepting this change tentatively?
<emilio> q+
<astearns> ack emilio
<emilio> iank_: what's going on is web devs really hate it when the aspect-ratio gets dropped
<emilio> q+
<emilio> iank_: I think proposed resolution would be to change the AMS of flex items to be max(min-intrinsic-size, transferred-size via aspect-ratio)
<astearns> ack emilio
<fantasai> fantasai: ok, got it
<fantasai> emilio: Do we understand what Gecko and WebKit are doing?
<fantasai> iank_: I understand, it's complicated
<fantasai> iank_: WebKit and Gecko are inconsistent between row/column axes, and aren't consistent in when transferred size is applied.
<emilio> they are inconsistent between column / row and how the transferred size is getting applied
<fantasai> iank_: everyone is super inconsistent
<fantasai> emilio: if you could write it up, I'd be curious
<fantasai> iank_: my intention was to write out a suite of testcases for this behavior
<fantasai> iank_: that should show what's happening
<fantasai> iank_: issue is that Web devs are running into behavior where WEbkit/Gecko are consistent in honoring aspect ratio and Blink isn't
<fantasai> emilio: so your proposal would be breaking changes for all three engines
<fantasai> emilio: right?
<fantasai> iank_: yes, because we're all inconsistent. And inconsistent across axes. Basically everyone is bad.
<fantasai> iank_: we're probalby the most consistent, but not what web devs expect
<fantasai> iank_: so that's where we are
<fantasai> emilio: ok, this sounds good. We'll look at tests. On paper sounds reasonable.
<fantasai> iank_: given lack of bug reports on other engines, not worried
<fantasai> astearns: Proposed resolution to change minimum size of flex to be the larger of min-intrinsic and transferred size
<emilio> PROPOSED: change the AMS of flex items to be max(min-intrinsic-size, transferred-size via aspect-ratio)
<emilio> RESOLVED: change minimum size of flex items to be the larger of min-intrinsic and transferred size

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests