Closed Bug 972566 Opened 6 years ago Closed 6 years ago

Panels aren't correctly anchored after resizing the anchor's window

Categories

(Toolkit :: XUL Widgets, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
mozilla31
Tracking Status
firefox29 --- verified
firefox30 --- verified
firefox31 --- verified

People

(Reporter: sevaan, Assigned: enndeakin)

References

Details

(Whiteboard: [Australis:P4])

Attachments

(1 file, 1 obsolete file)

I was playing around with the first-run tour experience for Australis, and decided to resize the window while the tour was running. Sometimes it resizes fine, and other times some of the tour elements hang.:

http://cl.ly/image/251019421S24
http://cl.ly/image/143u080v1Z43
http://cl.ly/image/3T1I1l2Q3F3P

Sometimes they persist until I resize the browser again, and sometimes it redraws after a short period of latency.

Running: 30.0a1 (2014-02-12) on Mac OSX 10.9.1
I've seen the latency before but I haven't noticed it get stuck in the wrong position like that.

It sounds like this is only after resizing so P4?

Thanks for the report
Priority: -- → P4
Hardware: x86 → All
(In reply to Matthew N. [:MattN] from comment #1)
> I've seen the latency before but I haven't noticed it get stuck in the wrong
> position like that.
> 
> It sounds like this is only after resizing so P4?
> 
> Thanks for the report

Hey Matt, I can get it to persist after a few rough resizing back-and-forths. Once I interact with the window again, then it corrects itself.
According to bug 969350 comment 4, this is reproducible 100% of the time on Linux.

Neil, is this something you can take a look at?
Component: General → XUL Widgets
Flags: needinfo?(enndeakin)
OS: Mac OS X → All
Priority: P4 → --
Product: Firefox → Toolkit
Summary: fx-UITour redraw issues when resizing browser window during first-run tour → Panels aren't correctly anchored after resizing the anchor's window
Whiteboard: [Australis:P4]
nsXULPopupManager::AdjustPopupsOnWindowChange is getting called when I resize on Linux but it seems like layout isn't getting told to redraw until I interact with the window in some way.
Attached patch patch in progress (obsolete) — Splinter Review
This patch fixes the issue on Linux. It changes to call AdjustPopupsOnWindowChange during the other WindowResized listener, after it reflows the content. This might not work though on platforms that don't have both listeners, so I need to investigate a bit more.

I think the original Mac issue is a different problem though as I think in this case the repositioning is done at the os level there.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Flags: needinfo?(enndeakin)
Attachment #8379931 - Attachment is obsolete: true
Attachment #8389917 - Flags: review?(neil)
Comment on attachment 8389917 [details] [diff] [review]
Move popups within view notification rather than webshell notification

I don't really know what the difference between the webshell and view resize is nor am I clear as to how I could test the code so I'd prefer if someone else looked at it.

>+void nsXULPopupManager::AdjustPopupsOnWindowChange(nsIPresShell* aPresShell)
>+{
>+  if (aPresShell->GetDocument()) {
>+    AdjustPopupsOnWindowChange(aPresShell->GetDocument()->GetWindow());
>+  }
> }
...
>+      if (presShell && presShell->GetDocument()) {
>+        pm->AdjustPopupsOnWindowChange(presShell);
Why not cut out the middle man and call pm->AdjustPopupsOnWindowChange(presShell->GetDocument()->GetWindow()); directly?
Attachment #8389917 - Flags: review?(neil)
(In reply to neil@parkwaycc.co.uk from comment #8)
> nor am I clear as to how I could test the code

"Help > &brandShortName; tour" will open the tour then just resize the window
Attachment #8389917 - Flags: review?(tnikkel)
Comment on attachment 8389917 [details] [diff] [review]
Move popups within view notification rather than webshell notification

Ah, that makes sense if the panels are anchored to content that we need to process the resize of the parent window and move the anchored content before repositioning the popups.
Attachment #8389917 - Flags: review?(tnikkel) → review+
https://hg.mozilla.org/mozilla-central/rev/fee439b5eedb
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Hey Neil, are you planning on uplifting this? We should get it uplifted soon if possible.
Flags: needinfo?(enndeakin)
Comment on attachment 8389917 [details] [diff] [review]
Move popups within view notification rather than webshell notification

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none
User impact if declined: UI tour highlight appears in the wrong place
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): low 
String or IDL/UUID changes made by this patch: none
Attachment #8389917 - Flags: approval-mozilla-aurora?
Flags: needinfo?(enndeakin)
Comment on attachment 8389917 [details] [diff] [review]
Move popups within view notification rather than webshell notification

[Triage Comment]
Given this is low risk and improves the UI tour we definitely want this on the first Australis release so please uplift to beta as well unless the patch doesn't land cleanly in which case nominate a new patch for that branch only.
Attachment #8389917 - Flags: approval-mozilla-beta+
Attachment #8389917 - Flags: approval-mozilla-aurora?
Attachment #8389917 - Flags: approval-mozilla-aurora+
I've verified the bug using the following environment:

FF 29.0b7 
Build id: 20140410150427
OS: Win Xp x86, Mac Os x 10.9, Ubuntu 13.04 x64
Verified on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OS X 10.9 using:
#latest Aurora, build ID: 20140422004001
#latest Nightly, build ID: 20140422030204
Status: RESOLVED → VERIFIED
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.