Happens if downloads button is in the tab bar, or if tabs are on bottom. The color is white when customizing but changes to black once it's done.

Tested with the following cases: * Button in nav-bar * Button in tab-strip * Button in custom toolbar I tested these cases with no downloads in progress, some downloads in progress, and downloads paused. Tested this on Windows 7 and Windows XP. We don't appear to invert any icons in the tabstrip on Windows XP, so I figure it's OK if we keep that behaviour with the downloads button.

unfortunately we can't fix win8 due to bug 816803.

Comment on attachment 705450 [details] [diff] [review] Patch v1.0 Review of attachment 705450 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/winstripe/downloads/downloads-aero.css @@ +37,5 @@ > text-shadow: 0 0 1px rgba(0,0,0,.7), > 0 1px 1.5px rgba(0,0,0,.5); > } > + > + #toolbar-menubar #downloads-indicator:not([counter]) > #downloads-indicator-anchor > #downloads-indicator-progress-area > #downloads-indicator-counter, These rules look pretty scary, but they're basically a copy of the the rules on line 25-29, with the added provision that we drill down to the #downloads-indicator-counter when #downloads-indicator:not([counter]).

Comment on attachment 705450 [details] [diff] [review] Patch v1.0 Review of attachment 705450 [details] [diff] [review]: ----------------------------------------------------------------- The first part is OK, the second bunch of added rules is instead useless, due to this rule: http://mxr.mozilla.org/mozilla-central/source/browser/components/downloads/content/downloads.css#85 See also bug 812487. On a side note, if you want to remove those pointless rules here I'd be fine with that. r=me with all that scary rules removed from the patch :)

so, what we have to override is #downloads-indicator:not([counter]) > #downloads-indicator-anchor > #downloads-indicator-progress-area > #downloads-indicator-counter { background: -moz-image-rect(url("chrome://browser/skin/Toolbar.png"), 0, 108, 18, 90) center no-repeat; background-size: 12px; } though if not([counter]), not([progress]) and not([paused)] this area is hidden so it's left [progress] and [paused] that is what actually the above rule is supposed to cover. If we want to keep that simplification of just specifying not([counter]); it may be fine, though it's quite easy to misinterpret it like in bug 812487 that is indeed invalid we should either add a comment or explicit it to -moz-any([progress], [paused])

I've added some commentary, and I think we only really care about the [paused] state for the download indicator, since this is the only state where we show an icon with the progress bar. Does this clear it up a little?

Comment on attachment 705960 [details] [diff] [review] Patch v1.1 Review of attachment 705960 [details] [diff] [review]: ----------------------------------------------------------------- So, just [paused] is wrong, cause if a download has unknown size (may happen with some servers) we have [progress] but we don't have nor [counter] nor [paused].. Let's revert to your first patch here but: 1. add the nice comments you added here 2. move these rules up with the others, since they are the same 3. add a comment that :not([counter]) is used as a shurtcut for :-moz-any([progress], [paused]) 4. add the same comment about not-counter to any rule out of this list (http://mxr.mozilla.org/mozilla-central/search?string=downloads-indicator%3Anot%28[counter]%29&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central) that is relative to #download-indicator-progress-area

Done and done.

Comment on attachment 706389 [details] [diff] [review] Patch v1.2 Review of attachment 706389 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/themes/gnomestripe/downloads/downloads.css @@ +238,5 @@ > background-image: url("chrome://browser/skin/downloads/download-glow.png"); > } > > +/* In the next few rules, we use :not([counter]) as a shortcut that is > + equivalent to -moz-any([progress], [paused]). */ nit: would be good to use the same identical comment across all 3 themes :) ::: browser/themes/winstripe/downloads/downloads-aero.css @@ +31,5 @@ > + #nav-bar + #customToolbars + #PersonalToolbar[collapsed=true] + #TabsToolbar[tabsontop=false]:last-child #downloads-indicator-icon:not(:-moz-lwtheme), > + > + /* The following rules are for the downloads indicator when in its paused > + state. We use :not([counter]) as a shortcut for > + :-moz-any([progress], [paused]). */ nit: "in its paused or undetermined progress state."

Thanks mak! Done!

Landed on mozilla-inbound as https://hg.mozilla.org/integration/mozilla-inbound/rev/7b9583f2c837

Comment on attachment 706587 [details] [diff] [review] Patch v1.3 (r+'d by mak) [Approval Request Comment] Bug caused by (feature/regressing bug #): downloads panel feature User impact if declined: the button on glass is barely visible Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): minor polish, css-only change, limited to the feature String or UUID changes made by this patch: none

I confirm the fix is verified on Windows 7 x64 using FF 21b6 20130430204233

Verified as fixed on FF 20 Release: 20130409194949 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0