428713 - [proto] nsLookAndFeel color for sidebar (instead of hardcoded css color)

Status: RESOLVED WONTFIX

(Core :: Widget: Cocoa, enhancement)

Description

[Stefan [:stefanh]]

Attachment: use nsLookAndFeel color "-moz-mac-primarysourcelistbackground"

If the current sidebar background is supposed to match the finder one, it's better to have a lookAndFeel color for it (see https://bugzilla.mozilla.org/show_bug.cgi?id=403169#c12).


Note to Kevin:
browser/themes/pinstripe/browser/places/organizer.css:

173 #placesList {
174   -moz-appearance: none;
175   background-color: #d2d8e2;
176   width: 160px;
177   margin: 0px;
178   border: 0px;
179 }

I'm not going to change the background color here, because it's obviously not the same as #d6dde5... Is the color difference intentional?

Comment 1

[Kevin Gerich]

The difference is not intentional. please make the change to #placesList too. Thanks

Comment 2

[Kevin Gerich]

What about the color of a sidebar where the window doesn't have focus? Can we get that from the system as well?

Comment 3

[Stefan [:stefanh]]

(In reply to comment #1)
> The difference is not intentional. please make the change to #placesList too.
> Thanks
> 
OK, will do.

(In reply to comment #2)
> What about the color of a sidebar where the window doesn't have focus? Can we
> get that from the system as well?
> 
Just note that I don't really take the color "from the system" if you by system mean the OS. What I do in this patch is that I just hardcode the color in nsLookAndFeel in order to make it safe for using with color management. If there are those kind of issues with e8e8e8, I guess we could do it. Is this color used elsewhere (for other widgets where the window doesn't have focus)? 

Comment 4

[Stefan [:stefanh]]

Actually, can you get away with "-moz-appearance: dialog;"?

Comment 5

[Stefan [:stefanh]]

I just tried using #e8e8e8 in the css files and the color do change if you enable color management. So, maybe we should implement a nsLookAndFeel color for the inactive color as well.

Comment 6

[Stefan [:stefanh]]

Comment on attachment 315309 [details] [diff] [review]
use nsLookAndFeel color "-moz-mac-primarysourcelistbackground"

>Index: browser/themes/pinstripe/browser/browser.css
>===================================================================
>RCS file: /cvsroot/mozilla/browser/themes/pinstripe/browser/browser.css,v
.
.
.
> 
> #sidebar-box {
>   -moz-appearance: dialog;
>   -moz-appearance: none;
> }

Heh, I guess it's the last we want here ;-)

Comment 7

[Stefan [:stefanh]]

Attachment: Use inactive/active colors

This version uses two colors, one for the "active" state and one for the "inactive" state. The inactive color is not used in the theme atm - I'll leave that to Kevin. comment #1 and comment #6 is addressed.

I'm a bit confused here, though. I just discovered that those colors are not 100% constant if you try different color profiles when you have "gfx.color_management.enabled" set to "true"... They seem to vary slightly if I sample them.

Comment 8

[Stefan [:stefanh]]

With "gfx.color_management.enabled" set to "true" it seems that the colors do change slightly, depending on what color profile I use. 

If I sample the colors with Pixie, I get the following values for each profile:

S19-1 --> (231, 232, 233) and (213, 221, 229)
S19-1 calibrated --> (232, 232, 231) and (214, 221, 229)

Adobe RGB(1998) --> (232, 232, 232) and (214, 221, 229)
Generic RGB profile --> (233, 232, 233) and (214, 221, 229)
sRGB IEC61966-2.1 --> (232, 232, 232) and (214, 221, 229)
Syncmaster calibrated --> (233, 232, 233) and (214, 221, 229)


tor, Stuart - can anyone of you explain this? I'd expect the colors to always be the same, since the OS colors seems to be... Maybe I haven't understood how color management works?

Comment 9

[Stefan [:stefanh]]

Attachment: Testcase for attachment #315374

Attaching a testcase if someone wants to try the patch.

Comment 10

[Stefan [:stefanh]]

Attachment: Example of how "-moz-appearance: dialog;" can be used

Kevin,

I I've been playing a bit with the active/disabled states for the sidebar. I think using "-moz-appearance: dialog;" should work fine. Since the "disabled" color for the sidebar seems to be identical to the background color of a dialog it seems a bit of an overkill to add a new color in nsLookAndFeel for that.

I'm attaching a diff of what I did (browser.css contains stuff from from experiments in bug 406730, but you get the idea). Note that by adding some new rules, you can get away with just using one style rule for the disabled color ;-)

Comment 11

[Kevin Gerich]

(In reply to comment #10)
> Created an attachment (id=315569) [details]
> Example of how "-moz-appearance: dialog;" can be used

IIRC, -moz-appearance: dialog; gives a pinstriped background on Tiger, which might look odd in the sidebar. I may be mistaken though. I'll try it out. Thanks!

Comment 12

[Stefan [:stefanh]]

(In reply to comment #11)

> IIRC, -moz-appearance: dialog; gives a pinstriped background on Tiger, which
> might look odd in the sidebar

Hmm, you're right :-/

Comment 13

[Stefan [:stefanh]]

Hmm. I start to wonder whether this is a good idea. Thinking of it, since the background of the sidebar should be white on Tiger, a "system" color should really return white if you're on tiger. This is obviously not what proto wants, but I'm a bit unsure if it's acceptable for a nsLookAndFeel color to have the same color on all OS versions (when it shouldn't)...

Comment 14

[Markus Stange [:mstange]]

I think it's completely OK to return the Leopard color on Tiger. That's the direction Firefox has taken - with bug 439354, even Tiger's normal titlebars use the Leopard colors.

I'm going to convert a lot of the stuff that's currently themed nativey using CSS to using -moz-appearance. Maybe, if there's still time, at one point we can change the theme rendering to draw everything in Tiger style on Tiger without ending up with a zombie look - I'd actually love to do that. But I don't know if that's something the people in charge want.

Comment 15

[Markus Stange [:mstange]]

(In reply to bug 439354 comment #19)
> If the patch gets in, it'll also be possible to fix the
> statusbar/sidebar in a much saner way (since you can access info about the
> state of the window from nsNativeThemeCocoa).

So you're going to make the sidebar a -moz-appearance instead of an nsLookAndFeel color?

Comment 16

[Stefan [:stefanh]]

(In reply to comment #15)
> (In reply to bug 439354 comment #19)
> > If the patch gets in, it'll also be possible to fix the
> > statusbar/sidebar in a much saner way (since you can access info about the
> > state of the window from nsNativeThemeCocoa).
> 
> So you're going to make the sidebar a -moz-appearance instead of an
> nsLookAndFeel color?
> 

That seems like the best way (once you have fixed bug 439354). I still don't like the non-difference between Tiger and Leopard, though. But I guess no one will get it wrong if the implementation is done as a mac-specific NS_THEME_MOZ_MAC_SIDEBAR, documented as a "Leopard-styled primary source list" 

BTW, is there a bug for the statusbar?

Comment 17

[Markus Stange [:mstange]]

(In reply to comment #16)
> BTW, is there a bug for the statusbar?

If there is, I don't know about it. And we need a bug for normal toolbars (chrome toolbars under the unified toolbar, see DOM Inspector).

Comment 18

[Stefan [:stefanh]]

Actually, looking at safari/camino on Tiger it'd make sense to differentiate between Tiger and Leopard.

Comment 19

[Markus Stange [:mstange]]

How do you get a sidebar in Camino?
(And Safari on Tiger is generally ugly...)

Comment 20

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

>I'm going to convert a lot of the stuff that's currently themed nativey using
>CSS to using -moz-appearance.

It would be really useful if we could get all of the colors that OS X exposes, beyond just the color for sidebars.

Comment 21

[Stefan [:stefanh]]

Markus,

I'll be away for a couple of weeks, so feel free to take this ;-)

Comment 22

[Stefan [:stefanh]]

I'm resolving this as wontfix, since we now do the sidebar background with "-moz-appearance: -moz-mac-source-list;".