831432 - Downloads button has no text in Text button mode

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, defect)

Comment 1

[Marco Bonardo [:mak]]

not sure if it's worth spending time on this considered bug 573329.

Comment 2

[Marco Bonardo [:mak]]

I wonder if just adding the label would fix this without causing regressions in the anchor

Comment 3

[Marco Bonardo [:mak]]

Attachment: patch v1.0

this is the less invasive patch I may think of.
The only defect is that the arrow still points the icon, not the text, in full mode. Though fixing that would be more invasive and regression-prone.

I tested this only on Windows and afaict seems to work fine.

Comment 4

[Mike Conley (:mconley) (:⚙️) (PTO Dec 22 - Jan 2)]

Comment on attachment 703053 [details] [diff] [review]
patch v1.0

Your approach is fine as a stopgap until we remove those two other modes.

Seems to work fine in Ubuntu. Works well in OSX too - my only slight complaint is that when we're in full mode, the text label is misaligned with the labels of the other buttons.

Comment 5

[Marco Bonardo [:mak]]

(In reply to Mike Conley (:mconley) from comment #4)
> Seems to work fine in Ubuntu. Works well in OSX too - my only slight
> complaint is that when we're in full mode, the text label is misaligned with
> the labels of the other buttons.

only on OSX?

Comment 6

[Mike Conley (:mconley) (:⚙️) (PTO Dec 22 - Jan 2)]

Attachment: Label and icon on OSX

I believe so. Here's OSX.

Comment 7

[Marco Bonardo [:mak]]

ah probably my margin: 0 rule is wrong, I couldn't find where the margins were defined so that rule was too wide.

Comment 8

[Mike Conley (:mconley) (:⚙️) (PTO Dec 22 - Jan 2)]

Attachment: Label and icon on Ubuntu

And here we are on Ubuntu.  Bookmarks and Social are a bit derpy, but that's outside the scope of this bug.

Comment 9

[Marco Bonardo [:mak]]

yeah I think it's this rule
http://mxr.mozilla.org/mozilla-central/source/browser/themes/pinstripe/browser.css#520

Comment 10

[Marco Bonardo [:mak]]

Attachment: patch v1.1

The style is taken from toolbarbutton.css + browser.css

Comment 11

[Mike Conley (:mconley) (:⚙️) (PTO Dec 22 - Jan 2)]

Comment on attachment 706772 [details] [diff] [review]
patch v1.1

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

This does the job - tested on all three platforms. Code looks good too. Ship it!

Comment 12

[Marco Bonardo [:mak]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4be6e1db784

Comment 13

[Marco Bonardo [:mak]]

Comment on attachment 706772 [details] [diff] [review]
patch v1.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): downloads panel feature
User impact if declined: text and full toolbar modes won't work properly
Testing completed (on m-c, etc.): m-i, local
Risk to taking this patch (and alternatives if risky): limited to the feature and those 2 modes, css-only changes
String or UUID changes made by this patch: none

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b4be6e1db784

Comment 15

[Marco Bonardo [:mak]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2b43414e36c1

Comment 16

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20130213 Firefox/21.0 (20130213031137)
Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20130212 Firefox/20.0 (20130212042017)

Verified as fixed on the latest Nightly and Aurora - in text mode, the downloads button is properly displayed (with text).

Verified on Windows 7, Ubuntu 12.10 and Mac OS X 10.7.