425561 - Sidebar background color off on OSX

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Boris Zbarsky [:bzbarsky]]

I mentioned this to Alex a while back, but hadn't gotten around to filing this till now...

Over here, on Tiger, the sidebar background color Firefox shows is rgb(214,221,229) (determined by taking a screenshot with Command-Shift-4 and then using the GIMP color picker).  The iTunes sidebar color (which I understood this was meant to match) is rgb(209,215,226).  So it's a tiny bit darker.

In addition to the above, the contrast between the light color we use here and the very dark tabbar coloring is pretty jarring, at least to me.  Using the iTunes color would help some, though I wonder wheher we could somehow make the whole tabbar a little lighter, or whether we're not so worried about the way the two colors interact.

Comment 1

[Alex Faaborg [:faaborg] (Firefox UX)]

On Leopard here are the colors of various sidebars:

Finder       (214, 221, 229)
Mail.app 3.2 (212, 221, 230)
Preview 4.1  (231, 234, 245)
iTunes       (209, 215, 226)
iPhoto 7.1.2 (209, 215, 226)
iWeb 2.0.3   (213, 219, 228) some variability throughout the sidebar
Keynote 4.0.2(212, 219, 228) some variability throughout the sidebar
Numbers 1.0.2(212, 219, 228) some variability throughout the sidebar
Pages 3.0.2  (212, 219, 228) some variability throughout the sidebar

So, wow, apple is kind of sucking at having consistent sidebar colors.

Matching the finder as we currently do seems to make sense.  We could potentially use chrome overrides to get the correct lighter Finder sidebar color on Tiger, but given the massive amounts of inconsistently amongst apple applications, I'm not sure that is worth it.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Do all those applications have the same color immediately to the right of the sidebar (esp. the sidebar top)?  

Comment 3

[Stephen Horlander [:shorlander]]

On Snow Leopard the Finder, Mail, iCal and Safari (in Bookmarks Manager) are all (226,231,237) iTunes is darker and Preview is lighter. I am not too worried about iLife apps since they do their own thing most of the time anyway.

I think we should just match the Finder as it is kind of the "base" application and it is consistent with the most applications.

The separator is pretty light at (189,189,189) almost everywhere but Safari which is darker.

I think we should go rgb(226,231,237) for the background color and rgb(189,189,189) for the separator.

Comment 4

[Alex Faaborg [:faaborg] (Firefox UX)]

Yeah, we should match the new Finder sidebar color on snow leopard.  Looks like they keep tweaking.  We really need to get all of these colors defined in one place and every time a new OS comes out do a thorough pass to check all of them.

Comment 5

[Josiah Bruner [:jsbruner]]

We now should definitely lighten up the color to match Lion's pane. Even on Snow Leopard, are pane is way to dark.

I'm going to take this and try some different colors.

Comment 6

[Josiah Bruner [:jsbruner]]

Er... Our pane is way to dark.

Comment 7

[Josiah Bruner [:jsbruner]]

Attachment: Patch.

Changes all sidebar panes, including inside Organizer, to use the new color. The colors are what Stephen suggested, and I am also requesting yet another uireview from him.

Dão, are you the right person to review for this?

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 733534 [details] [diff] [review]
Patch.

> #placesList > treechildren::-moz-tree-row {
>-  background-color: transparent;
>+  background-color: #E2E7ED;

Why this change?

Comment 9

[Josiah Bruner [:jsbruner]]

(In reply to Dão Gottwald [:dao] from comment #8)
> Comment on attachment 733534 [details] [diff] [review]
> Patch.
> 
> > #placesList > treechildren::-moz-tree-row {
> >-  background-color: transparent;
> >+  background-color: #E2E7ED;
> 
> Why this change?


Well, why not this change. I suppose using transparent is perfectly acceptable, however, the way I see it, it's easier to understand how the color is working here when the tree row background is this color. 


However, if you think I should keep it transparent, then that is fine. I'll have an update a little later.

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 733534 [details] [diff] [review]
Patch.

Yes, please keep transparent. Also please make the hex values lowercase.

Comment 11

[Josiah Bruner [:jsbruner]]

Attachment: Patch.

Addresses feedback. Resetting review flags.

Comment 12

[Josiah Bruner [:jsbruner]]

Stephen, would you like to pass the ui-review on? You are probably busy with Australis reviews at the moment.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/50e931146b0e

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/50e931146b0e