<select> button is blank & sized-as-if-empty, with `content:none`

👁 Image	Daniel Holbert [:dholbert] Reporter
	Description • 10 months ago

(spinning this off from bug 1886616)

STR:

1. Load attached testcase.

EXPECTED RESULTS:
The dropdown button should size to fit its <option>s, and should show "Can you see me?" (the selected one) or whatever selection you pick.

ACTUAL RESULTS:
The first dropdown-button is sized as if it were empty.
Both dropdown-buttons are blank (initially, and also if you explicitly try to pick an option).

This is triggered by content:none in the testcase. That style doesn't seem to make a difference here in Chrome, and it doesn't make a difference for other form-controls in Firefox either (I just tried it on button and multi-select <select> elements), so it's weird that we give it special significance for dropdown-menu select elements.

👁 Image	Daniel Holbert [:dholbert] Reporter
	Comment 1 • 10 months ago

Here's a variant with 'appearance:none' to remove native-theme impact. (Same bug happens here.)

👁 Image	Daniel Holbert [:dholbert] Reporter
	Comment 2 • 10 months ago

Regression range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=536a892dd51f394af6c0638166e8cb7e467ae88c&tochange=1f8cbba9a51a930e1f81bd0f711faf057529e4d1

That's just when we added support for content:none in the first place, in bug 1699964 ( https://hg-edge.mozilla.org/mozilla-central/rev/dcc1d0a10a11 )

That did explicitly add a check for content:none to nsComboboxControlFrame.cpp, here:
https://hg-edge.mozilla.org/mozilla-central/rev/dcc1d0a10a11#l3.13

Maybe that's the culprit here?

At that point in time, that check made sense, because the spec text (quoted in bug 1699964 comment 0) said:

On elements, this inhibits the children of the element from being rendered as children of this element, as if the element was empty.

That's kinda what we're doing right now.

However the spec has changed, with none now being defined like so:

On elements, this behaves as normal.

👁 Image	Daniel Holbert [:dholbert] Reporter
	Comment 3 • 10 months ago • Edited

Probably we just need to remove the && !StyleContent()->mContent.IsNone() checks (for sizing and painting purposes):
https://searchfox.org/mozilla-central/rev/45aff2630491c295fc6c28198b629ff2de14e637/layout/forms/nsComboboxControlFrame.cpp#169-170,177

nscoord nsComboboxControlFrame::IntrinsicISize(const IntrinsicSizeInput& aInput,
 IntrinsicISizeType aType) {
...
 if (!containISize && !StyleContent()->mContent.IsNone()) {

https://searchfox.org/mozilla-central/rev/45aff2630491c295fc6c28198b629ff2de14e637/layout/forms/nsComboboxControlFrame.cpp#304,310

void nsComboboxControlFrame::GetLabelText(nsAString& aLabel) {
...
 if (mDisplayedIndex != -1 && !StyleContent()->mContent.IsNone()) {

👁 Image	Daniel Holbert [:dholbert] Reporter
	Updated • 10 months ago

👁 Image	Daniel Holbert [:dholbert] Reporter
	Updated • 10 months ago

👁 Image	Daniel Holbert [:dholbert] Reporter
	Comment 4 • 10 months ago

Hmm, supposedly content:none is supposed to be disabled for non-pseudo-elements, as of bug 1719239, via the layout.css.element-content-none.enabled about:config pref. So it's unexpected that content:none is even affecting a non-pseudo-element here. Clearly our check for that pref isn't as robust as it's intended to be.

(In any case, given the spec discussion in https://github.com/w3c/csswg-drafts/issues/6503 , we probably want to remove that pref entirely rather than polishing it and making it properly control support for content:none on elements.)

👁 Image	Emilio Cobos Álvarez [:emilio] Assignee
	Comment 5 • 10 months ago

That pref is broken, it's getting checked in ToResolvedValue (the thing we expose in getComputedStyle) rather than ToComputedValue...

👁 Image	BugBot [:suhaib / :marco/ :calixte]
	Comment 6 • 10 months ago

Set release status flags based on info from the regressing bug 1719239

👁 Image	Emilio Cobos Álvarez [:emilio] Assignee
	Comment 7 • 10 months ago

👁 Image	Phabricator Automation
	Updated • 10 months ago

👁 Image	Daniel Holbert [:dholbert] Reporter
	Updated • 10 months ago

👁 Image	Pulsebot
	Comment 8 • 10 months ago

Pushed by ealvarez@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/e0cc2150beb9 https://hg.mozilla.org/integration/autoland/rev/83bf92828693 Remove experimental, never-shipped support for content: none on elements. r=dholbert

👁 Image	Gregory Pappas [:gregp]
	Updated • 10 months ago

👁 Image	Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)
	Comment 10 • 10 months ago

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

👁 Image	Pascal Chevrel (relman team) -> :pascalc
	Updated • 10 months ago

👁 Image	Cristian Tuns
	Comment 11 • 10 months ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/83bf92828693

👁 Image	Ryan VanderMeulen [:RyanVM]
	Updated • 10 months ago

👁 Image	Daniel Holbert [:dholbert] Reporter
	Updated • 10 months ago

👁 Image	Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)
	Comment 12 • 10 months ago

Upstream PR merged by moz-wptsync-bot