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-sizing] clarification around Compressible Replaced Elements and min-content size #5665

Open
davidsgrogan opened this issue Oct 26, 2020 · 27 comments

Comments

@davidsgrogan
Copy link
Member

davidsgrogan commented Oct 26, 2020

5.2.2 Compressible Replaced Elements applies to 5.2 Intrinsic Contributions. Does it also apply to 5.1 Intrinsic Sizes?

The answer affects the size of <input> below because the flexbox will respect the min-content size of the <input> element when shrinking.

<div style="display: flex; width: 150px;">
  <div style="flex: 0 0 100px;"></div>
  <input style="width: 100%">
</div>

https://jsfiddle.net/pfv2x6sd/1/

Indications that 5.2.2 applies to 5.1:

  • The text of 5.2.2 explicitly says min-content size, which is clearly in the purview of 5.1: The following elements can have their min-content size compressed

Indications 5.2.2 does not apply to 5.1:

  • 5.2.2 is only in the 5.2 section, not an independent section like a hypothetical 5.3
  • 5.2.2 refers to 5.2.1 for clarification about when min-content sizes can be "compressed"

Chrome behaves as if 5.2.2 applies to 5.1. Firefox behaves as if it does not apply. (In an unlikely scenario, the implementors of the respective engines think the other engine is right.)

Can we get this part of the spec clarified so we know how to size the <input> element in the above example? One suggestion from the chrome bug is

  1. Replace "compressible" with a clearer term
  2. Make sure the two places match about whether this affects min-content size or contribution

The firefox bug is at https://bugzilla.mozilla.org/show_bug.cgi?id=1585485

/cc @dholbert @cbiesinger who I've been discussing this with

@fantasai fantasai added the css-sizing-3 Current Work label Oct 27, 2020
@fantasai
Copy link
Collaborator

fantasai commented Oct 27, 2020

In an unlikely scenario, the implementors of the respective engines think the other engine is right.

Heh. So, from the higher-level principles side of things:

  • This behavior was specced as a result of compat requirements. See [css-sizing] percentage [max-]width|height and intrinsic sizes #765 and CSSWG resolution.
  • The goal of a min-content size is to prevent overflow / collapsing to zero such that the element’s content is unviewable.
  • Specifying width: min-content should give the same results as a float or table sized into a zero-width container, as its intention was to give a syntax for representing the bottom edge of the CSS2 shrink-to-fit (fit-content) size range.
  • Arguably, the percentage is acting as a clamp on the actual min-content size the same way a length would, except because we don't know what the percentage resolves to yet we're treating it as zero(ish) during intrinsic contribution computations. So having the min-content size and min-content contribution diverge here isn't unreasonable, for the same reason they diverge with max-width: 100px.
  • The auto value of min-width is clamped by the width and max-width so that an explicit value by the author always overrides the implied minimum. See https://www.w3.org/TR/css-flexbox-1/#min-size-auto However min-width: min-content does not get clamped, and always takes priority the same way min-width: 100px does. We could apply that same logic here for percentages, we just currently don't (clamping is limited to "definite" percentages).

So overall my take is that compressible elements is about the min-content contribution only, not about the min-content size. We should fix the spec to be clear about that, and consistent with the CSSWG resolution.

We do also specify that a percentage size or max-size should have a similar clamping effect on the automatic minimum size used in Flexbox and Grid. But again, this doesn't affect the actual min-content size.

Anyway, that's my take on it. Interested to hear from @dholbert / @MatsPalmgren / @tabatkins also. :)

@fantasai
Copy link
Collaborator

Just noticed we actually do specify the effect on the automatic minimum size in css-sizing-3. But it's not reflected clearly in the Flexbox and Grid specs.

This rule also applies when calculating a content-based automatic minimum size or its corresponding size contribution, yielding a definite “specified size suggestion”.

fantasai added a commit that referenced this issue Oct 28, 2020
@aethanyc
Copy link

Does the edit in b05b8d7 mean that Firefox's current rendering on the reporter's testcase is desirable?

@davidsgrogan
Copy link
Member Author

Does the edit in b05b8d7 mean that Firefox's current rendering on the reporter's testcase is desirable?

That's my understanding.

@ZerdoX-x
Copy link

https://bugzilla.mozilla.org/show_bug.cgi?id=1641602
Run into this issue from here. This thing can't be obvious for me as for regular web developer

