1444385 - Popup panel does not follow the anchor position when resizing window

Status: RESOLVED WONTFIX

(Firefox :: Toolbars and Customization, defect, P5)

Description

[magicp]

Attachment: steps-to-reproduce_20180309_223145.mp4

Steps to reproduce:
1. Launch Nightly on Windows or Ubuntu
2. Install the window manipulator in webextensions examples(https://github.com/mdn/webextensions-examples/tree/master/window-manipulator)
3. Select "Resize window to 768x1024" in window manipulator's popup
4. Select "Resize all windows to 1024x768"

Actual results:
Popup panel does not follow the anchor position when resizing window. I can reproduce on Windows 10 and Ubuntu 17.10, also cannot reproduce on macOS because a popup panel is closed when resizing window.

Expected results:
Popup panel should follow the anchor position.

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2d815cb45cee4537af1e08c7ca8623c02c8a91b6&tochange=d8321357c4a7577860801e5fced1e2257ec428bf

Comment 1

[:Gijs (he/him)]

Based on the regression window, Sam, could you have a look?

Comment 2

[Sam Foster [:sfoster] (he/him)]

(In reply to :Gijs from comment #1)
> Based on the regression window, Sam, could you have a look?

Yeah I'm looking into this. The popup should get closed when the window resizes, but it appears to stop doing this with this patch.

Comment 3

[Sam Foster [:sfoster] (he/him)]

(In reply to magicp from comment #0)

> Actual results:
> Popup panel does not follow the anchor position when resizing window. I can
> reproduce on Windows 10 and Ubuntu 17.10, also cannot reproduce on macOS
> because a popup panel is closed when resizing window.
> 
> Expected results:
> Popup panel should follow the anchor position.

For better or worse, the expected results here are that the popup should close. This is the behavior that was changed by the patch in the regression range: before, when you clicked the resize links it would close the popup. After, with the patch applied, the window resizes (and moves relative to the popup) and the popup remains open in what is now the wrong position. 

Removing this.autoPosition = false; in the popuppositioned handler in PMV "restores" the previous behavior, but it also undoes one of the glitches this patch was trying to fix: 

1. move the browser window to the lower part of the screen
2. open e.g. the overflow menu - it opens below the anchor. 
3. open the library sub-panel (or any longer sub-panel) - it opens above the anchor, then moves back below the anchor. 

This is because when autoPosition is false, the position is recalculated each time. PMV tries hard to ensure the height of the popup doesnt change once opened, so we want to avoid that.  

I'm looking around to see *why* the popup would close when the window resizes - that's not actually obvious to me right now. Regarding the position, we may be able to keep autoPosition=true, but compare to previous values to see keep the popup from jumping around? (and also from triggering the relatively expensive adjustArrowPosition code each time)

Comment 4

[Sam Foster [:sfoster] (he/him)]

Unless this shows up as an issue in automation, I'm inclined to wontfix this. Practically speaking, its not possible for an end-user to trigger this behavior. Any PanelMultiView popup will hide once the user clicks anywhere to resize the window. That leaves developer-focused cases like this extension, or resizing from devtools, where this is a legitimate issue, but a rare edge case. 

I'll keep any eye on it for a while to see if there is further fallout before closing it. I'm also open to other suggestions if someone wants to make a different case for this issue.

Comment 5

[Sam Foster [:sfoster] (he/him)]

:paolo, do you have an opinion on this? I'm still leaning towards wontfix but at this point you have been in this code more than I.

Comment 6

[:Paolo Amadini]

I agree we can close this bug, given that it only affects rare edge cases, and fixing it has downsides for some relatively more common edge cases that can be user-triggered. Thank you Sam for the detailed analysis in comment 3 and 4!