702558 - FF8 breaks themes using default Toolbar.png file path in Mac OSX Lion (toolbar buttons not loading)

Status: RESOLVED WORKSFORME

(Firefox :: Theme, defect)

Description

[KLB]

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]

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]]

marking as new based on the report

Comment 3

[Carsten Book [:Tomcat]]

might have been caused by Bug 667480 ?

Comment 4

[Stephen Horlander [:shorlander]]

(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

[u279076]

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]

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]]

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]]

Attachment: 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]]

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]]

(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]

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]]

We need a solution that scales better than that, given how many *-aero.png variants we have.

Comment 13

[Dão Gottwald [:dao]]

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]]

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 (he/him)]

I'm pretty sure this is WFM as of Fx 29, because we got rid of the lion-specific imagery override.