371485 - Selected item text should be white instead of black (given dark blue background color)

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect)

Description

[Jesse Ruderman]

The background color for selected list items was recently changed to dark blue in bug 371053.  The text color in Firefox's download manager should be changed to be readable / look good against the new background.

Comment 1

[Stefan [:stefanh]]

Attachment: One-liner, use HighlightText as color

We didn't change the text color before since we used the lighter system color. What's good is that we now can use HighlightText instead of hard-coding a white text color.

Comment 2

[Stefan [:stefanh]]

Note that the background-color of selected list items in dm isn't changeable via system prefs. This is because we use "Highlight" instead of "-moz-mac-alternateprimaryhighlight" and the "Highlight" color is hard-coded to blue.

Comment 3

[Stefan [:stefanh]]

Filed bug 371609 for the "Highlight" issue.

Comment 4

[Asaf Romano (gone)]

Comment on attachment 256283 [details] [diff] [review]
One-liner, use HighlightText as color

r=mano.

Comment 5

[Jesse Ruderman]

Bug 372427 points out that the download progress information (which is gray) is even harder to read against the dark blue background than the black text.

Comment 6

[Stefan [:stefanh]]

Comment on attachment 256283 [details] [diff] [review]
One-liner, use HighlightText as color

(In reply to comment #5)
> Bug 372427 points out that the download progress information (which is gray) is
> even harder to read against the dark blue background than the black text.
>
Indeed. Better fix that in here as well.

Comment 7

[Stefan [:stefanh]]

OK, so I had this idea that I could keep the gray text for non-selected download's. But it turns out that you'll have to use pretty expensive css in order to achieve this:

download[selected="true"] vbox:first-child > label:last-child,
download[selected="true"] hbox:last-child > label:last-child {
  color: highlightText;
}

Now, we could add an id to the relevant label in download.xml. But that would still leaves us with a style rule with 4 child selectors.

So, for anyone that has read http://developer.mozilla.org/en/docs/Writing_Efficient_CSS this feels a bit expensive ;) .

Comment 8

[Stefan [:stefanh]]

Attachment: Same as previous, but remove gray color

See comment #7 for why I prefer removing the gray color. I fixed a few empty lines in the file as well.

Comment 9

[Stefan [:stefanh]]

Comment on attachment 257138 [details] [diff] [review]
Same as previous, but remove gray color

Uh, wrong patch :(

Comment 10

[Stefan [:stefanh]]

Attachment: Correct patch

Sorry, picked wrong tree.

Comment 12

[Uri Bernstein]

(In reply to comment #0)
> The background color for selected list items was recently changed to dark blue
> in bug 371053.

This might seem silly - but shouldn't the fix for this be just changing the background color back to what it used to be - i.e., the selection color? That's what Safari uses, and I don't see a reason we should do anything different in this case. 

Comment 13

[Stefan [:stefanh]]

(In reply to comment #12)
> (In reply to comment #0)
> > The background color for selected list items was recently changed to dark blue
> > in bug 371053.
> 
> This might seem silly - but shouldn't the fix for this be just changing the
> background color back to what it used to be - i.e., the selection color? That's
> what Safari uses, and I don't see a reason we should do anything different in
> this case. 
>
 "Highlight" is supposed to be "Item(s) selected in a control", according to the specs.

Apple uses two selection colors (10.3 and higher):

1) a text selection color (the one you set in your system prefs, background of selected text). This was the color we used before.
2) a darker version of the text selection color, used in listboxes etc (10.3 and higher). You can see this color in the Finder list views. We use this color in xul trees/listboxes (but it's currently not through the "Highlight" color)

Note that I'm now only talking about the OS selection colors, so I'm not necessarily arguing against you. I have always thought of "Highlight" as the color that is used in selected listboxes etc, though. But I could be wrong.

Comment 14

[Uri Bernstein]

Right, thanks (and I also now see comment #2, which I missed before - sorry).
So I'll follow bug 371609.

Comment 15

[Asaf Romano (gone)]

Comment on attachment 257139 [details] [diff] [review]
Correct patch

Please keep the gray text for now, we can tweak the download bindings later to make this cheaper.

Comment 16

[Stefan [:stefanh]]

I'll get to this in a few days

Comment 17

[Stefan [:stefanh]]

Attachment: New version (keep gray text)

OK, this one will keep the gray "progresstext" when items are non-selected. Do I (really) need ui-review?

Comment 18

[Asaf Romano (gone)]

Comment on attachment 260032 [details] [diff] [review]
New version (keep gray text)

r=mano, thanks.

Comment 19

[Asaf Romano (gone)]

mozilla/toolkit/themes/pinstripe/mozapps/downloads/downloads.css 1.13