1735589 - [css-flexbox] content size suggestion of image flex items

Status: RESOLVED FIXED

(Core :: Layout: Flexbox, defect)

Description

[Mozilla Apprentice]

A resolution was made for csswg-drafts/#6693.

[css-flexbox] content size suggestion of image flex items

• RESOLVED: no substantive change to spec, but clarify the text to avoid confusion

Discussion.

Comment 1

[Daniel Holbert [:dholbert]]

The essential behavior-change here is: we need to take the "pre-stretched" cross-size into consideration, when computing its automatic minimum size.

Testcase is https://jsfiddle.net/dgrogan/ug9rsf2a/ , and when this bug is fixed, that testcase should render with a 50px-tall box (instead of 100px which is how we size it now).

markup from that fiddle:

<div style="display: flex; flex-direction: column; width:100px; height: 0px;">
  <img src="https://placehold.jp/20/008000/008000/300x150.png" style="height: 100px;" />
</div>

Also, there's a reasonable chance that this will impact grid min-content-sizing behavior as well, since the automatic-minimum-sizing spec text is similar there (and stretching is possible there just as it is in flexbox).

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

https://github.com/web-platform-tests/wpt/pull/30900 updates the tests, and we import them via Bug 1733091.

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1735589 Part 1 - Respect flex item's aspect-ratio when measuring its main size in block-axis.

We've already set the pre-stretched cross-size via StyleSizeOverrides. All we
have to do is remove the aspect-ratio override to allow the pre-stretched
cross-size in the inline-axis transfer through aspect-ratio to the main-axis as
the block-size.

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1735589 Part 2 - Respect flex item's pre-stretched cross-size when computing its content size suggestion in inline-axis.

This allows the pre-stretched cross-size in the block-axis transfer through
aspect-ratio to main-axis as the inline-size.

Depends on D131260

Comment 6

[Daniel Holbert [:dholbert]]

As discussed in phabricator for part 2, I filed bug 1742042 preemptively, on what will effectively become a regression from this bug's patch-stack.

We might need either have a fix for that or have a plan for fixing it before we let this ship....

(It's a bit iffy spec-wise since the spec text on intrinsic sizing of flex containers is substantially different from what we [and Chrome I think] actually do under the hood. But we should still aim to preserve the invariant that content doesn't spill out of a container, if the container is sized to its contents....)

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

My original goal is to prevent webcompat issue preemptively because dgrogan mentioned in bug 1735891 comment 0 that blink gets bug reports because of changes likes this. However, I completely agree we'll need to fix bug 1742042 before landing my patches because we don't want a new behavior partially compatible with blink and our old behavior.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:TYLin, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 9

[Daniel Holbert [:dholbert]]

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #8)

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.

r+ patches are indeed present, but we don't want to land until we have a solution for bug 1742042, as noted in comment 7.

Comment 10

[dgrogan (chrome)]

This behavior change in chrome just hit stable and we got some bug reports, but not enough that would indicate a real compat issue, so we're planning on keeping the new behavior, and, of course, it would be great to have alignment across the engines here.

https://bugs.chromium.org/p/chromium/issues/detail?id=1287031&sort=-modified&colspec=ID%20Stars%20Pri%20Type%20Status%20Summary%20Modified%20Owner%20BlockedOn&x=mstone&y=owner&cells=tiles&q=component%3ABlink%3ELayout%3EFlexbox&can=2

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 5 See Also bugs.
:TYLin, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 12

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

S3 is OK. We are waiting for the outcome of a spec discussion. See bug 1785914 comment 4.

Comment 13

[Frank Griffith Jr]

Ting-Yu: Is this bug still open with the patches accepted, or are additional changes needed?

Comment 14

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

This is still open. Comment 7 & comment 9 are still valid, and we'll need to fix bug 1742042 before landing the patches here.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db620eb72d71
Part 1 - Respect flex item's aspect-ratio when measuring its main size in block-axis. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/d100672dcf40
Part 2 - Respect flex item's pre-stretched cross-size when computing its content size suggestion in inline-axis. r=dholbert

Comment 16

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/db620eb72d71
https://hg.mozilla.org/mozilla-central/rev/d100672dcf40

Comment 19

[Richard Marti (:Paenglab)]

Attachment: image.png

With this patch the dropmarker in Settings/Search is bigger than normal and forces an ellipsis on the text.