421595 - CSS and packaging changes for APNG throbber

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Justin Dolske [:Dolske]]

Attachment: APNG version of the FF2 throbber

Bug 326817 covers the design of a new APNG throbber. Before such a throbber can be used, there needs to be some CSS and packaging changes done to point at "foo.png" instead of "foo.gif" for the throbber image. We should go ahead and do these changes now (with an APNG throbber identical to the existing throbber), so that a redesigned throbber can be dropped in late in the cycle without causing a bunch of code churn.

This is mostly just doing a s/gif/png/. Pinstripe doesn't need modified, because it's already using an APNG throbber (loading_16.png). However, since it's filename is different, I'm also doing to modify the throbber filename in the other themes so that they're all consistent.

So: Throbber-small.gif ---> loading_16.png everywhere.

Throbber-small.png (a confusingly named non-animated image, for an inactive throbber) will become notloading_16.png.

Comment 1

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

First pass. Untested.

Nits/notes:

* Code in other projects (mailnews) needs updated. I don't have a tree handy when I tossed this together.

* Breakpad seems to use the GIF in the native client, and won't be converted
  to APNG.
   - toolkit/crashreporter/client/crashreporter_linux.cpp
   - toolkit/crashreporter/client/Makefile.in
   - browser/installer/unix/packages-static

* Winstripe and Gnomestripe refer to Throbber.gif and Throbber.png. These 20x20 throbbers don't seem to be used anywhere, and can be removed.

* Hmm, why do these seem to package everything except Throbber-small.gif?
  - browser/themes/winstripe/browser/jar.mn
  - browser/themes/gnomestripe/browser/jar.mn

Comment 2

[Justin Dolske [:Dolske]]

Untargeting, if we want to use an APNG throbber on Windows/Linux for  FF3, we should look at a lower-impact change... Either minimal changes, or consider naming it as ".gif" (without changing CSS) even though it's a PNG.

Comment 3

[Ian Spence]

What if we just used chrome overrides? (wrapping added for clarity)

override chrome://global/skin/throbber/Throbber-small.gif \
    chrome://global/skin/icons/loading_16.png

override chrome://global/skin/throbber/Throbber-small.png \
    chrome://global/skin/icons/notloading_16.png

override chrome://browser/skin/Throbber-small.png \
    chrome://browser/skin/notloading_16.png



Those overrides were generated from the patch posted.  I feel I must ask why the browser only gets a not-busy image and the toolkit gets both. Seems to me it'd probably be better to replace that last rule with

override chrome://browser/skin/Throbber-small.png \
    chrome://global/skin/icons/notloading_16.png

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Doesn't block, but if we can get this with chrome overriddes, I definitely want it. The old throbber is embarrassing in comparison to the new one.

Comment 5

[Justin Dolske [:Dolske]]

Attachment: Patch v.2

Lighter-weight version of patch v.1...

My original thinking (v.1) was to completely eliminate usage of the old throbbers throughout the tree, such that the source images could be cvs removed. That ends up touching a lot of extra files, and removing the files causes the problems for anything that gets missed (3rd parties?).

So, this patch only updates browser/themes/*/browser.css to use the new throbber URI. The old images stay, and are still used in some /toolkit stuff that isn't seen nearly as often (app/addon updates, mostly). We can fix that in another pass, or leave it as-is for FF3.

For the initial landing, this would still use the APNG throbber attached here (looks the same as the existing Windows/Linux throbber). This allows for a later throbber change without risk CSS/packaging changes.

[Haven't actually built this on Win/Lin yet, but will do so now.]

Comment 6

[Justin Dolske [:Dolske]]

(apparently bugzilla's diff viewer is a bit retarded, and gets confused by the toolkit gnomestripe changes, see raw diff for changes there)

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 314479 [details] [diff] [review]
Patch v.2

Smooth.

Comment 8

[Justin Dolske [:Dolske]]

Checked in. Note that the "loading_16.png" is just the APNG version of the existing throbber (attached to this bug), and "notloading_16.png" is just a copy of the existing non-animated "Throbber-small.png". The new names are hopefully a little more obvious, and also happen to match what the OS X is using.

Checking in browser/themes/gnomestripe/browser/browser.css;
  new revision: 1.207; previous revision: 1.206
Checking in browser/themes/winstripe/browser/browser.css;
  new revision: 1.196; previous revision: 1.195
Checking in toolkit/themes/gnomestripe/global/jar.mn;
  new revision: 1.37; previous revision: 1.36
Checking in toolkit/themes/gnomestripe/global/icons/loading_16.png;
  initial revision: 1.1
Checking in toolkit/themes/gnomestripe/global/icons/notloading_16.png;
  initial revision: 1.1
Checking in toolkit/themes/winstripe/global/jar.mn;
  new revision: 1.48; previous revision: 1.47
Checking in toolkit/themes/winstripe/global/icons/loading_16-aero.png;
  initial revision: 1.1
Checking in toolkit/themes/winstripe/global/icons/loading_16.png;
  initial revision: 1.1
Checking in toolkit/themes/winstripe/global/icons/notloading_16-aero.png;
  initial revision: 1.1
Checking in toolkit/themes/winstripe/global/icons/notloading_16.png;
  initial revision: 1.1