872455 - icon in social toolbar button is mis-sized on retina display

Status: VERIFIED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Jonathan Kew [:jfkthame]]

Attachment: screenshot showing Social toolbar button on Retina Mac

The problem is that http://mxr.mozilla.org/mozilla-central/source/browser/themes/osx/browser.css#3709 loads a "@2x" version of the icon (i.e. an image with 32x32 actual pixels) on a Retina display, but there's nothing in the style to constrain the size to 16x16 CSS pixels. So it gets displayed with its 32 pixels scaled to 32 CSS px, instead of providing "double resolution" in a 16 CSS px area.

Comment 1

[Lukas Blakk [:lsblakk] use ?needinfo]

Not clear why we should track this - it's not a regression and retina macs have been available for a while now.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

It is a regression, from bug 858321.

Comment 3

[:Gijs (he/him)]

Attachment: Patch

Something like this?

I removed the -moz-image-rect because the images only contain this icon, so it's unnecessary. I can add it back if that's desired, and add one for the 2x icon. Generally, as we're shipping this now, I'm not sure why this isn't in Toolbar.png?

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Yeah, I agree that this should be in Toolbar.png.

Comment 5

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

Comment on attachment 752136 [details] [diff] [review]
Patch

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

LGTM, but I've no retina mac to test on - so r+ under the assumption you have tested it there.  Would you mind also opening a bug to have the image added to Toolbar.png?

Comment 6

[:Gijs (he/him)]

Pushed to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/c6c467df9154

Filed bug 874773 for the Toolbar.png inclusion.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

In that case, tracking and looking for uplift nom.

Comment 9

[:Gijs (he/him)]

Comment on attachment 752136 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 858321
User impact if declined: Users who enable and then disable a social provider will see an ugly (cut-off) icon in their toolbar.
Testing completed (on m-c, etc.): Tested on m-c, looks much better there
Risk to taking this patch (and alternatives if risky): zero to none, it's just CSS changes pertaining only to this button.
String or IDL/UUID changes made by this patch: none

Comment 10

[:Gijs (he/him)]

Apologies for the delay; pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/9d3e8e88a722 .

Comment 11

[u279076]

Matt, can you please test this to verify it's fixed in Firefox 23?

Comment 12

[Matt Wobensmith [:mwobensmith][:matt:]]

Attachment: Social toolbar button in 23b4

Comment 13

[Matt Wobensmith [:mwobensmith][:matt:]]

Attachment: Social toolbar button in 23b4 - inactive

See attachments. 

Using Mac Retina:

On FF23 2013-05-15, the button is blank.

On FF23b4 2013-07-09, the button is blank when not engaged, and has a different image when toggled. Neither of these match the screenshot attached to this bug.

More info welcomed, thank you.

Comment 14

[Shane Caraveo (:mixedpuppy)]

(In reply to Matt Wobensmith from comment #13)
> Created attachment 774237 [details]
> Social toolbar button in 23b4 - inactive
> 
> See attachments. 
> 
> Using Mac Retina:
> 
> On FF23 2013-05-15, the button is blank.
> 
> On FF23b4 2013-07-09, the button is blank when not engaged, and has a
> different image when toggled. Neither of these match the screenshot attached
> to this bug.
> 
> More info welcomed, thank you.

I think you're looking at the share button, which is not what this bug addressed.  The share button still needs retina icons and we have bug 886816 for that.  The button in question is the facebook icon you are looking at, however you need to turn off social from the menu in that button to see the cloud icon in the original image showing the problem.

Comment 15

[Matt Wobensmith [:mwobensmith][:matt:]]

Thank you Shane, that was the information that I needed.

Confirmed bug FF23 2013-05-15.
Confirmed fixed FF23b4 2013-07-09.
Confirmed fixed FF24 2013-07-09.