Closed Bug 1027014 Opened 8 years ago Closed 7 years ago

Loop Panel is cut off on first display (Panel resizing is not always fired on first load)

Categories

(Hello (Loop) :: Client, defect, P1)

defect

Tracking

(firefox33 verified, firefox35 verified)

VERIFIED FIXED
mozilla33
Tracking Status
firefox33 --- verified
firefox35 --- verified

People

(Reporter: standard8, Assigned: standard8)

References

Details

(Keywords: regression)

Attachments

(2 files)

STR:

1) Start up Firefox
2) Click the button to open the loop panel.

=> Panel has bottom half cut-off as per screenshot.

3) Close the panel and re-open it.

=> Panel is displayed correctly.

I'm not sure how this has happened yet, I'm pretty sure I wasn't seeing it during my testing.
Priority: -- → P1
Target Milestone: --- → mozilla33
Ok, I've just confirmed the right patch landed, and that my old branch still works fine.

Now to go regression hunting...
Assignee: nobody → standard8
I suspect this has to do with one display being HiDPI and the other not.  The browser I'm typing in, for example, if I dropdown the panel on my (non-HiDPI) dell monitor, it looks great.  If I drag this same window over to my Macbook HiDPI display, and drop down the same panel again, it's cut off.


I've been seeing bugs of this sort in the SocialAPI for quite a while.  I have the impression that at least one of them is already on file.
(In reply to Dan Mosedale (:dmose - needinfo? me for responses) from comment #2)
> I suspect this has to do with one display being HiDPI and the other not. 
> The browser I'm typing in, for example, if I dropdown the panel on my
> (non-HiDPI) dell monitor, it looks great.  If I drag this same window over
> to my Macbook HiDPI display, and drop down the same panel again, it's cut
> off.

That sounds like a different bug (mine actually ends up 2 x wider). My issue is 100% reproducible on a non-HiDPI display.
I've now done a bisect and tracked this down to bug 994117, specifically this patch:

https://hg.mozilla.org/mozilla-central/rev/0e9d11e5302a

I can't reproduce on Windows, so I suspect this could be a timing related issue, and from the details on that patch it certainly could be.
Blocks: 994117
The thing I describe is a regression, :andreio and I both saw it appear for the first time immediately after updating for the first time after your patch landed a few days ago.
But I see that it could well have been caused for us by the checkin you mention as well.  We're both on OS X.
I would suggest filing a separate bug for your issue (and attach screenshots please!), I strongly suspect HiDPI issues are a different bug, possibly in the code we're now sharing with social (which would also have shown up with the patch landing).
FYI: I was able to reproduce this on Windows.  I saw it yesterday when I upgraded and just now with today's upgrade.  The panel is cut off (I don't see "do not disturb"), and it's only the first time after upgrade.  If I close the panel and reopen, it's not cut off.
Attached image cutoff panel.png
Cut-off panel on the first run after Nightly upgrade on June 23rd.
This happens only once after the upgrade - if I close the panel (without generating a URL) and open it again it will always be displayed correctly afterwards.
Target Milestone: mozilla33 → 33 Sprint 2- 7/7
OS: Mac OS X → All
This seems to fix the issue. After some debugging, I found that sometimes the newly created panel didn't have the DynamicResizeWatcher started on it, which means it isn't getting a size to content call, and hence it isn't resizing properly on first load.

I tracked this down to the wasAlive option - when setting up the load handlers to initialize the frame and setup the dynamic resizer. If I removed the check for wasAlive, then the panel would be resized correctly.

I think this is seen in the Loop panel case (especially after bug 994117) as the panel is being loaded from content, and hence is faster than the animation, so by the time "onpopupshown" now gets triggered, we've already loaded the panel content, hence the code then waits for a "load" event, but never gets one.

I tracked down wasAlive to bug 811247, but couldn't really see why it was introduced. I'm not convinced its needed, hence I'm proposing we remove it here.

There's also some old code for loop I'm removing - it was an artefact of how we used to resize before we moved to a shared panel, and now doesn't do anything anyway.


I've not run automated tests on this yet, but I don't think there will be an issue.

Shane, what do you think about removing this wasAlive check?
Attachment #8444503 - Flags: feedback?(mixedpuppy)
Comment on attachment 8444503 [details] [diff] [review]
Loop panel is cut off on first display - always fire the mechanism to resize the panel if the ready state is complete.

I checked this today on try server and it was fine, hence bumping up to a review request.

https://tbpl.mozilla.org/?tree=Try&rev=58465aeb5caa
Attachment #8444503 - Flags: feedback?(mixedpuppy) → review?(mixedpuppy)
Attachment #8444503 - Attachment description: Loop panel is cut off on first display - always fire the mechanism to resive the panel if the ready state is complete. → Loop panel is cut off on first display - always fire the mechanism to resize the panel if the ready state is complete.
Summary: Loop Panel is cut off on first display → Loop Panel is cut off on first display (Panel resizing is not always fired on first load)
Attachment #8444503 - Flags: review?(mixedpuppy) → review+
Blocks: 1027053
https://hg.mozilla.org/integration/mozilla-inbound/rev/71ebefd8f3e7
Target Milestone: 33 Sprint 2- 7/7 → mozilla33
Are there reliable steps to reproduce this bug (including system config)? I've yet to be able to reproduce this and I'd like this regression to have test coverage.
Flags: qe-verify?
Mark, can you please answer comment 14?
Flags: needinfo?(standard8)
(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #14)
> Are there reliable steps to reproduce this bug (including system config)?
> I've yet to be able to reproduce this and I'd like this regression to have
> test coverage.

Comment 0, on Mac with a non-HiDPI display is the best I can give you. In theory it shouldn't matter with a opt or debug build, but it might be more likely to happen on a slower debug build.
Flags: needinfo?(standard8)
Paul, can you try to verify this in the latest Nightly based on comment 0 and comment 16?
Flags: qe-verify?
Flags: qe-verify+
Flags: needinfo?(paul.silaghi)
I reproduced the issue on nightly 2014-06-18 on Win 7 x64.
Verified fixed 33.0a1 (2014-07-01), 33b5, 35.0a1 (2014-09-21).
Status: RESOLVED → VERIFIED
Flags: needinfo?(paul.silaghi)
You need to log in before you can comment on or make changes to this bug.