528834 - Home button lacks styling for the disabled state, breaking compatibility with the UsableHomeButton extension

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Marat Tanalin]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; ru-RU; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; ru-RU; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 (.NET CLR 3.5.30729)

Icons for disabled state of home button (chrome element id is 'home-button') are missing as of Firefox 3.6 (including current nightlies—Gecko/20091115 Namoroka/3.6b3pre and Gecko/20091115 Minefield/3.7a1pre). Firefox 3.5 (current stable branch) *has* this icons.

Reproducible: Always

Steps to Reproduce:
1. Go to Firefox (Windows version) installation directory.
2. Open "chrome\classic.jar\skin\classic\browser\Toolbar.png" or "chrome\classic.jar\skin\aero\browser\Toolbar.png". (Toolbar-small.png is affected too.)

Or just open provided testcase URL where sprite from 3.5 is compared with one from 3.6 side by side.
Actual Results:  
In Firefox 3.5, there is icons for disabled state of home button. But as of Fx 3.6, such icons are missing (in both Toolbar.png and Toolbar-small.png).

Expected Results:  
Icons for all states of home button, *including disabled* (missing in current 3.6/3.7 nightlies), should exist.

Currently affected version of Firefox are both (3.6 and 3.7) trunk branches. Latest tested versions are Gecko/20091115 Namoroka/3.6b3pre and Gecko/20091115 Minefield/3.7a1pre.

Disabled state icons are important for extensions that use them. For example, UsableHomeButton (https://addons.mozilla.org/en-US/firefox/addon/10315) disables home button if user is already on the home page of the current site. In current Fx 3.6/3.7 builds such navigational effect is unachievable, and therefore appropriate usability aspect is broken.

Besides, disabled states icons are missing not only for home button, but for adjucent icons in the same sprite too. May be all these missing icons are subject to be added soon, but this is uncertain, so here is bug report. Thanks.

Comment 1

[Marat Tanalin]

(some typo in last paragraph: adjucent => adjacent)

Comment 2

[Marat Tanalin]

Fx 3.6 Beta 3 is out, but disabled-state icons for home button (and a few other as initially described) are still missing.

It would be nice to see some clarification from Firefox developers (especially personally from that one who removed these icons from 3.6 and/or has made such a decision).

We're dangerously close to final release. Hard to imagine this bug non-fixed. Thanks.

Comment 3

[Marat Tanalin]

The bug has been introduced in build "2009-09-22-05-mozilla-1.9.2" (Gecko/20090922 Namoroka/3.6b1pre). Build "2009-09-21-05-mozilla-1.9.2" (Gecko/20090921 Namoroka/3.6a2pre) was not affected yet.

Besides, Mac version of Fx 3.6 is NOT affected by this bug at all and, unlike Windows version, has full icon set with all disabled-state icons in place.

Comment 4

[Dão Gottwald [:dao]]

Attachment: patch

Comment 5

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Comment on attachment 416964 [details] [diff] [review]
patch

Should we not just encourage extensions to use SVG filters on the default states (e.g. http://tinyurl.com/528834), rather than adding the disabled states back one-by-one?

Comment 6

[Dão Gottwald [:dao]]

It's not that easy, since it's platform specific and icon theme specific on Linux.

I think /we/ should at some point use SVG filters, to eliminate the overhead of maintaining that stuff (bug 507940). I don't think extensions should have to deal with this, unless they're shipping custom toolbarbuttons... but even then we should by default just apply the right filters.

We can probably do bug 507940 pretty easily for Windows in the 1.9.3 timeframe, but it's too late for 1.9.2.

Comment 7

[Marat Tanalin]

(In reply to comment #5)
> (From update of attachment 416964 [details] [diff] [review])
> Should we not just encourage extensions to use SVG filters on the default
> states (e.g. http://tinyurl.com/528834), rather than adding the disabled states
> back one-by-one?

Agreed with Dão, styling of default/standard (available out of the box) elements of browser is a matter of browser implementation. API should be consistent no matter what particular way styling is implemented. Extensions developers should be able to just use (full set of) vendor-predefined states (by just setting disabled="disabled", for example) with no any need to dive into issues of cross-platform styling of *standard* toolbar elements.

Comment 8

[Ryan Flint [:rflint] (ping via IRC for reviews)]

Comment on attachment 416964 [details] [diff] [review]
patch

I'm still not totally convinced that we need to do this. The bits in browser/themes/* have never really offered API guarantees and once the user installs a non-default theme all bets are off entirely. We should probably just revert bug 510442 to avoid affecting any other extensions we're not currently aware of - but since this is effectively zero-cost, you have r=me to do whatever you deem appropriate.

Comment 9

[Dão Gottwald [:dao]]

I still think bug 510442 is basically valid, it doesn't make sense to ship and maintain stuff where we're convinced it's not needed. Even though the judgement wasn't quite right for Home, it likely holds for Downloads, New Tab etc.

Comment 10

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/f8d2c6f4631d

Comment 11

[Dão Gottwald [:dao]]

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