Closed Bug 1762497 Opened 2 years ago Closed 2 years ago

!important prevents styling unfocused checked option in listbox

Categories

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

Firefox 99
defect

Tracking

()

RESOLVED FIXED
102 Branch
Tracking Status
firefox102 --- fixed

People

(Reporter: u8wdwl699q, Assigned: emilio)

References

Details

Attachments

(2 files)

Attached image firefox_option.png

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:99.0) Gecko/20100101 Firefox/99.0

Steps to reproduce:

Developing a web application, I added styling to change the option:checked background-color and color (application has both dark and light themes using css variables)

Actual results:

App styling was not used. Looking at the built-in styling (resource://gre-resources/forms.css), it appears, like a lot of properties in forms.css, the styling uses the !important flag.

However, even adding more specificity and adding the !important flag to the styling, the app styling was still not used.

(For comparison, Chromium does not force its styling (at least for option:checked) and it can easily be overwritten.)

The tested styling was:
select option:checked {
background-color: var(--selected-background) !important;
color: var(--selected-color) !important;
}

:root {
--selected-background: #111;
--selected-color: #AAA;
}

Attached image shows inspector, style and computed style

Expected results:

All built-in styling should not use the !important flag and should be able to be overridden by the pages styles.

A quote from MDN (https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity#the_!important_exception)
"Using !important, however, is bad practice and should be avoided because it makes debugging more difficult by breaking the natural cascading in your stylesheets. When two conflicting declarations with the !important rule are applied to the same element, the declaration with a greater specificity will be applied."

Apologies for the inclusion of the quote - it is unproductive. I would remove it, but I can't. I'm sure the !important flags were used for a reason, but hopefully there is a better way (given how powerful !import is).

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

Component: Untriaged → Inspector
Product: Firefox → DevTools

The request here is about styling an option, not directly related to devtools.

Component: Inspector → DOM: Core & HTML
Product: DevTools → Core
Component: DOM: Core & HTML → Layout: Form Controls
See Also: → 1553930

So, you're right that we're using !important, but that's for a good reason.

Consider this:

option { background: red; color: white }

Without !important now there's no way to recognize the selected option. Chrome seems to be fine with that, but still doesn't allow you to override this while the select has focus.

So in order to get Chromium's behavior we could have something like this:

diff --git a/layout/style/res/forms.css b/layout/style/res/forms.css
index 1366613213a55..73b51e05f0999 100644
--- a/layout/style/res/forms.css
+++ b/layout/style/res/forms.css
@@ -332,12 +332,11 @@ select > option {
 }
 
 option:checked {
-  background-color: -moz-cellhighlight !important;
-  color: -moz-cellhighlighttext !important;
+  background-color: -moz-cellhighlight;
+  color: -moz-cellhighlighttext;
 }
 
-select:focus > option:checked,
-select:focus > optgroup > option:checked {
+select:focus option:checked {
   background-color: SelectedItem !important;
   color: SelectedItemText !important;
 }

But I'm not sure the result is particularly better? Now you need to focus the select in order to figure out what's selected in a case like the above.

But maybe since other browsers support this it's not such big of a deal, I guess we can try.

Summary: !import flags used in resource://gre-resources/forms.css → !important prevents used in resource://gre-resources/forms.css
Summary: !important prevents used in resource://gre-resources/forms.css → !important prevents styling unfocused checked option in listbox
Severity: -- → S3
Priority: -- → P3

See bug comments for reasoning. While this could cause undistinguishable
selected options when the select doesn't have focus, the idea is that
this shouldn't be happening often. This matches other browsers.

Assignee: nobody → emilio
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1d85507a5598
Allow authors to style unfocused option listboxes. r=dholbert
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/875dc4244896
Annotate a test that's now passing.
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch

Sorry - didn't see the comments. Some comments below.

Just did a bit more double checking (in particular checking select:focus option:checked overriding). Part of the issue is page styling even with !important flags and higher specificity than the internal styling does not overwrite the in-built styling. This can also be seen in the MDN example https://developer.mozilla.org/en-US/docs/Web/CSS/:checked#basic_example (both for option:checked and for select:focus option:checked. At least in the version/theme I am using (FFDev101 in dark mode)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

So, you're right that we're using !important, but that's for a good reason.

Consider this:

option { background: red; color: white }

Without !important now there's no way to recognize the selected option. Chrome seems to be fine with that, but still doesn't allow you to override this while the select has focus.

Isn't that where CSS specificity comes in?

option { background: red; color: white }

would be overwritten by the :checked styling

option:checked {
  background-color: -moz-cellhighlight;
  color: -moz-cellhighlighttext;
}

Using the !important flag should be superfluous. I suppose the case where it wouldn't be would be where you have extra specificity on the rule, eg select.special > option. I can see the reason for the !important flag in that case, which I guess fair. The issue would be that even with the !important flag and higher specificity, I still don't seem to be able to override the inbuilt styling.

So in order to get Chromium's behavior we could have something like this:

diff --git a/layout/style/res/forms.css b/layout/style/res/forms.css
index 1366613213a55..73b51e05f0999 100644
--- a/layout/style/res/forms.css
+++ b/layout/style/res/forms.css
@@ -332,12 +332,11 @@ select > option {
 }
 
 option:checked {
-  background-color: -moz-cellhighlight !important;
-  color: -moz-cellhighlighttext !important;
+  background-color: -moz-cellhighlight;
+  color: -moz-cellhighlighttext;
 }
 
-select:focus > option:checked,
-select:focus > optgroup > option:checked {
+select:focus option:checked {
   background-color: SelectedItem !important;
   color: SelectedItemText !important;
 }

But I'm not sure the result is particularly better? Now you need to focus the select in order to figure out what's selected in a case like the above.

But maybe since other browsers support this it's not such big of a deal, I guess we can try.

It would be good to get rid of the !important flags on the select:focus option:checked rules as well.

(In reply to Jack from comment #9)

(FFDev101 in dark mode)

(FF on Ubuntu)

(In reply to Jack from comment #9)

Isn't that where CSS specificity comes in?

option { background: red; color: white }

would be overwritten by the :checked styling

option:checked {
  background-color: -moz-cellhighlight;
  color: -moz-cellhighlighttext;
}

Using the !important flag should be superfluous. I suppose the case where it wouldn't be would be where you have extra specificity on the rule, eg select.special > option. I can see the reason for the !important flag in that case, which I guess fair. The issue would be that even with the !important flag and higher specificity, I still don't seem to be able to override the inbuilt styling.

It is not superfluous, and specificity doesn't matter in this case because browser styles are loaded in a different origin. That's why you can't override with !important no matter what. See https://drafts.csswg.org/css-cascade-5/#cascading-origins

(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)

It is not superfluous, and specificity doesn't matter in this case because browser styles are loaded in a different origin. That's why you can't override with !important no matter what. See https://drafts.csswg.org/css-cascade-5/#cascading-origins

Wow. Funky. Cool. Thanks for the info - I never knew about the inversion. That would explain it.

It would be nice to be able to adjust the select:focus option:checked styling as well, but yeah, it matches other browsers, so it'll do. Thank you very much for looking at it.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: