Last Comment Bug 448767 - [Mac] Allow themes to differentiate between Blue and Graphite system appearance color / Add -moz-system-metric(mac-graphite-theme)
: [Mac] Allow themes to differentiate between Blue and Graphite system appearan...
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All Mac OS X
: -- normal (vote)
: mozilla1.9.1b1
Assigned To: Markus Stange [:mstange]
:
Mentors:
Depends on: 487393
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-01 01:15 PDT by Markus Stange [:mstange]
Modified: 2010-10-22 23:44 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
the fix (7.12 KB, patch)
2008-08-01 01:15 PDT, Markus Stange [:mstange]
vladimir: review+
dbaron: superreview+
Details | Diff | Splinter Review
make it live (2.12 KB, patch)
2008-08-06 13:51 PDT, Markus Stange [:mstange]
roc: review+
roc: superreview+
Details | Diff | Splinter Review
Testcase (145 bytes, text/html)
2008-08-06 13:52 PDT, Markus Stange [:mstange]
no flags Details
updated patch (7.61 KB, patch)
2008-09-09 17:13 PDT, Markus Stange [:mstange]
no flags Details | Diff | Splinter Review

Description Markus Stange [:mstange] 2008-08-01 01:15:41 PDT
Created attachment 331915 [details] [diff] [review]
the fix
Comment 1 Aronnax 2008-08-01 02:28:53 PDT
Hi, 
Markus = the Mac theme producers best friend 

and here is one more example why  ;-)   

many thanks
Comment 2 Markus Stange [:mstange] 2008-08-06 13:51:02 PDT
Created attachment 332585 [details] [diff] [review]
make it live

Bug 447045 makes this possible.
Comment 3 Markus Stange [:mstange] 2008-08-06 13:52:17 PDT
Created attachment 332586 [details]
Testcase
Comment 4 David Baron :dbaron: ⌚️UTC+1 (busy September 14-25) 2008-08-08 16:02:14 PDT
Comment on attachment 331915 [details] [diff] [review]
the fix

>+    case eMetric_MacGraphiteTheme:
>+      aMetric = [NSColor currentControlTint] == NSGraphiteControlTint ? 1 : 0;
>+      break;

This seems a little hacky; at least comment explaining why there isn't a better way, perhaps?  It might be good to have somebody with a mac background review this bit.

Also, the " ? 1 : 0" is unneeded.  Booleans convert to integers exactly the way you want.

sr=dbaron
Comment 5 Markus Stange [:mstange] 2008-08-08 16:45:19 PDT
(In reply to comment #4)
> This seems a little hacky; at least comment explaining why there isn't a better
> way, perhaps?

Why does it seem hacky?
According to http://developer.apple.com/documentation/Cocoa/Conceptual/DrawColor/Tasks/SystemTintAware.html it's the preferred way to do it:

> You can get the current system-wide control tint using the NSColor class
> method currentControlTint. This method returns an NSControlTint that
> represents the currently selected appearance color. Currently
> NSBlueControlTint or NSGraphiteControlTint are the possible return values
> representing the Aqua and Graphite appearances respectively.


> Also, the " ? 1 : 0" is unneeded.  Booleans convert to integers exactly the
> way you want.

Uh, yeah.
Comment 6 David Baron :dbaron: ⌚️UTC+1 (busy September 14-25) 2008-08-08 16:51:41 PDT
(In reply to comment #5)
> According to
> http://developer.apple.com/documentation/Cocoa/Conceptual/DrawColor/Tasks/SystemTintAware.html
> it's the preferred way to do it:

ok.
Comment 7 Rob Arnold [:robarnold] 2008-08-12 15:32:52 PDT
Can we also add a special color to for the tint? -moz-mac-tint? We do this for several windows-only colors currently.
Comment 8 Markus Stange [:mstange] 2008-08-12 16:19:38 PDT
I think there is already a color for that: -moz-mac-focusring
Or did you have something different in mind?
Comment 9 Rob Arnold [:robarnold] 2008-08-12 16:52:11 PDT
I didn't know if that was the same color as the tint. I don't use OSX very often :)
Comment 10 Vladimir Vukicevic [:vlad] [:vladv] 2008-09-09 10:39:30 PDT
Comment on attachment 331915 [details] [diff] [review]
the fix

Gah, I hate that we're propagating these CSS selectors for themes, but I don't know of a better way to do this :(
Comment 11 Markus Stange [:mstange] 2008-09-09 17:13:41 PDT
Created attachment 337790 [details] [diff] [review]
updated patch
Comment 13 Henrik Skupin (:whimboo) 2008-12-06 12:41:08 PST
Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2) Gecko/20081201 Firefox/3.1b2 ID:20081201061100

Any focusable widget has a nice graphite tint now, except the location and search bar. But this is covered by bug 422203.
Comment 14 Henrik Skupin (:whimboo) 2008-12-06 12:44:49 PST
Would even be nice to have this in Litmus.
Comment 15 Markus Stange [:mstange] 2008-12-06 14:37:40 PST
(In reply to comment #13)
> Any focusable widget has a nice graphite tint now

This is not really what this bug is about. Currently the default theme doesn't use -moz-system-metric(mac-graphite-theme), so you can't really verify it without looking at the testcase (attachment 332586 [details]).
Another way to verify it is to install the latest GrApple theme from takebacktheweb.org and look at the location bar focus ring color.
Comment 16 Henrik Skupin (:whimboo) 2008-12-06 15:01:21 PST
Yes, the background color of attachment 332586 [details] is changing when switching between these two modes. I've also installed the GrApple theme and everything works fine. The focus tint of the location bar changes to graphite. So this time it's really verified. Thanks for the hint, Markus.
Comment 17 Jose Fandos 2009-01-03 15:39:32 PST
(In reply to comment #13)
> Verified with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US;
> rv:1.9.1b2) Gecko/20081201 Firefox/3.1b2 ID:20081201061100
> 
> Any focusable widget has a nice graphite tint now, except the location and
> search bar. But this is covered by bug 422203.

What about the Find bar that appears when you press Command + F? Shouldn't that now change accordingly? It doesn't.
Comment 18 Henrik Skupin (:whimboo) 2009-01-03 16:02:47 PST
(In reply to comment #17)
> What about the Find bar that appears when you press Command + F? Shouldn't that
> now change accordingly? It doesn't.

Good catch. I've added this to bug 422203.
Comment 19 Henrik Skupin (:whimboo) 2009-02-05 08:21:23 PST
I'll remove the in-litmus flag for now. We should wait until the default theme will support -moz-system-metric completely.

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