@davidsgrogan
Copy link
Member Author

Does the edit in b05b8d7 mean that Firefox's current rendering on the reporter's testcase is desirable?

That's my understanding.

Argh, now I'm not so sure based on the text in 5.2.1c that @fantasai pointed out above.

I think that spec text implies that <input> gets a specified size suggestion of 0, and hence a used min-width of 0.

That seems weird to me. 5.2.1 exists to deal with cyclic percentages but there is no cyclic percentage in this case.

@aethanyc
Copy link

5.2.1 exists to deal with cyclic percentages but there is no cyclic percentage in this case.

So 5.2.1c just doesn't apply to the example in the original comment? The flex container has a definite width:150px, so <input width="100%"> is not a cyclic percentage. It's definite and should resolve to a specified size suggestion of 150px by following just the Flexbox spec 4.5.

I agree that we should make 5.2.1c reflected clearly in the Flexbox and Grid specs. At least, I was not aware of 5.2.1c while implementing flexbox size suggestions.

@davidsgrogan
Copy link
Member Author

5.2.1 exists to deal with cyclic percentages but there is no cyclic percentage in this case.

So 5.2.1c just doesn't apply to the example in the original comment?

I'm not sure...

The flex container has a definite width:150px, so <input width="100%"> is not a cyclic percentage. It's definite and should resolve to a specified size suggestion of 150px by following just the Flexbox spec 4.5.

Yeah, I agree that's what I'd expect to happen. My confusion stems because I think the current spec in 5.2.1.c dictates that specified size suggestion is 0px in this case. @fantasai said that this compressible behavior was needed for compat, and it looks like it was originally for images in tables -- I doubt replaced elements as flexbox items had compat problems back in 2017. So, my point is, I'm not sure why the flex/grid automatic minimum size stuff was ever put in 5.2.1c. There probably was a good reason, I just don't know it and I'm confused by the results.

I agree that we should make 5.2.1c reflected clearly in the Flexbox and Grid specs. At least, I was not aware of 5.2.1c while implementing flexbox size suggestions.

Neither did I! I agree: if we do continue with the compressible behavior in automatic minimum size calculations, we should incorporate it into flex and grid specs somehow. Or a note in flex/grid pointing to here or something.

@dholbert
Copy link
Member

I think the current spec in 5.2.1.c dictates that specified size suggestion is 0px in this case. @fantasai said that this compressible behavior was needed for compat, and it looks like it was originally for images in tables -- I doubt replaced elements as flexbox items had compat problems back in 2017. So, my point is, I'm not sure why the flex/grid automatic minimum size stuff was ever put in 5.2.1c. There probably was a good reason, I just don't know it and I'm confused by the results.

It looks like that spec text was added to 5.2.1c in 534f792, with a bit of explanation in #2674 (comment)

In particular, the intent of the change was 'to make the cyclic percentage rules that zero out percentages apply to automatic minimum size calculations, making the “specified size suggestion” definite (and equal to zero, in the case of a pure percentage).'

It looks like this was briefly discussed in the subsequent CSSWG meeting, as quoted in the next comment there.

I think this all means that @davidsgrogan is correct that the specified size suggestion is indeed 0px in this case, which means Chrome's rendering of the original testcase is correct. It's true that there isn't any cyclic dependency, but that doesn't matter; the spec says we should apply the same rule, and we should resolve the percent to 0, for the purpose of computing the specified size suggestion.

@davidsgrogan
Copy link
Member Author

In particular, the intent of the change was 'to make the cyclic percentage rules that zero out percentages apply to automatic minimum size calculations

Agreed. And agreed that Chrome's current behavior obeys this change. But I haven't seen anything that explains the motivation behind the change. As in, why should this replaced-element exception

  • in the min-content contribution algorithm
  • for cyclic cases
  • that was required for compat

apply to

  • the automatic minimum size algorithm
  • for non-cyclic cases
  • when there is no compat requirement?

They seem like different situations 3x over. What's the benefit to web devs here?

@dholbert
Copy link
Member

dholbert commented Dec 9, 2020

As in, why should this replaced-element exception
[...]apply[...]
* when there is no compat requirement?

