Last Comment Bug 659407 - Remove duplicate theme files
: Remove duplicate theme files
Status: RESOLVED FIXED
: footprint
Product: Firefox
Classification: Client Software
Component: Theme (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 11
Assigned To: Dão Gottwald [:dao]
:
Mentors:
: 696565 (view as bug list)
Depends on:
Blocks: 597678 647453 706100 706103
  Show dependency treegraph
 
Reported: 2011-05-24 12:04 PDT by Mike Hommey [:glandium]
Modified: 2012-01-27 04:23 PST (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
part 1 - Deduplicate skin files in the winstripe theme (46.00 KB, patch)
2011-05-25 04:40 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
part 2 - Deduplicate skin files in the gnomestripe theme (8.12 KB, patch)
2011-05-25 04:40 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
part 3 - Deduplicate skin files in the pinstripe theme (11.37 KB, patch)
2011-05-25 04:40 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
part 4 - Deduplicate skin files shared between gnomestripe and winstripe themes (32.90 KB, patch)
2011-05-25 04:41 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
part 1 - Deduplicate skin files in the winstripe theme (49.62 KB, patch)
2011-05-25 09:32 PDT, Mike Hommey [:glandium]
dao+bmo: review-
Details | Diff | Review
part 2 - Deduplicate skin files in the gnomestripe theme (9.73 KB, patch)
2011-05-25 09:32 PDT, Mike Hommey [:glandium]
dao+bmo: review-
Details | Diff | Review
part 3 - Deduplicate skin files in the pinstripe theme (14.94 KB, patch)
2011-05-25 09:33 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
part 4 - Deduplicate skin files shared between gnomestripe and winstripe themes (32.96 KB, patch)
2011-05-25 09:34 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
part 5 - Replace chrome copy of files by overrides (101.68 KB, patch)
2011-05-25 09:38 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
part 3 - Deduplicate skin files in the pinstripe theme (14.45 KB, patch)
2011-06-02 22:56 PDT, Mike Hommey [:glandium]
dao+bmo: review-
Details | Diff | Review
part 4 - Deduplicate skin files shared between gnomestripe and winstripe themes (11.39 KB, patch)
2011-06-02 22:57 PDT, Mike Hommey [:glandium]
dao+bmo: review-
Details | Diff | Review
part 5 - Replace chrome copy of files by overrides (101.77 KB, patch)
2011-06-02 22:58 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Validation, part 1 - duplicate trees (498.81 KB, patch)
2011-06-02 23:00 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Validation, part 2 - mochitest (4.00 KB, patch)
2011-06-02 23:10 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
part 1 - Remove duplicate winstripe files, v2 (45.15 KB, patch)
2011-11-10 11:02 PST, Dão Gottwald [:dao]
dolske: review+
Details | Diff | Review
part 2 - Remove duplicate gnomestripe files, v2 (10.32 KB, patch)
2011-11-10 11:24 PST, Dão Gottwald [:dao]
no flags Details | Diff | Review
part 2 - Remove duplicate gnomestripe files, v2 (10.51 KB, patch)
2011-11-10 11:30 PST, Dão Gottwald [:dao]
dolske: review+
Details | Diff | Review
part 3 - Remove duplicate pinstripe files, v2 (10.77 KB, patch)
2011-11-10 11:42 PST, Dão Gottwald [:dao]
dolske: review+
Details | Diff | Review

Description Mike Hommey [:glandium] 2011-05-24 12:04:14 PDT
https://bugzilla.mozilla.org/show_bug.cgi?id=647453#c14
Comment 1 Mike Hommey [:glandium] 2011-05-25 04:40:00 PDT
Created attachment 535030 [details] [diff] [review]
part 1 - Deduplicate skin files in the winstripe theme

This removes duplicated files from the source tree. This does not affect the footprint. Further patches will.
I tested that the resulting omni.jar has the same content.
Comment 2 Mike Hommey [:glandium] 2011-05-25 04:40:19 PDT
Created attachment 535031 [details] [diff] [review]
part 2 - Deduplicate skin files in the gnomestripe theme

This removes duplicated files from the source tree. This does not affect the footprint. Further patches will.
Comment 3 Mike Hommey [:glandium] 2011-05-25 04:40:36 PDT
Created attachment 535032 [details] [diff] [review]
part 3 - Deduplicate skin files in the pinstripe theme

This removes duplicated files from the source tree. This does not affect the footprint. Further patches will.
Comment 4 Mike Hommey [:glandium] 2011-05-25 04:41:23 PDT
Created attachment 535033 [details] [diff] [review]
part 4 - Deduplicate skin files shared between gnomestripe and winstripe themes

This removes duplicated files from the source tree. This does not affect the footprint. Further patches will.
Note that this one is for files that are in both gnomestripe and winstripe (since gnomestrip is actually an overlay)
Comment 5 Mike Hommey [:glandium] 2011-05-25 09:32:16 PDT
Created attachment 535093 [details] [diff] [review]
part 1 - Deduplicate skin files in the winstripe theme

Refreshed
Comment 6 Mike Hommey [:glandium] 2011-05-25 09:32:39 PDT
Created attachment 535094 [details] [diff] [review]
part 2 - Deduplicate skin files in the gnomestripe theme

Refreshed
Comment 7 Mike Hommey [:glandium] 2011-05-25 09:33:05 PDT
Created attachment 535095 [details] [diff] [review]
part 3 - Deduplicate skin files in the pinstripe theme

Refreshed
Comment 8 Mike Hommey [:glandium] 2011-05-25 09:34:03 PDT
Created attachment 535097 [details] [diff] [review]
part 4 - Deduplicate skin files shared between gnomestripe and winstripe themes

Refreshed
Comment 9 Mike Hommey [:glandium] 2011-05-25 09:38:36 PDT
Created attachment 535100 [details] [diff] [review]
part 5 - Replace chrome copy of files by overrides

While previous patches are meant not to change the resulting omni.jar, this one replaces duplicate files with chrome overrides. That's a lot of them, but it's safer and faster than trying to find all places using these urls (considering they could be relative or constructed). From my local builds, it looks like this saves around 400K of omni.jar on windows builds. I still need to double check that all urls still work as before, though. I'll also most actual size differences from try builds.
Comment 10 Mike Hommey [:glandium] 2011-05-25 09:47:51 PDT
chrome/browser/content/browser/places/bookmarkProperties.xul is also duplicated as chrome/browser/content/browser/places/bookmarkProperties2.xul, because of bug 380232.

There are also these:
res/table-remove-row-active.gif res/table-remove-column-active.gif
res/table-remove-row.gif res/table-remove-column.gif
res/table-remove-row-hover.gif res/table-remove-column-hover.gif

But they require changes to places where they are used (which may also be out of the tree).

There are also more files that are duplicated across winstripe, pinstripe and gnomestripe, that could be picked from a single location from the jar.mn, but that's unrelated to the binary footprint.
Comment 11 Marco Bonardo [::mak] 2011-05-25 16:37:51 PDT
(In reply to comment #10)
> chrome/browser/content/browser/places/bookmarkProperties.xul is also
> duplicated as chrome/browser/content/browser/places/bookmarkProperties2.xul,
> because of bug 380232.

We started evaluating the removal of the duplicate, but finally was a larger change than we could do at that time (was near FF4 release). We plan to remove that though, when resources allow, you're free to file a bug against Bookmarks & History and I'll then evaluate dependencies and such, but doubt can be fixed early.
Comment 12 Justin Dolske [:Dolske] 2011-05-25 23:13:11 PDT
Couple of general comments:

* Not sure we should be pulling stuff from /toolkit in /browser's jar.mn.

* There are a few cases of different contexts currently happening to use the same icon. Should consider if we want to leave some of these duplicated for easier theme maintenance?

* Dao (or gavin?) should sign off on the chrome override approach. Dunno if that was explicitly considered/rejected when the Windows Aero stuff was originally done. At the very least it might be nice to see if we can add some preprocessing/manifest-fu to make the syntax nicer.
Comment 13 Mike Hommey [:glandium] 2011-05-25 23:35:07 PDT
(In reply to comment #12)
> Couple of general comments:
> 
> * Not sure we should be pulling stuff from /toolkit in /browser's jar.mn.

Note this goes away in part 5.
Comment 14 Mike Hommey [:glandium] 2011-05-26 09:39:41 PDT
So, patches up to part 4 are working properly on windows and mac, but miss some files on linux, so I'll have to work that out.
Part 5 has an unexpected carriage return in one file that breaks mac build, but other than that looks to be working ok.

As for sizes:
          Before       After      Difference
Windows   5369184      4950972    418212 (7.78%)
Mac       4689823      4667164    22659 (0.48%)

Definitively a massive win on Windows. I still need to validate that no chrome:// url was harmed in the process.
Comment 15 Dão Gottwald [:dao] 2011-05-26 10:00:51 PDT
(In reply to comment #4)
> Created attachment 535033 [details] [diff] [review] [review]
> part 4 - Deduplicate skin files shared between gnomestripe and winstripe
> themes
> 
> This removes duplicated files from the source tree. This does not affect the
> footprint. Further patches will.
> Note that this one is for files that are in both gnomestripe and winstripe
> (since gnomestrip is actually an overlay)

The browser part of gnomestripe isn't an overlay.
Comment 16 Mike Hommey [:glandium] 2011-06-02 22:56:46 PDT
Created attachment 537085 [details] [diff] [review]
part 3 - Deduplicate skin files in the pinstripe theme
Comment 17 Mike Hommey [:glandium] 2011-06-02 22:57:31 PDT
Created attachment 537086 [details] [diff] [review]
part 4 - Deduplicate skin files shared between gnomestripe and winstripe themes
Comment 18 Mike Hommey [:glandium] 2011-06-02 22:58:06 PDT
Created attachment 537087 [details] [diff] [review]
part 5 - Replace chrome copy of files by overrides
Comment 19 Mike Hommey [:glandium] 2011-06-02 23:00:50 PDT
Created attachment 537090 [details] [diff] [review]
Validation, part 1 - duplicate trees

This is the first part of what I used to validate nothing breaks(*) with the 5 parts applied.

* at least, that the chrome urls have the same content as before.
Comment 20 Mike Hommey [:glandium] 2011-06-02 23:10:27 PDT
Created attachment 537091 [details] [diff] [review]
Validation, part 2 - mochitest

This is second part, containing a mochitest that checks whether all chrome:// urls with the duplicated tree from validation part 1 match the corresponding chrome:// urls with the 5 patches applied.

Note validation part 1 needs to be applied before the 5 patches. This one can be applied any time.

This is a try build with an earlier iteration of the patch that has one error on linux, one on mac, and none on windows:
http://tbpl.mozilla.org/?tree=Try&rev=1e45f098a4d9

This is the try build I'm currently running with the patch queue I just sent, which should not fail on mac, and still have the error on linux, but it's actually not important.
http://tbpl.mozilla.org/?tree=Try&rev=596af34a60d5

The reason why that linux error is not important is that the failing url, chrome://global-orig/skin/console/bullet-warning.png, is defined as an override in winstripe and is inherited from there, but the url is actually not used in the gnomestripe theme so we don't care it's different from what it used to be. The reason it's different from the original case is that in the original case, we were copying icons/warning-16.png, and with this patch queue, we just override console/bullet-warning.png to be icons/warning-16.png. But, icons/warnings-16.png is overridden in browser/themes/gnomestripe/browser/jar.mn (which it really shouldn't, btw, that should be done in toolkit) to be the corresponding moz-icon:// gtk native warning icon. The console uses the gtk native warning icon urls directly in the gnomestripe stylesheet anyways.
Comment 21 Mike Hommey [:glandium] 2011-06-02 23:11:28 PDT
I will file separate bugs for the remaining (non theme) duplicate files.
Comment 22 Dão Gottwald [:dao] 2011-06-02 23:39:31 PDT
Comment on attachment 537085 [details] [diff] [review]
part 3 - Deduplicate skin files in the pinstripe theme

>-  skin/classic/browser/places/tag.png                       (places/tag.png)
>+  skin/classic/browser/places/tag.png                       (../../../../toolkit/themes/pinstripe/mozapps/places/tagContainerIcon.png)

Please remove tag.png and use chrome://mozapps/skin/places/tagContainerIcon.png instead of chrome://browser/skin/places/tag.png.

>-  skin/classic/global/dirListing/local.png                           (dirListing/local.png)
>+  skin/classic/global/dirListing/local.png                           (dirListing/folder.png)

Note that local.png actually isn't being used...

>   skin/classic/mozapps/extensions/category-languages.png          (extensions/category-languages.png)
>   skin/classic/mozapps/extensions/category-searchengines.png      (extensions/category-searchengines.png)
>   skin/classic/mozapps/extensions/category-extensions.png         (extensions/category-extensions.png)
>   skin/classic/mozapps/extensions/category-themes.png             (extensions/category-themes.png)
>   skin/classic/mozapps/extensions/category-plugins.png            (extensions/category-plugins.png)
>   skin/classic/mozapps/extensions/category-recent.png             (extensions/category-recent.png)
>   skin/classic/mozapps/extensions/category-available.png          (extensions/category-available.png)
>   skin/classic/mozapps/extensions/discover-logo.png               (extensions/discover-logo.png)
>-  skin/classic/mozapps/extensions/extensionGeneric.png            (extensions/extensionGeneric.png)
>+  skin/classic/mozapps/extensions/extensionGeneric.png            (extensions/category-extensions.png)
>   skin/classic/mozapps/extensions/extensionGeneric-16.png         (extensions/extensionGeneric-16.png)
>-  skin/classic/mozapps/extensions/themeGeneric.png                (extensions/themeGeneric.png)
>+  skin/classic/mozapps/extensions/themeGeneric.png                (extensions/category-themes.png)
>   skin/classic/mozapps/extensions/themeGeneric-16.png             (extensions/themeGeneric-16.png)
>-  skin/classic/mozapps/extensions/localeGeneric.png               (extensions/localeGeneric.png)
>+  skin/classic/mozapps/extensions/localeGeneric.png               (extensions/category-languages.png)

Please do the opposite, e.g. let category-extensions.png copy extensionGeneric.png.

>-  skin/classic/mozapps/extensions/background-texture.png          (extensions/background-texture.png)
>+  skin/classic/mozapps/extensions/background-texture.png          (../global/icons/tabprompts-bgtexture.png)

Not cool.

>-  skin/classic/mozapps/update/buttons.png                         (update/buttons.png)
>+  skin/classic/mozapps/update/buttons.png                         (downloads/buttons.png)

Also not cool. There's no strong link between these files, they won't necessarily stay in sync in the future.

>-  skin/classic/mozapps/places/defaultFavicon.png                  (places/defaultFavicon.png)
>+  skin/classic/mozapps/places/defaultFavicon.png                  (../global/tree/item.png)

We should remove item.png and use chrome://mozapps/skin/places/defaultFavicon.png instead.
Comment 23 Dão Gottwald [:dao] 2011-06-02 23:41:58 PDT
Comment on attachment 535094 [details] [diff] [review]
part 2 - Deduplicate skin files in the gnomestripe theme

a bunch of the pinstripe comments apply here as well
Comment 24 Dão Gottwald [:dao] 2011-06-02 23:54:26 PDT
Comment on attachment 535093 [details] [diff] [review]
part 1 - Deduplicate skin files in the winstripe theme

>-        skin/classic/aero/browser/preferences/plugin.png             (preferences/plugin-aero.png)
>+        skin/classic/aero/browser/preferences/plugin.png             (../../../../toolkit/themes/winstripe/mozapps/plugins/pluginGeneric-16-aero.png)

This appears to be unused.

>-        skin/classic/global/console/bullet-error.png             (console/bullet-error.png)
>-        skin/classic/global/console/bullet-question.png          (console/bullet-question.png)
>-        skin/classic/global/console/bullet-warning.png           (console/bullet-warning.png)
>+        skin/classic/global/console/bullet-error.png             (icons/error-16.png)
>+        skin/classic/global/console/bullet-question.png          (icons/information-16.png)
>+        skin/classic/global/console/bullet-warning.png           (icons/warning-16.png)

We can remove these in favor of chrome://global/skin/icons/error-16.png etc.

>-        skin/classic/global/icons/notfound.png                   (icons/notfound.png)
>+        skin/classic/global/icons/notfound.png                   (icons/information-16.png)

We can remove this in favor of chrome://global/skin/icons/information-16.png
Comment 25 Dão Gottwald [:dao] 2011-06-03 00:00:37 PDT
Comment on attachment 537086 [details] [diff] [review]
part 4 - Deduplicate skin files shared between gnomestripe and winstripe themes

>-+ skin/classic/mozapps/extensions/category-search.png      (extensions/category-search.png)

This is actually a tango icon that winstripe happens to steal, therefore it should remain in gnomestripe.
Comment 26 Mike Hommey [:glandium] 2011-06-03 02:48:30 PDT
(In reply to comment #22)
> Comment on attachment 537085 [details] [diff] [review] [review]
> part 3 - Deduplicate skin files in the pinstripe theme
> >-  skin/classic/mozapps/extensions/background-texture.png          (extensions/background-texture.png)
> >+  skin/classic/mozapps/extensions/background-texture.png          (../global/icons/tabprompts-bgtexture.png)
> 
> Not cool.

What is not cool ? to use tabprompts-bgtexture.png or to use something from ../global ?
Comment 27 Dão Gottwald [:dao] 2011-06-03 03:02:34 PDT
Using tabprompts-bgtexture.png. The idea that touching tabprompts-bgtexture.png would affect the add-on manager seems weird.
Comment 28 Mike Hommey [:glandium] 2011-06-03 03:11:55 PDT
(In reply to comment #27)
> Using tabprompts-bgtexture.png. The idea that touching
> tabprompts-bgtexture.png would affect the add-on manager seems weird.

Well, that could be said of many of the changes from this bug... but then, we could also expect people modifying or reviewing a modification to the main theme to be careful about the consequences of their changes (which is also true now, since some non duplicated images in the tree are copied several times in chrome, so modifying the only file will lead to all these to be modified)
Comment 29 Dão Gottwald [:dao] 2011-06-03 04:18:39 PDT
> Well, that could be said of many of the changes from this bug

To varying extents. extensions/background-texture.png is just background noise, whereas tabprompts-bgtexture.png overlays web content (together with a color overlay). This is quite a different context. Changing one has no consequence whatsoever for the other.
Comment 30 Dão Gottwald [:dao] 2011-06-04 05:04:05 PDT
Comment on attachment 537087 [details] [diff] [review]
part 5 - Replace chrome copy of files by overrides

Needs updates for the requested changes on the other patches.

>+% override chrome://browser/skin/feeds/audioFeedIcon.png chrome://browser/skin/feeds/feedIcon.png
>+% override chrome://browser/skin/feeds/audioFeedIcon16.png chrome://browser/skin/feeds/feedIcon16.png
>+% override chrome://browser/skin/feeds/videoFeedIcon.png chrome://browser/skin/feeds/feedIcon.png
>+% override chrome://browser/skin/feeds/videoFeedIcon16.png chrome://browser/skin/feeds/feedIcon16.png

Could you add spaces so that the overriding sources line up in the same column? Exceptions for overly long file names are fine.
Comment 31 Mike Hommey [:glandium] 2011-06-28 04:48:52 PDT
(In reply to comment #29)
> > Well, that could be said of many of the changes from this bug
> 
> To varying extents. extensions/background-texture.png is just background
> noise, whereas tabprompts-bgtexture.png overlays web content (together with
> a color overlay). This is quite a different context. Changing one has no
> consequence whatsoever for the other.

How about completely changing the file name so that it is explicit that it's tied to neither the addon manager nor tabs? I'm insisting on deduplicating this file because since bug 653143, we output duplicates from omni.jar during build. We don't error-out because we have so many duplicates right now but I guess we should in the future. And even if we don't, we still don't want a few known files to add noise.
Comment 32 Dão Gottwald [:dao] 2011-07-21 03:57:07 PDT
Warning is fine, but I don't think we want two images that happen to have the same pixel data to be an error. Given this motivation that I don't share, I'd prefer if you left these two images alone.
Comment 33 Dão Gottwald [:dao] 2011-10-22 07:20:23 PDT
*** Bug 696565 has been marked as a duplicate of this bug. ***
Comment 34 Jeremy Morton 2011-10-23 02:21:22 PDT
This bug seems to be taking about removing *some* files from the aero/ directories, but can't we go further than this?  As I said in bug #696565, we should be able to use CSS selectors for everything we're using separate Aero files for now (@media all and (-moz-windows-theme: aero) {...}), and no aero/ directories need to exist; Aero stuff can all be in the main theme files.  This would reduce all the unnecessary duplicate CSS files there, and make life easier for theme developers (checking changes to Aero files as well as non-Aero files is more work).
Comment 35 Dão Gottwald [:dao] 2011-11-10 11:02:25 PST
Created attachment 573572 [details] [diff] [review]
part 1 - Remove duplicate winstripe files, v2
Comment 36 Dão Gottwald [:dao] 2011-11-10 11:24:22 PST
Created attachment 573577 [details] [diff] [review]
part 2 - Remove duplicate gnomestripe files, v2
Comment 37 Dão Gottwald [:dao] 2011-11-10 11:30:58 PST
Created attachment 573578 [details] [diff] [review]
part 2 - Remove duplicate gnomestripe files, v2

forgot to hg rm tag.png
Comment 38 Dão Gottwald [:dao] 2011-11-10 11:42:51 PST
Created attachment 573583 [details] [diff] [review]
part 3 - Remove duplicate pinstripe files, v2
Comment 39 Marco Bonardo [::mak] 2011-11-28 08:45:51 PST
Hm, I honestly don't understand why tagContainerIcon.png exists in mozapps at all, it's not even used in toolkit code... I'd prefer us doing the opposite (keep tag.png in browser and kill the useless entry in mozapps). Do you think that'd break anything?
Comment 40 Marco Bonardo [::mak] 2011-11-28 08:49:15 PST
Ah, I found why it's there, the original Places toolkit code was using it, but that code has been removed long long time ago, still the icon is still in the wrong place, can be removed (and switch to using the browser's theme one)
Comment 41 Justin Dolske [:Dolske] 2011-11-28 16:21:56 PST
Comment on attachment 573572 [details] [diff] [review]
part 1 - Remove duplicate winstripe files, v2

Review of attachment 573572 [details] [diff] [review]:
-----------------------------------------------------------------

I didn't go through and verify correctness of all the changes, I'll rely on Dao's usual thoroughness for that. :) Reflag for review if you want a second set of nitpicky eyes on things, though.

All the change here make sense to me, though.
Comment 42 Marco Bonardo [::mak] 2011-11-28 16:37:52 PST
Just please invert the tag.png <-> tagContainerIcon.png replacement, we should keep the former and remove the latter. Otherwise we'll have to do the same in a follow-up inverting part of this patch, that would make things a bit harder to follow.
Comment 43 Justin Dolske [:Dolske] 2011-11-28 17:24:18 PST
Yeah, I assumed that would be sorted out one way or the other. :-)

Oh, and for the other work in this bug from Glandium... If there are parts that make sense to keep, can we spin off other bugs for that? I don't want to lose that work if so, but I also don't want to have this bug drag on further.
Comment 44 Dão Gottwald [:dao] 2011-11-29 07:16:41 PST
I added tag.png back and removed tagContainerIcon.png instead.

https://hg.mozilla.org/mozilla-central/rev/68ba25ded632
https://hg.mozilla.org/mozilla-central/rev/1bb37562781c
https://hg.mozilla.org/mozilla-central/rev/45311e361fd1

Filed bug 706100 and bug 706103 for the missing parts.
Comment 45 Pardal Freudenthal (:ShareBird) 2012-01-27 01:14:37 PST
Oh please!! We can't override files that are inside the skin provider! We don't know nothing about it contents, since the skin provider is fully replaced according to our theme system. Overriding files that are "supposed" to be inside the skin provider can cause every kind of problems and brake badly third-party themes.
Comment 46 Dão Gottwald [:dao] 2012-01-27 02:58:28 PST
(In reply to Pardal Freudenthal (:ShareBird) from comment #45)
You're commenting in the wrong bug.
Comment 47 KLB 2012-01-27 04:23:05 PST
While I don't see overrides in attachments parts 1, 2 & 3, which have 'review +' flags, but there other attachments with no flags that do have overrides. Hopefully these attachments will not be approved.

The practice of trying to "fix" things using overrides in the skin provider is very troubling and should be prohibited. As we found with the OS X Lion bugs, skin overrides can and do break lots of AMO themes.

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