http://s13.postimage.org/4tl33qh03/Untitled.png This should be fixed before shipping it in Beta / Aurora

no doubt, it is quite visible so we are clearly aware of that from a long time, though we are still waiting for the final icons.

These might help ? https://people.mozilla.com/~shorlander/files/australis-toolbar-buttons/australisToolbarButtons-07.html https://people.mozilla.com/~shorlander/files/download-button/download-button.html More mockups by Stephen @ https://people.mozilla.com/~shorlander/files/

the mockups are not the icons, we need the UX team to give us the final artwork.

Mac Download button needs a new icon, too.

Shorlander, can you help? This is even awfuller on Windows.

How long would it take? :( Sorry for being impatient but it seems such a small cosmetic bug , but really annoying... Thanks

Untested on Linux or Mac, but the icon size is the same for all three platforms. In order to get the proper glow around the icon, the icon must be larger than 16x16, hence the 48x48 size of the icon. Here's a screenshot of how it looks on Windows: http://screencast.com/t/U8FTDHLOZZz

Here is another screenshot of it on Windows, showing the button with the :hover state, http://screencast.com/t/zWhPlfWhe

Comment on attachment 666276 [details] [diff] [review] Patch Review of attachment 666276 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for looking into this and writing this patch, Jared. Although it may not ideal to have the glow expand that far outside of the toolbar button, it's definitely better than the current situation. Just a few things to tweak before approving: ::: browser/themes/gnomestripe/downloads/downloads.css @@ +179,5 @@ > 0, 16, 16, 0) center no-repeat; > } > > #downloads-indicator[attention] > #downloads-indicator-anchor > #downloads-indicator-icon { > + background: url("chrome://browser/skin/downloads/download-glow.png") center no-repeat; We should investigate how to apply this fix to #downloads-indicator:not([counter])[attention] #downloads-indicator-counter in a followup bug. The flow of states is difficult to follow, and I have not seen #downloads-indicator:not([counter])[attention] actually occur. ::: browser/themes/pinstripe/downloads/downloads.css @@ +189,5 @@ > > #downloads-indicator[attention] > #downloads-indicator-icon { > + background: url("chrome://browser/skin/downloads/download-glow.png") center no-repeat; > + margin: -16px; This should probably be -14px, given the -moz-image-rect offsets that were here. ::: browser/themes/winstripe/downloads/downloads.css @@ +200,5 @@ > } > > #downloads-indicator[attention] > #downloads-indicator-anchor > #downloads-indicator-icon { > + background: url("chrome://browser/skin/downloads/download-glow.png") center no-repeat; > + margin: -16px; This should probably be -15px. ::: toolkit/themes/gnomestripe/global/alerts/alert.css @@ +66,5 @@ > } > + > +.alertBox[hide] { > + visibility > +} \ No newline at end of file Please remove this from the diff.

Thanks for the speedy review Frank. I filed bug 795806 for the :not([counter])[attention] rule. I updated the patch to have the more correct margin values. Thanks for catching that!

Patch looks good on Ubuntu.

what happens if the button is on the lowest or highest toolbar (eg bookmarks toolbar), I suppose the glow is still cut but the border of the toolbox... if so likely we should ensure the glow fits into the button size, not go over it.

Thanks for asking about that Marco. I loaded about:blank and compared and saw that the green was affecting the color of the page. I was able to lower the value on Windows to |margin: -8px -15px| and still achieve the desired effect without bleeding into content. I'll fix the patch.

Please check small icons mode too.

Thanks, we'll need new graphics from Stephen that don't have the glow around them. Small icon mode in particular makes it hard to have any glow that expands in the y-axis.

As well the download button looks kind of ugly in Linix when compared to the one in windows No matter what theme (Firefox uses internal icons for the button) Also on small buttons the download icons looks odd

The overflow on to content is fixed by setting overflow:hidden on the #navigator-toolbox. I set it on #navigator-toolbox instead of #nav-bar so that it wouldn't clip at the boundaries of other toolbars.

Comment on attachment 669875 [details] [diff] [review] Patch v2.1 This adds a scroll frame for the toolbox, which is overhead we don't want.

Unassigning as I think the only reasonable approach is to get a new icon that doesn't have the glow (unless we are OK with overlapping content, which I doubt).

An icon with a glow that stays inside the usual size would be fine too, I suppose that glow should be like 1 or 2 px (don't remember how much space there's available in the current icon)

What is wrong about overlapping content or other toolbars with the glow?

(In reply to Jared Wein [:jaws] from comment #27) > What is wrong about overlapping content or other toolbars with the glow? I think it looks weird, so we would be exchanging a weird look for another one. Plus the current glow is far too large and hurts on background themes, something thinner would be nicer globally.

The content area is for Web content. The glow just doesn't belong there. I guess overlaying content like this could also hurt performance.

We overlay content for things like doorhangers, and bug 541656, so this wouldn't be the first time.

Neither doorhanger notifications nor the status panel are comparable to this. They don't spill over to the content area, they intentionally overlay it. Their implementation is also different such that performance characteristics may differ. Popups get their own widget, the status panel has position:fixed.

Basically this needs a new icon.

I'm starting thinking we should just remove the glow completely. It's non-native, since no other button does any glowing stuff.Plus whatever color we take it will look bad on some Persona. It's definitely creating more issues than benefits, since the icon is already changing color to green, that looks like enough to show a status change without being distracting. Thus, imo, we should just change download-glow.png (And the aero version) to have no external glow, the internal one is fine. At least this is the interim solution we should take before next Aurora uplift.

Stephen agreed.

I've got new graphics from shorlander, and I've got them up on Windows 7 and XP. Screenshots coming.

Fixes OSX. I've made the required changes for Retina displays, but I haven't tested that yet. I'll see if I can scrounge one up here once I'm done with gnomestripe.

Ok, I think this covers it.

yes, we definitely need a follow-up bug for gnomestripe.

Comment on attachment 682047 [details] [diff] [review] Patch v1 Review of attachment 682047 [details] [diff] [review]: ----------------------------------------------------------------- You forgot to remove download-glow-aero.png from the winstripe/downloads folder, apart this looks fine.

(In reply to Marco Bonardo [:mak] from comment #43) > Comment on attachment 682047 [details] [diff] [review] > Patch v1 > > Review of attachment 682047 [details] [diff] [review]: > ----------------------------------------------------------------- > > You forgot to remove download-glow-aero.png from the winstripe/downloads > folder, apart this looks fine. Thanks! Fixed.

Landed in mozilla-inbound as https://hg.mozilla.org/integration/mozilla-inbound/rev/61232c5c748d