FWIW, I should push back on this part of the question: there is actually some amount of webcompat requirement for Chrome's behavior here. See the list of "see also" webcompat bug reports at https://bugzilla.mozilla.org/show_bug.cgi?id=1585485 -- there are 9 distinct reports there right now, which include quite high-profile sites like Yahoo, Bing, GitHub, NameCheap, and Heineken (the last of which is at least a big brand if not a big web presence).

That's part of why we're leaning towards switching Firefox to match Chrome's behavior (and the spec, to the extent that I understand it correctly). I suspect if instead Chrome switched to match Firefox, you'd end up needing to roll back the change due to webcompat fallout.

@davidsgrogan
Copy link
Member Author

Argh, good point, we might have missed our chance here.

@dholbert
Copy link
Member

dholbert commented Dec 9, 2020

Some of those bug reports are kind of old, so I just retested a few of them (Heineken, Yahoo, Bing) in Firefox with RDM mode (with Galaxy S9 as the choice of emulation target), and I confirmed the bugs are indeed still reproducible on those live sites. :(

It seems that this is a mobile web-design trick that's spread somehow (or been repeatedly rediscovered?) as a way of creating a textfield/button combo that fills the available width in a mobile viewport; the web developer just wraps the textfield & other content in a flex container and gives the textfield flex:1;width:100%, and they expect that to make the input compressible (and it does, in Chrome/WebKit, which means nearly-everywhere on mobile). Kind of like https://jsfiddle.net/dholbert/21jpntL9/

So, it does look like our hands are tied here.

@aethanyc
Copy link

I've fixed Firefox's Bug 1585485 to change Firefox's behavior toward Chrome and Safari to allow flex item listed in 5.2.2 being compressible when computing its specified size suggestion.

Note that Chrome and Safari doesn't comply to the spec in 5.2.1c example 4 with a main-size being calc(50% + 50px). Both implementations seem to have specified size suggestion of 0, but the spec requires the calc being resolved as 50px. WPT tests added in web-platform-tests/wpt#26956.

fantasai added a commit that referenced this issue Mar 5, 2021
… intrinsic size contribution and thus special rules in css-sizing-3 (like cyclic percentage resolution) apply. #5665"
@tabatkins
Copy link
Member

Reading over this again and revisiting our earlier thoughts on what we drafted, @fantasai and I believe the current spec text (allowing the input to shrink, due to Sizing-3 5.2.1.c) is indeed correct and intentional. The fact that compat apparently ties us to that interpretation is a nice bonus. ^_^

To hopefully make it a little clearer as @aethanyc requested in #5665 (comment) we've added notes to Flexbox and Grid pointing back to this section, so you don't have to remember that this bit changes how you interpret those Flexbox/Grid sections.

Let us know if there is anything else that needs to be addressed here?

@davidsgrogan
Copy link
Member Author

davidsgrogan commented Mar 5, 2021

The fact that compat apparently ties us to that interpretation is a nice bonus. ^_^

Compat does agree with that interpretation for input elements, but not images.

E.g. The pass condition in https://bugzilla.mozilla.org/attachment.cgi?id=9201228 (as reported in https://crbug.com/1174924) is consistent with the current spec but not with any engine until Firefox 86 (current stable release).

On top of the unknown compat story for images, I foresee this rule as causing confusion. In these two cases the only difference is the image having fixed width vs percent width that resolve to the same thing, which I suspect authors would think have identical layouts. But according to the spec and Firefox 86, these layouts are different.

https://jsfiddle.net/dgrogan/1a06xyjw/1/

<div style="display: flex; width: 100px;">
  <img src="//placekitten.com/100" style="width:100%;">
  <div style="width:100px"></div>
</div>
<div style="display: flex; width: 100px;">
  <img src="//placekitten.com/100" style="width:100px;">
  <div style="width:100px"></div>
</div>

This gets back to my questions in #5665 (comment), with respect to images, not form controls: Why are we copying table's weirdness in one situation to flexbox in a different situation? What's the benefit to web devs here?

@davidsgrogan
Copy link
Member Author

(sorry for the noise, I've edited the above comment and added AND deleted another comment, but I'm going to leave it now and want to hear y'all's feedback)

@fantasai
Copy link
Collaborator

fantasai commented Mar 5, 2021

A few stray thoughts (I haven't fully grokked this entire issue):

  • The automatic minimum size is similar to intrinsic contributions: in both cases the preferred and maximum size constraints are applied to them as limits. In the case of an intrinsic size contribution, because the box can't get larger than those sizes even if its contents do, and in the case of the automatic minimum size because one of our design goals is that the automatic minimum size doesn't impose a constraint stronger than either the preferred or maximum size. So the cyclic percentage width/max-width behavior should apply to both equally.

  • I think the testcase you gave should have both of those images resolve the same way because in both cases is the width is definite. If it's Web-compatible and reasonably implementable, that would be my preference. It's the cases where a percentage is being resolved against an indefinite size (i.e. is cyclic) that we need to treat it as a zero-based clamp on the intrinsic size contribution / automatic minimum size. This case isn't cyclic, so we should resolve the percentage and use the resulting length.

  • The example in 5.2.1(c) needs clarification. The whole section here is about the case of cyclic percentages, but the example doesn't provide any context so it's unclear whether the statement is true only in cyclic cases or always. I think we should fix that to be clear about its context.

@tabatkins
Copy link
Member

Okay, yes, I misunderstood my own spec text. ^_^

The note we added to Flexbox and Grid is correct - an auto min size is an intrinsic size, so the stuff in Sizing section 5.2.1 applies. But that only affects cyclic percentages, which don't appear in any of the testcases here, since in all these examples the container has a definite width and so the % is definite, not cyclic.

So I guess compat might force us to add an exception for non-button-like form elements (so they shrink all %s to 0 when calculating mins, not just cyclic ones), but normal replaced elements just follow the spec as written.

To be clear, your two examples should indeed render identically.

@aethanyc
Copy link

aethanyc commented Mar 10, 2021

Firefox's behavior was change in Bug 1585485 in the way that when computing flex item's specified size suggestion, the element listed in 5.2.2 is always resolve the percentage part of size to 0 whether the % is cyclic or not.

So I guess compat might force us to add an exception for non-button-like form elements (so they shrink all %s to 0 when calculating mins, not just cyclic ones), but normal replaced elements just follow the spec as written.

It would be great to see this exception documented for non-button-like form elements or other types of elements that might want this behavior, and apparently normal replaced elements like <img>, <video> doesn't want this. @davidsgrogan Do you know where is the code in blink that enforces this behavior? I hope it's easier to dig into to the code to come up with a list of elements than writing a test case to reverse engineering the behavior.

@fantasai
Copy link
Collaborator

OK, so let me see if I understand correctly. For Web-compat reasons we need:

  • The min-content contribution of replaced elements to calculate cyclic percentages only against zero. Non-cyclic percentage resolve against their containing block as normal.
  • The min-content contribution of semi-replaced elements (INPUT etc.) to calculate all percentages against zero, even if they could resolve against its definite containing block.

Is that 100% correct?

@davidsgrogan
Copy link
Member Author

@aethanyc I think this is a partial list and there's probably a few more that we may have to write tests for. Sorry for not responding sooner, I will prioritize digging in on monday. Lots of good progress on this thread :)

@tabatkins
Copy link
Member

Thanks @davidsgrogan! That list looks like it matches closely with our existing list, so that's nice. We'll hold final edits for you to finish digging in and making sure the list is correct.

@davidsgrogan
Copy link
Member Author

  • The min-content contribution of replaced elements to calculate cyclic percentages only against zero. Non-cyclic percentage resolve against their containing block as normal.

Yes.

  • The min-content contribution of semi-replaced elements (INPUT etc.) to calculate all percentages against zero, even if they could resolve against its definite containing block.

As observed via a flex item's specified size suggestion[1], Blink makes the min-content contribution of these semi-replaced elements 0 when they have a percent width, even if the percent width could resolve against its definite containing block.

However, the semi-replaced elements do not have a 0 min-content contribution when they have a percent max-width, even though 5.2.1c should apply to an element with % width or % max-width. Firefox 86 matched Blink wrt % max-width here, so we equally ignore that part of the spec 🤷

You asked about what we need for Web-compat reasons. I instead described what Blink does because I don't know that we need to stick with all of this behavior. Maybe some stuff could be changed with an acceptably small amount of compat pain? We don't have any data yet, but if we come up with some candidate changes, I can start gathering some data. But given @dholbert's research in #5665 (comment) about eliminating compressibility of flex items (which I'd hoped would be possible), I'd vote for just converging on Blink's behavior ☹️ . I don't think we should expand compressibility to include elements with a % max-width, because that would cause some elements on existing pages to get size 0 and completely disappear, which is a severe failure mode.

[1] How would you observe the min-content contribution of an element with percent width whose containing block has a definite width? I could only think of flex or grid's automatic minimum size so that's what I used. Is there another way?

As for IsContentMinimumInlineSizeZero, that function does hold the complete list of elements that Blink treats as compressible, at least in non-table contexts.

@fantasai
Copy link
Collaborator

fantasai commented Mar 25, 2021

However, the semi-replaced elements do not have a 0 min-content contribution when they have a percent max-width, even though 5.2.1c should apply to an element with % width or % max-width. Firefox 86 matched Blink wrt % max-width here, so we equally ignore that part of the spec ... I don't think we should expand compressibility to include elements with a % max-width, because that would cause some elements on existing pages to get size 0 and completely disappear, which is a severe failure mode.

Wrt difference in behavior for % width vs % max-width, see #765 particularly comment #765 (comment) ; we'd resolved to try to make replaced and semi-replaced elements behave the same (if possible), which is why the spec treats them the same even though implementations don't yet. [ Mozilla has a bug open at https://bugzilla.mozilla.org/show_bug.cgi?id=1388840 ; looks like @dbaron had patches but never submitted... ] If that's no longer considered likely to be Web-compatible, then we'll have to re-examine the issue and make replaced/semi-replaced elements inconsistent in the spec as well. :/

testcase with INPUT: http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=9112
testcase with IMG: http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=9113

Blink makes the min-content contribution of these semi-replaced elements 0 when they have a percent width, even if the percent width could resolve against its definite containing block.

Do you think we can change this so that semi-replaced elements resolve their percent widths against a definite containing block just like replaced elements do?

@davidsgrogan
Copy link
Member Author

Thanks for the summary of #765 WRT max-width. I get the intention now.

Blink makes the min-content contribution of these semi-replaced elements 0 when they have a percent width, even if the percent width could resolve against its definite containing block.

Do you think we can change this so that semi-replaced elements resolve their percent widths against a definite containing block just like replaced elements do?

I don't think so, based on what @dholbert summarized in #5665 (comment). Looks like this is a semi-common pattern that exists out in the wild today.

@dholbert
Copy link
Member

dholbert commented Aug 3, 2021

I'm confused about the definition of "cyclic percentage" here (and whether or not it's affected by the CB having a specified size). This is relevant because TYLin (@aethanyc ) has a proposed patch on https://bugzilla.mozilla.org/show_bug.cgi?id=1700474 to change Firefox's behavior to match Chrome, but I think it may represent a step away from spec-conformance.

