371609 - CSS2 system color "Highlight" should obey color changes in system prefs

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Stefan [:stefanh]]

If you change selection color in system prefs the "Highlight" color doesn't change, it's still blue. You can see this in Firefox's download manager, in multiple <select>:s etc.

Rather than using "-moz-mac-alternateprimaryhighlight" instead of "Highlight", cocoa look and feel should be changed. This will require that we use "-moz-menuhover" for <select>:s styled as menulists (since the selection color in those always should be blue).

Comment 1

[Stefan [:stefanh]]

Attachment: Testcase

Comment 2

[Stefan [:stefanh]]

Attachment: Use kThemeBrushAlternatePrimaryHighlightColor instead of hard-coded blue

Note that I think most browsers will produce a "lighter" Highlight color, ie the normal text selection bg color that we use before bug 371053.

But IMO this is the way to go if we're going to use a darker highlight color. Note that this will make the selection color in <select>:s change according to system prefs. This seems correct in <select>:s styled as listboxes, but not in <select>:s styled as menulists. I don't know the plans for forms.css, but one solution could be to use -moz-menuhover instead. The only problem with that is that it might not be correct for gtk2 (windows and gtk1 uses same color for Highlight and -moz-menuhover, though).

Comment 3

[Stefan [:stefanh]]

Comment on attachment 257154 [details] [diff] [review]
Use kThemeBrushAlternatePrimaryHighlightColor instead of hard-coded blue

I'll file a follow-up bug for replacing our current "-moz-mac-alternateprimaryhighlight" style rules with "Highlight" if this gets through.

Comment 4

[Josh Aas]

Thanks for the patch. It'll be a bit until I get to this, I want to consider it when I'm looking at some similar issues soon.

Comment 5

[Stuart Morgan]

The CSS2 descriptions for Highlight are:

Highlight
    Item(s) selected in a control.
HighlightText
    Text of item(s) selected in a control.

I don't think those should depend on the user's text selection color, since that's not what they appear to describe. I think the current trunk behavior is the correct behavior based on the description.

Comment 6

[Stuart Morgan]

(In reply to comment #2)
> This seems correct in <select>:s styled as listboxes, but not in
> <select>:s styled as menulists.

I'm also not clear on why changing to the text highlight color would be correct for listbox selects; all similar controls I can think of on the OS--table views, outline views, Finder's column view, Safari's implementation of exactly the same control--use the system's white-on-blue select style, not the text selection highlight.

Comment 7

[philippe (part-time)]

Stuart, change your System to use the Graphite theme instead of the Aqua one (System Preferences > Appearance 'Appearance' and 'Highlight' - try Graphite + Gold ). Then Finder, Safari, and others use those colours, whereas Minefield has the white-on-blue hard-coded (as in in <select> listboxes and a bunch of other places in Minefield).

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

At least in 10.3, the "Highlight" pref actually has the hint text "For selected text and lists".  

Beyond that, for each selected color in that "Highlight" list, you actually get two slightly different colors, one lighter one for selected text, and one darker one for selected list items--which I'm sure everyone already knows.

(When the "Highlight" color is set to "Blue", the selected list item color is very close to the "Blue" [non-"Graphite"] menu item color.)

Sooo many colors ;)

Comment 9

[Stuart Morgan]

Wow, now I feel stupid. Sorry about that, I thought that pref was just the selected text highlight based on the 10.4 tip ("For selected text").

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I should add, it would be nice if this happened live, too (we've had bugs on this before which were duped to bug 122000, but maybe live updating belongs, or is already fixed by the patch, here).

Comment 11

[Stefan [:stefanh]]

(In reply to comment #10)
> I should add, it would be nice if this happened live, too (we've had bugs on
> this before which were duped to bug 122000, but maybe live updating belongs, or
> is already fixed by the patch, here).
> 

Live updating is unfortunately not fixed by the patch here. The patch I have submitted simply switches the hard-coded blue color to a constant that'll make the color change according to your system prefs.

Note btw, that the text selection color and list selection color was the same before 10.3. This could be the reason why most browsers still uses the "lighter" color for selected items.

Comment 12

[Josh Aas]

Comment on attachment 257154 [details] [diff] [review]
Use kThemeBrushAlternatePrimaryHighlightColor instead of hard-coded blue

I'd like Mano to review this as well.

Comment 13

[Asaf Romano (gone)]

Comment on attachment 257154 [details] [diff] [review]
Use kThemeBrushAlternatePrimaryHighlightColor instead of hard-coded blue

r=mano

Comment 14

[Stefan [:stefanh]]

Checking in nsLookAndFeel.mm;
/cvsroot/mozilla/widget/src/cocoa/nsLookAndFeel.mm,v  <--  nsLookAndFeel.mm
new revision: 1.10; previous revision: 1.9
done

Comment 15

[Patrick]

Attachment: Screenshot of the download manager

The latest color scheme still breaks the colors of the download manager.

Using 
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a5pre) Gecko/20070508 Minefield/3.0a5pre 
-> the progress bar disappears against the background
-> the "cancel" and "pause" texts are hard to read.

Comment 16

[Stefan [:stefanh]]

> -> the progress bar disappears against the background

I don't see this - the progress bar displays just fine. Note, btw, that this bug didn't really change the default selection color (since the default selection color is blue). But if you change your selection color, you'll now see the new color in DM. For instance, I tend to use "Gold" as selection color - selected items in DM then has a gold-yellowish background-color.

Comment 17

[Patrick]

(In reply to comment #16)
> > -> the progress bar disappears against the background
> 
> I don't see this - the progress bar displays just fine. Note, btw, that this
> bug didn't really change the default selection color (since the default
> selection color is blue). But if you change your selection color, you'll now
> see the new color in DM. For instance, I tend to use "Gold" as selection color
> - selected items in DM then has a gold-yellowish background-color.

You're right. Just after i posted the screenshot i downloaded some other files and that problem was not reproducible anymore. However, the observation about hard-to-read texts is still valid.

Comment 18

[Stefan [:stefanh]]

Yeah, it's not optional (another bug?).

Comment 19

[Patrick]

(In reply to comment #18)
> Yeah, it's not optional (another bug?).
> 

I filed bug 390196 for this