Open Bug 1700474 Opened 7 months ago Updated 3 months ago

Revisit the set of compressible elements when computing specified size suggestion

Categories

(Core :: Layout: Flexbox, defect)

defect

Tracking

()

ASSIGNED

People

(Reporter: TYLin, Assigned: TYLin)

References

Details

Attachments

(2 files)

The discussion in https://github.com/w3c/csswg-drafts/issues/5665 seems leaning toward that when computing specified size suggestion, we shouldn't resolve the percentage part in the main size of replaced elements (like image) against 0 if the percentage part is not cyclic. Blink doesn't do that.

At least from blink's IsContentMinimumInlineSizeZero() implementation, there's only some special case for the form control elements.

We'd probably want to add more wpt tests to ensure webcompat.

Severity: -- → S3

In Bug 1585485, if a replaced element or a form control elements listed
in the CSS Sizing 5.2.2 [1] has percentage in their preferred size, we
apply rule 5.2.2 to it to effectively compress its auto min size down to
zero in flexbox (via zeroing its specified size suggestion).

Bug 1585485 was fixed to match the Chromium's behavior specifically to
compress <input>. However, in reality Chromium only applies this rule to
form control elements, but not to replaced elements like <img>. So
again, we need to tweak our behavior.

I've verified that the added test passes on Chrome 92 on my Linux
machine via:

./mach wpt testing/web-platform/tests/css/css-flexbox/flex-item-compressible-003.html --product chrome

[1] https://drafts.csswg.org/css-sizing-3/#min-content-zero

Depends on D120518

Assignee: nobody → aethanyc
Status: NEW → ASSIGNED

We should check with the spec editors and Chrome developers to see if this difference (on Chrome's part) is intentional or not... Reviewing the css-sizing-3 spec sections that you mention in the patch (5.2.2 and 5.2.1), it feels like our behavior is correct (per spec) here.

Relatedly, I'm curious whether Chrome treats images and other non-form-control replaced elements with this special "compressible" handling in other intrinsic-sizing scenarios, and if they just left out this particular flexbox case. I don't want to end up in a situation where we've got different subsets of elements getting this treatment in different scenarios...

As dgrogan's raised the question about our behavior difference in flexbox between <img> and <input> in https://github.com/w3c/csswg-drafts/issues/5665#issuecomment-791673203, I doubt Blink would want to change the behavior to match the spec.

Also, quote from dgrogan's comment in https://github.com/w3c/csswg-drafts/issues/5665#issuecomment-800007759,

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.
[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?

So I think Blink has this special behavior observable only in flexbox, but not in other intrinsic-sizing scenarios. If a grid item has a percent width, it's resolved against the definite grid column's width, and its min auto width is fixed. I'm also curious whether it's possible to observe the special behavior in grid or in other scenario.

dgrogan, does my interpretation of Blink's intention look reasonable to you?

(also cc fantasai on this topic)

Flags: needinfo?(dgrogan)

I'll get back with a real response on friday.

(In reply to Ting-Yu Lin [:TYLin] (UTC-7) from comment #4)

As dgrogan's raised the question about our behavior difference in flexbox between <img> and <input> in https://github.com/w3c/csswg-drafts/issues/5665#issuecomment-791673203, I doubt Blink would want to change the behavior to match the spec.

I think this patch deals with the same issue as that github comment, which is: should the cats in https://jsfiddle.net/dgrogan/1a06xyjw/1/ be the same size? Blink says yes. Firefox says no. The spec says yes: there are no cyclic percentages present so compressibility doesn't apply.

If that's what your question is about then we don't currently plan to change our behavior.
But if I misunderstood the question or you're looking for more detail let me know with a concrete case that we can refer to?

Thanks for pushing on this compat issue; if it's so confusing to us it must be impenetrable to a web author.

Also, quote from dgrogan's comment in https://github.com/w3c/csswg-drafts/issues/5665#issuecomment-800007759,

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.
[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?

So I think Blink has this special behavior observable only in flexbox, but not in other intrinsic-sizing scenarios. If a grid item has a percent width, it's resolved against the definite grid column's width, and its min auto width is fixed. I'm also curious whether it's possible to observe the special behavior in grid or in other scenario.

dgrogan, does my interpretation of Blink's intention look reasonable to you?

(also cc fantasai on this topic)

Flags: needinfo?(dgrogan)

(In reply to dgrogan(chrome) from comment #6)

Thanks for pushing on this compat issue; if it's so confusing to us it must be impenetrable to a web author.

s/compat/interop/. Sorry. At least, Blink community calls differences between engines 'interop'. We use 'compat' for engine changes that break existing sites. So I guess whenever either engine changes to move toward 'interop', we risk a 'compat' issue.

I think this patch deals with the same issue as that github comment, which is: should the cats in https://jsfiddle.net/dgrogan/1a06xyjw/1/ be the same size? Blink says yes. Firefox says no. The spec says yes: there are no cyclic percentages present so compressibility doesn't apply.

Yes, in your cat test case, there's no cyclic percentages, and Firefox is the outlier that renders them differently. My patch in this bug can make the cats be the same size.

More background story: In bug 1585485, Firefox interpreted css sizing 5.2.1c This rule also applies when calculating a content-based automatic minimum size or its corresponding size contribution, yielding a definite “specified size suggestion” as: when computing flex's specified size suggestion, replaced elements as well as the compressible elements in 5.2.2 should be compressed regardless whether they have cyclic percentage or not. However, in reality, when resolving flex item's auto min size, blink and webkit don't compress the replaced elements like <img> unconditionally, but only those form controls elements like <input>. The difference has been observed in bug 1690635 and in https://github.com/whatwg/html/issues/6886#issuecomment-883658619

If that's what your question is about then we don't currently plan to change our behavior.
But if I misunderstood the question or you're looking for more detail let me know with a concrete case that we can refer to?

Thanks for letting us know. You might want to close https://crbug.com/1174924 to keep blink's current behavior.

I fail to construct any testcase, but as Daniel asked in comment 3, do you know whether the behavior that blink unconditionally treats some elements' min size as 0 via IsContentMinimumInlineSizeZero() can be observed in other intrinsic-sizing scenarios other than in the flexbox's auto min size resolution?

Flags: needinfo?(dgrogan)

I have also failed to construct a testcase. There may be a corner case somewhere but I haven't found it :)

Further, I think you've figured out that IsContentMinimumInlineSizeZero() adjusts min-content size, not min-content contribution, and I don't think that difference is observable.

I will close https://crbug.com/1174924.

Thanks again for pushing on the tangled mess that is compressibility, hopefully we get it figured out.

Flags: needinfo?(dgrogan)

Relatedly, I'm curious whether Chrome treats images and other non-form-control replaced elements with this special "compressible" handling in other intrinsic-sizing scenarios, and if they just left out this particular flexbox case. I don't want to end up in a situation where we've got different subsets of elements getting this treatment in different scenarios...

Daniel, given dgrogan's reply, do you feel it's OK to proceed with changing our behavior? The only way to guarantee that we have the same set of compressible elements as blink is through the WPT test in Part 2. I deduce the list from IsContentMinimumInlineSizeZero() and CSS Sizing 5.2.2.

Flags: needinfo?(dholbert)

I'm still confused/skeptical about correctness here.

David: thanks for weighing in here & helping us think through this. Regarding your cat example, I have a few counterexamples/counterpoints that I'd love your thoughts on. You said:

(In reply to dgrogan(chrome) from comment #6)

I think this patch deals with the same issue as that github comment, which is: should the cats in https://jsfiddle.net/dgrogan/1a06xyjw/1/ be the same size? Blink says yes. Firefox says no. The spec says yes: there are no cyclic percentages present so compressibility doesn't apply.

Counterpoint/counterexample #1: if I take your jsfiddle and replace the img elements with input elements, then Firefox doesn't change its behavior vs. your original testcase -- we still compress the one that has a percent -- but Chrome does change to agree with Firefox's rendering. So, I'm skeptical of your above-quoted interpretation of Chrome's behavior and the spec's intentions on compressibility. If there's a cyclic percentage in my version (as Firefox/Chrome seem to agree), then there is in yours as well, right?

Counterpoint/counterexample #2: if I take your jsfiddle and change the flex container to use width:min-content, then Chrome seems to lay out the first flex container in an interesting way -- it treats the img as compressible when computing the min-content contribution (which is why the flex container ends up being 100px wide -- that comes fully from the orange div's contribution, with 0 coming from the img); but Chrome does not treat the img as compressible when computing its automatic minimum size (which is why the img ends up filling the flex container and insisting on being 100px wide). CSS-sizing 5.2.1c says the browser should treat the element as compressible in both cases (which is how Firefox handles this example).

All of which is to say, I think I disagree with TYLin's first line of comment 8 (I think there is a cyclic percentage in dgrogan's cats testcase, just as browsers seem to agree that there is in my counterexample#1), and it feels like Firefox is internally-consistent & matching the spec here, and I don't fully understand what Chrome is doing or how it ties back to the spec.

Flags: needinfo?(dholbert) → needinfo?(dgrogan)

Alternate interpretation: maybe there's an argument to be made that my counterexample#1 doesn't have a cyclic percentage, since the containing block has a definite width. (I'm not sure I buy that, since we're still asking the flex item for its min-content contribution, which is a measurement that's supposed to be independent of the surroundings, and hence feels like it becomes cyclic if it tries to resolve against the content block size.) Under this alternate interpretation, https://github.com/w3c/csswg-drafts/issues/5665#issuecomment-797046688 would sort of explain my notes on Chrome's rendering of counterexample #1 .

Nonetheless: Example #4 in the css-sizing-3 spec (right after 5.2.1c) seems to implicitly refute that alternate interpretation. It immediately follows the discussion of cyclic percentages, and it says "For example, an <input> assigned width: calc(50% + 50px) has a min-content contribution of 50px". This implies that the spec understands the 50% to be cyclic in that scenario, regardless of the containing block's styling. (Chrome agrees that the contribution is 50px in this example but IIUC that's only because <input> is on Chrome's special IsContentMinimumInlineSizeZero() list, not because Chrome is following 5.2.1c and judging the percentage to be cyclic...)

(Also, this interpretation doesn't explain what Blink is doing on my counterexample#2... My best guess is that Blink is behaving like the percentage is no-longer cyclic when it gets to the point of resolving the automatic minimum size, since the containing-block has a resolved width at that point. But then, it seems like the 5.2.1c spec-text about automatic minimum sizes would simply never take effect in the inline axis, which feels odd and inconsistent, particularly if there's another list of semi-replaced elements that behave another way.)

(this is in response to comment 11, I hadn't read comment 12 or 13 when I wrote this)

Great counterexamples for getting to the bottom of this. It might finally be starting to crystallize in my mind. So, explanations:

For the original cats example https://jsfiddle.net/dgrogan/1a06xyjw/1/, I think the spec intends for the top case to have no cyclic percentages because of this github comment where I shared the same fiddle. fantasai and Tab both immediately said there is no cyclic percentage in the subsequent 3 comments. (I'm treating their opinion on this as equivalent to the spec's intention)

So in the rest of this comment I am assuming there are no cyclic percentages in the original cat example while trying to explain Blink's very confusing behavior in the counterexamples.

Counterexample 1 (top case in the fiddle)
There are no cyclic percentages in this fiddle, but a Blink bug affects the content size suggestion.
Blink bug 1: Blink is overaggressive when compressing. Blink makes "semi-replaced" (but NOT replaced) elements resolve % width against 0 for their min content even when there is no cyclic %. [code]

So in this example, the min-content of the input is 0px because it has width:100%. And content size suggestion = min-content = 0px (modulo any input-element specific floor). The input's specified size suggestion is 100px. So the input's automatic minimum size is min(0px, 100px) = 0px. The orange box also has auto min-width:0px so they end up 50px each.

I suspect that the spec's compressibility notion came about by trying to reverse engineer this Blink behavior I labeled 'bug 1'. Before flex arrived, I think the difference between Blink's behavior and the spec's definition of compressible was not observable. I think it would be ok to specify bug 1 if fixing it has compat problems.

Counterexample 2 (top case in the fiddle)
There is a cyclic percentage AND a different Blink bug affects content size suggestion.

In Blink, replaced (but NOT "semi-replaced") elements resolve % width and % max-width against 0 for their min content contribution, not for min content. I think that's per spec so far. Blink is overaggressive about this too: this happens always, even if there is no cyclic percentage, though I think this overaggressiveness is not relevant to this example. [code]

So in this case, the image's width:100% resolves to 0px for the image's min content contribution. That's why the image doesn't contribute to the min-content of the container and hence why the container has width 100px, where the 100px comes from the orange square.

Blink bug 2: Blink ignores the Note that comes right after https://drafts.csswg.org/css-flexbox/#content-size-suggestion that says the content size suggestion is a contribution. Blink just uses min content as the image's content size suggestion.

Because of bug 2, the width:100% on the image is ignored for the image's content size suggestion even though the spec requires the width:100% to cause the content size suggestion to be 0. and the image's content size suggestion ends up being the images' min content natural width of 100px. The specified size suggestion is also 100px after resolving width:100%, so the automatic minimum size is 100px. So because the container has width of 100px, its entire width gets allocated to the image, and the orange square gets width 0px.

Contrary to bug 1, I think Blink needs to fix bug 2 and the compat damage will be manageable. Fixing bug 2 means that the cat and orange square at the top of counterexample 2 will each be 50x50, which matches Firefox 90.

So, that's the analysis of Blink's <expletive>-ing logic for these cases.

From what I could tell, TYLin's patch under review changes behavior for cases that are not affected by either of these Blink bugs. TYLin, is that your interpretation too?

Again, your counterexamples are great. If you find more please post them.

Thank you for the detailed response! I need to read it more thoroughly; and I'm hoping to double-check with Tab and fantasai about whether in fact they think there are cyclic percentages in the cats example. (I pinged in #css, but it's Friday afternoon so my timing is bad for synchronous conversations, so I might try following up with a comment on the github issue.)

I'll just add: the reason I'm pushing back here is because this already feels like a special-casey area of behavior, and I'm reluctant to add changes to make us bug-compatible with Chrome (if we're not actively getting bug reports where content depends on Chrome's behavior), particularly if it's conceivable that the Chrome bugs can be fixed.

From what I could tell, TYLin's patch under review changes behavior for cases that are not affected by either of these Blink bugs. TYLin, is that your interpretation too?

FWIW, the attached patches make Firefox agree with Chrome on my counterexample-fiddles (as well as on your cats fiddle).

Re comment 14:

From what I could tell, TYLin's patch under review changes behavior for cases that are not affected by either of these Blink bugs. TYLin, is that your interpretation too?

Quote Daniel's comment 12:

Counterpoint/counterexample #2: if I take your jsfiddle and change the flex container to use width:min-content, then Chrome seems to lay out the first flex container in an interesting way -- it treats the img as compressible when computing the min-content contribution (which is why the flex container ends up being 100px wide -- that comes fully from the orange div's contribution, with 0 coming from the img); but Chrome does not treat the img as compressible when computing its automatic minimum size (which is why the img ends up filling the flex container and insisting on being 100px wide). CSS-sizing 5.2.1c says the browser should treat the element as compressible in both cases (which is how Firefox handles this example).

I didn't notice that my proposed patches can change Firefox's behavior to what Daniel described above for counterexample #2. This exactly replicates a blink's bug in Firefox (David recognizes it as "Blink bug 2" in comment 14), which is not my intention. So I'll hold off my patches.

SGTM. Hopefully the spec authors give more clarity in https://github.com/w3c/csswg-drafts/issues/5665.

Flags: needinfo?(dgrogan)
You need to log in before you can comment on or make changes to this bug.