1202331 - FxAccounts badge wrong size on nightly

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

Attachment: fxa-hamburger.png

Bug 1188001 seems to have caused the same problem reported in bug 1198424, but for the FxA badge - see attachment.

Comment 1

[Edouard Oger [:eoger]]

Attachment: bug-1202331.patch

Height is 14px instead of 13px because we can see the image starting to repeat on the right otherwise.

Comment 2

[Matteo Ferretti [:zer0] [:matteo]]

Comment on attachment 8658237 [details] [diff] [review]
bug-1202331.patch

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

::: browser/themes/shared/customizableui/panelUIOverlay.inc.css
@@ +123,5 @@
>    background-color: transparent;
>    background-image: url(chrome://browser/skin/warning.svg);
>    box-shadow: none;
>    filter: drop-shadow(0 1px 0 hsla(206, 50%, 10%, .15));
> +  height: 14px;

> Height is 14px instead of 13px because we can see the image starting to repeat on the right otherwise.

Because this badge is an image – and is not using the regular badge's square – I'm not too picky about the different `height` value, I think it should use what it makes the badge looks better; however the repetition of the image shouldn't be the reason why we choose one value instead of another; we should be sure that doesn't matter what size we have, now or in future, we don't get the repetition issue anymore. So I will suggest to follow the same approach we got here: https://bug1198424.bmoattachments.org/attachment.cgi?id=8655415 and basically set `no-repeat` and `center` as background's image properties.

Comment 3

[Edouard Oger [:eoger]]

Attachment: bug-1202331.patch

Thank you Matteo, I went back to the 13px height for consistency and used the no-repeat center approach instead.

Comment 4

[Dão Gottwald [:dao]]

Why do we need to specify a height at all here?

Comment 5

[Matteo Ferretti [:zer0] [:matteo]]

Comment on attachment 8658264 [details] [diff] [review]
bug-1202331.patch

Thanks!

Comment 6

[Matteo Ferretti [:zer0] [:matteo]]

(In reply to Dão Gottwald [:dao] from comment #4)
> Why do we need to specify a height at all here?

To have a more consistent badge's size; otherwise it will be smaller than expected, due the SVG image. However, 'cause here we use image without the regular badge's box, we could also have a smaller size if it looks better to UX.

Comment 7

[Mark Hammond [:markh] [:mhammond]]

I think it gets closed once it makes its way to central...

Comment 8

[Matteo Ferretti [:zer0] [:matteo]]

Comment on attachment 8658264 [details] [diff] [review]
bug-1202331.patch

Approval Request Comment
[Feature/regressing bug #]: 1188001
[User impact if declined]: Visible UI regression, the badge's image will be repeated – see the attachment
[Describe test coverage new/current, TreeHerder]: No automated testing possible due to the visual nature of the bug, thus requires extensive QA on all platforms.
[Risks and why]: users will see repeating icon on the badge every time fxa badge status is set
[String/UUID change made/needed]: None

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/bec9ac8784ed

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8658264 [details] [diff] [review]
bug-1202331.patch

Matteo, this "[Risks and why]:" means "what kind of risk does it bring to the release.

Anyway, taking it.

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/28c5804defb1

Comment 13

[alex_mayorga]

¡Hola!

This looks OK in Mozilla/5.0 (Windows NT 6.1; WOW64; rv:43.0) Gecko/20100101 Firefox/43.0 ID:20150910030225 CSet: dd2a1d737a64d9a3f23714ec5cc623ec8933b51f

¡Gracias Edouard!

Comment 14

[Ada [:adalucinet]]

Reproduced with Nightly from 2015-09-08.
Verified fixed with Firefox 43 beta 4 (Build ID: 20151116155110), across platforms [1].

[1] Windows 7 64-bit, Ubuntu 12.04 32-bit and Mac OS X 10.8.5