My point of confusion is: does a percentage become non-cyclic just because the containing block has a definite size in the same axis? (for the purposes of css-sizing-3 section 5.2.1)

Tab and fantasai have comments here indicating "yes" (see below), but I'm not sure the spec's definition agrees (see further below).

Quoting Tab implying "yes":

[...] the stuff in Sizing section 5.2.1 applies. But that only affects cyclic percentages, which don't appear in any of the testcases here, since in all these examples the container has a definite width and so the % is definite, not cyclic.

Quoting fantasai implying "yes":

  • I think the testcase you gave should have both of those images resolve the same way because in both cases is the width is definite. If it's Web-compatible and reasonably implementable, that would be my preference. It's the cases where a percentage is being resolved against an indefinite size (i.e. is cyclic) that we need to treat it as a zero-based clamp on the intrinsic size contribution / automatic minimum size. This case isn't cyclic, so we should resolve the percentage and use the resulting length.

But my intuition (which leans "no") is that intrinsic sizes (e.g. an element's min-content contribution & min-content size) are not meant to be impacted by things external to the element like the definiteness/specifiedness of its containing block size. And I think the spec agrees with this intuition.

Quoting the spec implying "no" (css-sizing-3 5.2.1), emphasis added:

"a percentage value that resolves against a size in the same axis as the intrinsic size contribution (a cyclic percentage size)".
https://drafts.csswg.org/css-sizing/#cyclic-percentage-contribution

Note that there's no caveat for "but only if the containing block's size is indefinite" here, or anything like that...

So: I think that means the percent sizes in the testcases here are in fact "cyclic", right? (for the purposes of calculating the intrinsic size contribution & automatic minimum size, per 5.2.1)

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

No branches or pull requests

6 participants