513493 - Adjust Unified Toolbar to Match Snow Leopard

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Daniel Spiewak]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2

It seems that in Snow Leopard, Apple has slightly adjusted the gradient coloring for the standard unified toolbar.  Specifically, everything is just a shade lighter with a hint of tan.  This means that Firefox's unified toolbar looks a bit out of place when compared to Safari and Finder.  Screenshots to follow.

In keeping with Firefox's track record of visually catering to the latest version of Mac OS X, I propose that the unified toolbar gradient be adjusted in Firefox.next to match the Snow Leopard standard.

Reproducible: Always




Filed as a bug rather than an RFE as this issue is disruptive of Firefox's visual integration with the Mac platform.

Comment 1

[Daniel Spiewak]

Attachment: Unified Toolbar in Firefox

Comment 2

[Daniel Spiewak]

Attachment: Unified Toolbar in Safari 4

Comment 3

[Daniel Spiewak]

Attachment: Unified Toolbar in Finder

Comment 4

[Markus Stange [:mstange]]

Could you also upload screenshots of inactive windows, i.e. with the lighter gradient? Specifically, one screenshot of the inactive Firefox preferences window and one of the inactive Safari preferences window would be most useful.

Comment 5

[Daniel Spiewak]

Attachment: Unified Toolbar in Safari 4 (inactive)

Comment 6

[Daniel Spiewak]

Attachment: Unified Toolbar in Finder (inactive)

Requested screenshots now attached

Comment 7

[Daniel Spiewak]

Attachment: Unified Toolbar in Firefox Prefs (inactive)

I need to learn to read.  Sorry about the comment spam...

Comment 8

[Daniel Spiewak]

Attachment: Unified Toolbar in Safari 4 Prefs (inactive)

Comment 9

[John Locke]

Hi, I'm wondering when we can have these matched to Snow Leopard ?
1. unified toolbar
2. -moz-chrome-active and -moz-chrome-inactive
3. -moz-mac-focusring

And... will this be fixed in Firefox 3.5.*? I'm afraid when we have Firefox 3.6 and 3.7 will be 1 year later, I guess... because they are still in alpha right now:(

Thank you :)

Comment 10

[Markus Stange [:mstange]]

Oh, the focus ring colors have changed, too? Interesting.
I'll create a patch soon.

Comment 11

[Markus Stange [:mstange]]

Attachment: v1

Comment 12

[Markus Stange [:mstange]]

Comment on attachment 401772 [details] [diff] [review]
v1

new patch coming

Comment 13

[Markus Stange [:mstange]]

Attachment: v2

Comment 14

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/9bbef3004ce8

Comment 15

[Samuel Sidler (old account; do not CC)]

This really needs to land on 1.9.2 before we take it on 1.9.1.

Comment 16

[Markus Stange [:mstange]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/923d408f8db4

Comment 17

[Markus Stange [:mstange]]

Huh, a SunSpider regression on 10.4?

http://graphs.mozilla.org/graph.html#tests=[{%22machine%22:148,%22test%22:25,%22branch%22:10},{%22machine%22:149,%22test%22:25,%22branch%22:10},{%22machine%22:150,%22test%22:25,%22branch%22:10},{%22machine%22:151,%22test%22:25,%22branch%22:10},{%22machine%22:152,%22test%22:25,%22branch%22:10},{%22machine%22:153,%22test%22:25,%22branch%22:10},{%22machine%22:154,%22test%22:25,%22branch%22:10},{%22machine%22:155,%22test%22:25,%22branch%22:10},{%22machine%22:156,%22test%22:25,%22branch%22:10},{%22machine%22:157,%22test%22:25,%22branch%22:10},{%22machine%22:158,%22test%22:25,%22branch%22:10},{%22machine%22:159,%22test%22:25,%22branch%22:10},{%22machine%22:160,%22test%22:25,%22branch%22:10},{%22machine%22:161,%22test%22:25,%22branch%22:10},{%22machine%22:162,%22test%22:25,%22branch%22:10},{%22machine%22:163,%22test%22:25,%22branch%22:10},{%22machine%22:164,%22test%22:25,%22branch%22:10},{%22machine%22:165,%22test%22:25,%22branch%22:10}]&sel=1256324760,1256497560

Backed out of 1.9.2 for now:
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/b2cc08e4e372
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/5f3963040ee2

Comment 18

[Boris Zbarsky [:bzbarsky]]

This really shouldn't be affecting sunspider.  How is that test being run?

Comment 19

[Markus Stange [:mstange]]

Each sunspider test seems to cause redraws of the stop button, the tab title and the status bar. Maybe these redraws fall in the measured time window?
Since this patch makes us check the OS version every time we draw one of the native grey colors, those draw operations might take a tiny bit longer.

Comment 20

[Markus Stange [:mstange]]

Huh, no, the OS version is cached. Weird.

Comment 21

[Boris Zbarsky [:bzbarsky]]

> Maybe these redraws fall in the measured time window?

They shouldn't, unless something is really wrong in terms of how the test is measured, imo.

Comment 22

[Daniel Veditz [:dveditz]]

Comment on attachment 402018 [details] [diff] [review]
v2

Doesn't seem like the kind of fix we have to take on the stable branch. 1.9.1 approval denied.

Comment 23

[Markus Stange [:mstange]]

Attachment: 1.9.2 patch without regression

Removing the Objective C exception guards in OSXVersion() fixes the regression.
We don't make any Objective C calls in that method, so they shouldn't be necessary.

Comment 24

[Markus Stange [:mstange]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/a2af57fed584

Comment 25

[Markus Stange [:mstange]]

(In reply to comment #19)
> Each sunspider test seems to cause redraws of the stop button, the tab title
> and the status bar.

Actually, that's wrong. SunSpider is run in a window where all toolbars and the statusbar and tabbar are hidden.
The only thing that's redrawn is the window's title bar when the title changes.