Closed Bug 1476127 Opened 6 years ago Closed 5 years ago

Implement contain:size for select objects.

Categories

(Core :: Layout: Form Controls, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: morgan, Assigned: dholbert)

References

()

Details

Attachments

(1 file, 1 obsolete file)

Like the spec on size-containment mentions [1], objects that are size contained should render as if they have no contents. Currently, select objects (nsComboboxControlFrame) with no defined width render slightly smaller than empty, select objects without containment. They should render identically. I think this problem has to do with the select code calling nsBlockFrame methods (particularly getPrefISize and getMinISize) in cases without size-containment. In selects that are not size-contained, iSize comes in part from [2] a call to the child to determine its preferred iSize (this call propogates up to setting displayISize [3] which is a good spot to start debugging) In selects with size-containment, however, we avoid this call altogether based on the assumption that a select object without contents returns zero from that call; this assumption is incorrect. Instead, a value of ~400 nscoord units is returned. [1] https://drafts.csswg.org/css-contain/#containment-size [2] https://searchfox.org/mozilla-central/source/layout/generic/nsBlockFrame.cpp#874 [3] https://searchfox.org/mozilla-central/source/layout/forms/nsComboboxControlFrame.cpp#785
Component: Layout → Layout: Form Controls
Summary: Adjust intrinsically-sized, size-contained selects to render with non-zero isize → Adjust intrinsically-sized, size-contained <select> elements to render with non-zero isize
Depends on: 1470176
No longer depends on: 1470176
Summary: Adjust intrinsically-sized, size-contained <select> elements to render with non-zero isize → Implement contain:size for select objects.
Depends on: 1492338
Assignee: nobody → dholbert
Status: NEW → ASSIGNED

Note: Chrome seems not to honor contain:size at all on dropdown <select> elements though they do honor it correctly on (non-dropdown) <select multiple> elements.

I filed https://bugs.chromium.org/p/chromium/issues/detail?id=966673 on them.

I'm tentatively planning on splitting out the reftest part of Morgan's patch, and landing that on its own (with the tests annotated as failing), with r=me. Then I'll move on to looking into a possible fix.

(because, as I recall, the code part of the attached reviewboard patch isn't quite correct/complete, in part due to bug 1492338. Though, it might still be a pretty-reasonable stepping-stone / compromise in light of bug 1492338, though.... I'll see.)

Depends on: 1561717

(In reply to Daniel Holbert [:dholbert] from comment #3)

Note: Chrome seems not to honor contain:size at all on dropdown <select> elements

Update: they've fixed this, at least as of Chrome version 77.0.3833.0 (Official Build) dev (64-bit)

I'm tentatively planning on splitting out the reftest part of Morgan's patch, and landing that on its own (with the tests annotated as failing)

I just did this in helper bug 1561717. (The aforementioned Chrome version passes both reftests that I landed there, BTW.)

I'm going to be posting a "mostly-there" fix here, and I'm going to spin off a followup for the remaining work.

With my patch, we'll size all <select style="contain:size"> elements independently from their content, but we won't quite size them the same as we do for empty <select> elements (which is really what we should be doing) -- they'll be a bit skinnier.

This is because empty <select></select> elements aren't actually empty. Their frametree looks like this (cleaned up a bit for readability):

             ComboboxControl(select)(1)<
                line  <
                  ComboboxDisplay(select)(1)[cs=7fe232ee93d8:-moz-display-comboboxcontrol-frame]<
                    line <
                      Text(-1)"\u00a0"[cs=7fe232ee9888:-moz-text] [run=0][0,1,T] 
                    >

Note the \u00a0 character there -- that is an &nbsp; nonbreaking-space character.

So, "empty" <select> elements get a little bit of width from that automatically-inserted character, which means our usual contain:size behavior (use a zero size and/or emulate the we-have-no-descendant-frames codepath) doesn't quite do the right thing here. I think that's a reasonable compromise behavior for now, though, because:
(a) with native widget styling (on linux at least), we actually do end up matching the size of an empty select element, probably because the widget has a system-imposed minimum size or something.
(b) the main goal is to size independently of your contents, and we match that goal. (This is what enables e.g. reflow root optimizations.)
(c) the difference is pretty subtle.
(d) there aren't really any real-world use-cases for a size-contained auto-sized <select> element that I can think of (not any that would benefit from having exactly one &nbsp; worth of width, at least), so I doubt anyone will care about this not-quite-complete implementation in practice.

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5125acaaf6b0f37c6b1646f049015e2dddb63ac3

Note that I've optimistically marked the two layout/reftests/w3c-css/submitted/contain/ reftests (recently added in helper-bug 1561717) as passing, since they do pass on my system. I'm not 100% sure they'll pass on other platforms, but Try will let me know.

I did leave the WPT test failure-annotations untouched (for similarly-named-but-different WPT tests from rego):
https://searchfox.org/mozilla-central/source/testing/web-platform/meta/css/css-contain/contain-size-select-001.html.ini
https://searchfox.org/mozilla-central/source/testing/web-platform/meta/css/css-contain/contain-size-select-002.html.ini

Those tests fail due to the issue described in comment 6 -- the testcase renders like [v] and the reference case renders like [ v], basically. ("v" = my ascii art for a dropdown arrow) Those WPT tests exercise the unthemed-widget behavior (by virtue of setting a css background), and that behavior has "tighter" widget sizes than my system's native theming, and hence it reveals the &nbsp;-vs-truly-empty sizing difference discussed above.

Blocks: 1562057

I filed Bug 1562057 as a followup to address the remaining issue that I discussed above in comment 6.

Attachment #8993746 - Attachment is obsolete: true
Blocks: 1562312
Pushed by dholbert@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8bf1ded5ea19 Implement 'contain:size' for select elements. r=TYLin
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: