1596255 - Flex containers improperly shrink native-themed select elements below content size

Status: ASSIGNED

(Core :: Layout: Flexbox, defect, P3)

Description

[Emily McDonough [:alaskanemily]]

This can be seen by comparing a select element inside a flex container with height:0 and a select element with width:100% inside a regular div of height:0. The difference is visible on OS X, where the select elements are very slightly different. Dumping the frames shows that they have very slightly different heights.

Comment 1

[Daniel Holbert [:dholbert]]

Note: testing/web-platform/tests/css/css-flexbox/select-element-zero-height-001.html (soon to be added in bug 1591925) is expected to fail due to this bug.

This bug is about min-height:auto resolution on select elements in flex containers. We should be resolving it as the "content size suggestion" as described at https://drafts.csswg.org/css-flexbox/#min-size-auto . This is the min-content size, which in the block axis is the same as the max-content size per https://drafts.csswg.org/css-sizing/#min-content-block-size , and is roughly the element's preferred height.
https://drafts.csswg.org/css-sizing/#max-content-block-size

Right now, I think we instead treat the select element as if it had a min-height of its "os-theme-imposed minimum height", which is a useful clamp for us to apply, but may be too low of a clamp in this case (it's lower than the preferred height). We should instead be getting the preferred height and using that.

(Note: we do still care about the OS-imposed minimum height as a lower-bound on the used min-height, though. e.g. hypothetically if the select element had min-height:0 or min-height: [something small], we would still want to be clamping that value to the OS-theme-imposed minimum height as our "used" min-height value in the flex algorithm.)

Comment 2

[Daniel Holbert [:dholbert]]

For reference, the OS/theme-imposed widget size is determined here:
https://searchfox.org/mozilla-central/rev/e7c61f4a68b974d5fecd216dc7407b631a24eb8f/layout/generic/nsFlexContainerFrame.cpp#1330-1336

At first glance from code inspection, things should be working correctly there -- we seem to be using that as an additional lower-bound on the "dummy" min-height:0, and then we should do further min-size:auto resolution later on in UpdateMainMinSize:
https://searchfox.org/mozilla-central/rev/e7c61f4a68b974d5fecd216dc7407b631a24eb8f/layout/generic/nsFlexContainerFrame.cpp#695-696

(That function actually has special-case in its assertion, specifically to handle this scenario where the themed widget has already established a tentative min size, before we've resolved min-height:auto. It looks like we still should be handling things correctly, at first glance... we should be using the larger of the two.)

Emily: if you have time, it might be worthwhile for you to take a look at this in gdb and see if you can fix it. It's probably not an especially high-priority bug, but it's likely also pretty straightforward to fix, and might be a good way to get more familiar with a reasonably-self-contained chunk of our layout code.