Last Comment Bug 702558 - FF8 breaks themes using default Toolbar.png file path in Mac OSX Lion (toolbar buttons not loading)
: FF8 breaks themes using default Toolbar.png file path in Mac OSX Lion (toolba...
Status: RESOLVED WORKSFORME
[qa+]
: regression
Product: Firefox
Classification: Client Software
Component: Theme (show other bugs)
: 8 Branch
: x86 Mac OS X
: -- major with 2 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
:
Mentors:
Depends on:
Blocks: 667480 706103
  Show dependency treegraph
 
Reported: 2011-11-15 02:58 PST by KLB
Modified: 2014-05-08 05:43 PDT (History)
21 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
-


Attachments
use media queries for Toolbar.png (4.59 KB, patch)
2011-11-16 03:46 PST, Markus Stange [:mstange]
dao+bmo: review-
Details | Diff | Splinter Review

Description KLB 2011-11-15 02:58:48 PST
With FF8 on Mac OSX Lion, toolbar buttons do not load properly for all existing AMO themes that use the default Toolbar.png file path "chrome://browser/skin/Toolbar.png".

This is due to the fact that FF8 is remapping the path "chrome://browser/skin/Toolbar.png" to "chrome://browser/skin/lion/Toolbar.png" as a result of the following changes in browser/themes/pinstripe/browser/jar.mn (https://hg.mozilla.org/releases/mozilla-beta/annotate/e1d63abd60d5/browser/themes/pinstripe/browser/jar.mn):

mstange@74794
138 skin/classic/browser/lion/keyhole-circle.png (keyhole-circle-lion.png)
mstange@74794
139 skin/classic/browser/lion/Toolbar.png (Toolbar-lion.png)
mstange@74794
140
mstange@74794
141% override chrome://browser/skin/keyhole-circle.png chrome://browser/skin/lion/keyhole-circle.png os=Darwin osversion>=10.7
mstange@74794
142% override chrome://browser/skin/Toolbar.png chrome://browser/skin/lion/Toolbar.png os=Darwin osversion>=10.7

These changes add the following entry to nonlocalized.manifest of FF8 for Mac OSX:
override chrome://browser/skin/Toolbar.png chrome://browser/skin/lion/Toolbar.png os=Darwin osversion>=10.7

What this does is cause Lion to look in the wrong place for Toolbar.png.

A better way to handle the desire to give Lion a different Toolbar.png file would have been to use do something along the lines of what Winstripe uses for Windows Aero:
skin global classic/1.0 toolkit/skin/classic/aero/global/ os=WINNT osversion>=6

To produce this bug try the following theme:
https://addons.mozilla.org/firefox/downloads/file/130224/classic_compact-4.0.9.3-fx.jar?src=version-history

This issue breaks all existing themes on AMO that use the default file path for their toolbar buttons and thus has has the potential to impact many Mac OSX Lion users.

Thanks to Andreas Wagner who helped uncover this bug and Nils Maier who figured out what changes were causing it while we were at MozCamp EU.
Comment 1 KLB 2011-11-15 03:13:40 PST
Until this behavior is changed, theme developers should change the name of their Toolbar.png file to something else (e.g. ccToolbar.png) and change CSS references accordingly. This will not resolve the issue for existing themes, but it is a quick workaround that will restore toolbar button icons for OSX Lion users.
Comment 2 Carsten Book [:Tomcat] 2011-11-15 06:36:03 PST
marking as new based on the report
Comment 3 Carsten Book [:Tomcat] 2011-11-15 07:05:17 PST
might have been caused by Bug 667480 ?
Comment 4 Stephen Horlander [:shorlander] 2011-11-15 07:28:53 PST
(In reply to Carsten Book [:Tomcat] from comment #3)
> might have been caused by Bug 667480 ?

Yes, that is the bug that added the override for Lion.
Comment 5 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-11-15 08:17:37 PST
Unless we want to do another respin of Firefox 8, I think this needs to be relnoted.

Nominating for tracking in Beta to see if we can land a fix there when ready.
Comment 6 KLB 2011-11-15 14:02:58 PST
One way to resolve this issue might be to use media queries for OSX Lion the way Firefox uses media queries in Windows for Aero support.
Comment 7 Dave Townsend [:mossop] 2011-11-15 18:53:58 PST
An alternative would be to put the overrides (indeed all the default theme manifest directives) into a manifest file in the default theme's directory. That's an additional file to process on startup though so perhaps not what we want. It would provide better separation between the themes though.
Comment 8 Markus Stange [:mstange] 2011-11-16 03:46:23 PST
Created attachment 574866 [details] [diff] [review]
use media queries for Toolbar.png

This uses the suggestion from comment 6. But the same problem probably exists for all the other files that are overridden (e.g. places/toolbar.png), too, doesn't it?
Comment 9 Markus Stange [:mstange] 2011-11-16 03:55:27 PST
Oh, I see: For Firefox 8, the only other override is keyhole-circle.png which probably isn't used by third party themes. And the mac-lion-theme system metric only landed for Firefox 9 (bug 679717), so if we want to use the media query for Firefox 8 we need to land bug 679717 there, too.
Comment 10 Dão Gottwald [:dao] 2011-11-16 05:01:13 PST
(In reply to Dave Townsend (:Mossop) from comment #7)
> An alternative would be to put the overrides (indeed all the default theme
> manifest directives) into a manifest file in the default theme's directory.
> That's an additional file to process on startup though so perhaps not what
> we want. It would provide better separation between the themes though.

I think we'll need this for bug 659407.

(In reply to Markus Stange from comment #8)
> This uses the suggestion from comment 6. But the same problem probably
> exists for all the other files that are overridden (e.g.
> places/toolbar.png), too, doesn't it?

Yeah, this is going to be more delicate post Firefox 8.
Comment 11 KLB 2011-11-16 05:08:37 PST
Even though keyhole-circle.png probably isn't used by third party themes, it could be and the remapping of its file path could cause a lot of confusion for theme developers if they do decide to use keyhole-circle.png.  It would probably be best to avoid remapping of files all together and use media queries for keyhole-circle.png as well.
Comment 12 Dão Gottwald [:dao] 2011-11-16 08:47:31 PST
We need a solution that scales better than that, given how many *-aero.png variants we have.
Comment 13 Dão Gottwald [:dao] 2011-11-21 12:17:46 PST
Comment on attachment 574866 [details] [diff] [review]
use media queries for Toolbar.png

So, for 8 this doesn't work and for 9 it's insufficient...
Comment 14 Alex Keybl [:akeybl] 2011-12-27 07:01:46 PST
We're seeing very little (if any) support/bug volume caused by this. We'd still consider uplifting this fix to beta if a low risk fix were found, but I don't believe we need to track.
Comment 15 :Gijs Kruitbosch 2014-05-08 05:43:57 PDT
I'm pretty sure this is WFM as of Fx 29, because we got rid of the lion-specific imagery override.

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