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

Status: VERIFIED FIXED

(Toolkit :: UI Widgets, defect)

Description

[Sevaan Franks [:sevaan]]

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

Comment 1

[Matthew N. [:MattN]]

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

Comment 2

[Sevaan Franks [:sevaan]]

(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.

Comment 4

[Matthew N. [:MattN]]

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?

Comment 5

[Matthew N. [:MattN]]

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.

Comment 6

[Neil Deakin]

Attachment: patch in progress

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.

Comment 7

[Neil Deakin]

Attachment: Move popups within view notification rather than webshell notification

Comment 8

[[e-mail]]

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?

Comment 9

[Matthew N. [:MattN]]

(In reply to [e-mail] 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

Comment 10

[Timothy Nikkel (: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.

Comment 11

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fee439b5eedb

Comment 12

[Carsten Book [:Tomcat]]

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

Comment 13

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

Hey Neil, are you planning on uplifting this? We should get it uplifted soon if possible.

Comment 14

[Neil Deakin]

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

Comment 15

[Lukas Blakk [:lsblakk] use ?needinfo]

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.

Comment 16

[Mike Conley (:mconley) (:⚙️)]

Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/308399df0d7b

Beta: https://hg.mozilla.org/releases/mozilla-beta/rev/1a92004a684f

Comment 17

[Catalin Varga [QA][:VarCat]]

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

Comment 18

[Camelia Badau [:cbadau], Desktop Test Engineering]

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