1062335 - Loop panel size increases after switching themes

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect)

Description

[Darrin Henein [:darrin]]

Attachment: Screen Shot 2014-09-03 at 10.51.02 AM.png

To reproduce on Nightly (Sept 3 2014):

1. Enter customize mode, switch your lightweight theme.
2. Open Loop panel from toolbar.

Each successive theme switch will increase the panel size until it is full width.

Comment 1

[u279076]

I reproduce this in 35.0a1, 34.0a2, and 33.0b3 so as near as I can tell this is not a regression. FWIW, you don't need to enter customize mode -- it reproduces by installing themes when visiting addons.mozilla.org as well. I'm going to suggest we wontfix this for 33 since we aren't shipping in 33 anyway, but this should get fixed for 34 and 35.

[Tracking Requested - why for this release]: Poor user perception of quality when switching themes.

Comment 2

[u279076]

FWIW, I've added a Moztrap testcase to make sure this is covered in our sign-off.

Comment 4

[Lawrence Mandel [:lmandel] (use needinfo)]

Maire - Is this bug already on your list? Can you please get someone assigned to address this?

Comment 5

[Maire Reavy [:mreavy]]

Hi Jared -- Can you pick up this bug (as part of the 36.1 sprint)?  Or should I look for someone else? Thanks.

Comment 6

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

In Social.jsm's sizeSocialPanelToContent,
>  // add extra space the panel needs if any
>  width += panel.boxObject.width - iframe.boxObject.width;
>  height += panel.boxObject.height - iframe.boxObject.height;
there is the above code. Normally when the Loop panel is opened `panel.boxObject.width` is 10px more than `iframe.boxObject.width`. After a theme change however, `iframe.boxObject.width` is 0.

This is what causes the panel to get so wide. Shane, do you have any ideas as to why the iframe gets a 0x0 size after the theme change?

Comment 7

[Shane Caraveo (:mixedpuppy)]

I've no idea why it is reset on them change, but that particular code would be improved by verifying that the iframe sizes are > zero.

Comment 8

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

Attachment: Patch

After a theme change, the iframe width drops to 0, and the height drops to 100.

First time opening after theme change: width: 0, height: 100
Second time opening after theme change: width: 320, height: 594

Posting this patch here which fixes the problem, but still looking in to why the iframe gets unloaded or at least resized.

Comment 9

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

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

Comment 10

[u279076]

I think we should take this for Firefox 34 if we can. It makes Firefox Hello look pretty bad on first impression if not.

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/7b9007ba3e17

Comment 12

[Maire Reavy [:mreavy]]

Pauly -- This should be in today's Nightly.  Will you have a chance to verify it today?

Comment 13

[Paul Silaghi, QA [:pauly]]

36.0a1 (2014-10-21), Win 7 x64
There are still some visual glitches left.

Comment 15

[Lawrence Mandel [:lmandel] (use needinfo)]

This affects 34 and 35. Now that it has been verified on m-c (and a follow-up bug filed), can you request uplift approval?

Comment 16

[Maire Reavy [:mreavy]]

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

Approval Request Comment
[Feature/regressing bug #]: Bad visual glitch
[User impact if declined]: Successive theme changes will make Loop/Hello panel size increase (Poor user perception of quality)
[Describe test coverage new/current, TBPL]: Manually tested
[Risks and why]: No risk out side of Hello and minimal risk to Hello
[String/UUID change made/needed]: No strings

Comment 17

[Lawrence Mandel [:lmandel] (use needinfo)]

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

Beta+
Aurora+

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fd789527c891

Comment 19

[Paul Silaghi, QA [:pauly]]

Verified fixed 35.0a2 (2014-10-23) Win 7

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/033942f8f817

Comment 21

[u279076]

Paul, can you please verify this is fixed in the next Beta?

Comment 22

[Paul Silaghi, QA [:pauly]]

Verified fixed FF 34b4 Win 7