1760658 - Height does not grow to fit content, on element with aspect-ratio and explicit width, when element has display:flex or grid

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Mike Hemberger]

Attachment: firefox-aspect-ratio-flex.jpeg

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:98.0) Gecko/20100101 Firefox/98.0

Steps to reproduce:

Create a flex container and set an aspect-ratio on the flex items. Add content inside these flex items such that it should force the box to expand. See it failing here -- https://codepen.io/JiveDig/pen/YzYwENd. Screenshot also attached below.

Actual results:

The content overflows the aspect-ratio container.

Expected results:

The container should no longer keep the aspect ration and expand.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::Layout' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=c5d6c005ea91d38fbb380729bba0948f1732ef12&tochange=ae98627c9afe86b9fbde889a3574ec1067f62e8d

2bf96c6628de086fd2e53acccb24a28251bbd925 Boris Chiou — Bug 1672078 - Enable aspect-ratio on nightly. r=emilio

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Here's a reduced testcase.

In Chrome, the black-bordered box is tall enough to contain the green box. In Firefox, it is not.

Comment 4

[Daniel Holbert [:dholbert]]

(Firefox has the same behavior if I use display:grid on the container instead of display:flex, too.)

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: testcase 2 (further-reduced a bit, now using a row-oriented flexbox)

The original testcase happened to use a column-oriented flexbox, but this happens if we use a row-oriented flexbox too.

Comment 6

[Daniel Holbert [:dholbert]]

Boris, I think you've got a bundle of aspect-ratio work that you're planning in the near future - maybe worth including this as part of that? (or maybe this is a dupe of a known bug?)

Comment 7

[Boris Chiou [:boris]]

Sure. May take a look at this in H1. :)

Comment 8

[Boris Chiou [:boris]]

This is related to https://drafts.csswg.org/css-sizing-4/#aspect-ratio-minimum, and we added this for block frame, but not for flex and grid container frame. My bad.

Comment 9

[Boris Chiou [:boris]]

(In reply to Boris Chiou [:boris] from comment #8)

This is related to https://drafts.csswg.org/css-sizing-4/#aspect-ratio-minimum, and we added this for block frame, but not for flex and grid container frame. My bad.

We should add a similar implementation like this: https://searchfox.org/mozilla-central/rev/0ffae75b690219858e5a45a39f8759a8aee7b9a2/layout/generic/nsBlockFrame.cpp#1930-1941.

Comment 10

[Boris Chiou [:boris]]

Attachment: Bug 1760658 - Part 1: Apply automatic content-based minimum on flex container.

Basides, factor out the utility function to AspectRatio so everyone can
use it.

Comment 11

[Boris Chiou [:boris]]

Attachment: Bug 1760658 - Part 2: Apply automatic content-based minimum on grid container.

Comment 12

[Boris Chiou [:boris]]

Attachment: Bug 1760658 - Part 3: Apply automatic content-based minimum on fieldset element.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/739f65a44331
Part 1: Apply automatic content-based minimum on flex container. r=emilio
https://hg.mozilla.org/integration/autoland/rev/cf1c104f05d7
Part 2: Apply automatic content-based minimum on grid container. r=emilio
https://hg.mozilla.org/integration/autoland/rev/59770e5a8ff2
Part 3: Apply automatic content-based minimum on fieldset element. r=emilio

Comment 15

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/33901 for changes under testing/web-platform/tests

Comment 16

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/739f65a44331
https://hg.mozilla.org/mozilla-central/rev/cf1c104f05d7
https://hg.mozilla.org/mozilla-central/rev/59770e5a8ff2

Comment 17

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 18

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

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 19

[Boris Chiou [:boris]]

I'm not planning to uplift these patch for now. So mark it wontfix