Closed Bug 1062335 Opened 5 years ago Closed 5 years ago

Loop panel size increases after switching themes

Categories

(Hello (Loop) :: Client, defect)

defect
Not set
Points:
5

Tracking

(firefox33 wontfix, firefox34+ verified, firefox35+ verified, firefox36 verified)

VERIFIED FIXED
mozilla36
Iteration:
36.1
Tracking Status
firefox33 --- wontfix
firefox34 + verified
firefox35 + verified
firefox36 --- verified
Blocking Flags:
backlog Fx34?

People

(Reporter: dhenein, Assigned: jaws)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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.
Component: General → Client
Product: Firefox → Loop
QA Contact: anthony.s.hughes
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.
FWIW, I've added a Moztrap testcase to make sure this is covered in our sign-off.
Duplicate of this bug: 1068592
Maire - Is this bug already on your list? Can you please get someone assigned to address this?
Flags: needinfo?(mreavy)
Flags: qe-verify?
Flags: firefox-backlog+
Hi Jared -- Can you pick up this bug (as part of the 36.1 sprint)?  Or should I look for someone else? Thanks.
Flags: needinfo?(mreavy) → needinfo?(jaws)
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Iteration: --- → 36.1
Points: --- → 5
Flags: qe-verify?
Flags: qe-verify-
Flags: needinfo?(jaws)
OS: Mac OS X → All
Hardware: x86 → All
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?
Flags: needinfo?(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.
Flags: needinfo?(mixedpuppy)
Attached patch PatchSplinter Review
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.
Attachment #8507903 - Flags: review?(mixedpuppy)
Attachment #8507903 - Flags: review?(mixedpuppy) → review+
I think we should take this for Firefox 34 if we can. It makes Firefox Hello look pretty bad on first impression if not.
backlog: --- → Fx34?
Flags: qe-verify- → qe-verify+
https://hg.mozilla.org/mozilla-central/rev/7b9007ba3e17
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in fx-team]
Target Milestone: --- → mozilla36
Pauly -- This should be in today's Nightly.  Will you have a chance to verify it today?
Flags: needinfo?(paul.silaghi)
36.0a1 (2014-10-21), Win 7 x64
There are still some visual glitches left.
Status: RESOLVED → VERIFIED
Flags: needinfo?(paul.silaghi)
Duplicate of this bug: 1067210
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?
Flags: needinfo?(jaws)
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
Flags: needinfo?(jaws)
Attachment #8507903 - Flags: approval-mozilla-beta?
Attachment #8507903 - Flags: approval-mozilla-aurora?
Comment on attachment 8507903 [details] [diff] [review]
Patch

Beta+
Aurora+
Attachment #8507903 - Flags: approval-mozilla-beta?
Attachment #8507903 - Flags: approval-mozilla-beta+
Attachment #8507903 - Flags: approval-mozilla-aurora?
Attachment #8507903 - Flags: approval-mozilla-aurora+
Verified fixed 35.0a2 (2014-10-23) Win 7
Paul, can you please verify this is fixed in the next Beta?
Flags: needinfo?(paul.silaghi)
QA Contact: anthony.s.hughes → paul.silaghi
Verified fixed FF 34b4 Win 7
Flags: needinfo?(paul.silaghi)
You need to log in before you can comment on or make changes to this bug.