1766803 - `XULPopupElement::OpenPopup` opens popup at right physical screen instead of left physical screen

Status: NEW

(Core :: XUL, defect)

Description

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

STR:

1. Use two physical screens.
2. Apply the current patches of bug 1744524. They add a <menupopup> to <main-popupset.inc.xhtml> and open it via XULPopupElement::OpenPopup with anchorElement = null.
3. Trigger opening that <menupopup> by calling navigator.clipboard.readText() from a x-origin iframe: Open https://jsfiddle.net/m1afgw9n/ at the left screen, click the button labeled "x".

Expected:
A "Paste" menupopup should appear at the current mouse position. That is, at the button labeled "x".

Actual:
The "Paste" button appears at the right screen.

Corresponding Pernosco run, containing some debugging notes.

Happened on Ubuntu 20.04.

See https://bugzilla.mozilla.org/attachment.cgi?id=9274188 for an example without an iframe. There, the issue does not occur.

Comment 1

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: menupopup_shown_at_correct_screen.mp4

Comment 2

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: menupopup_shown_at_wrong_screen.mp4

Comment 3

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Excerpt from mail-thread of Neil and me:

The trace suggests that a menu is being opened with no anchor (the second argument to ShowPopup is null), so it will open relative to the document window.

Yes.

Large x and y offset values are used that seem to suggest an attempt to open at the lower-right corner of the window.

Yes.

But the call to ScreenForRect at https://searchfox.org/mozilla-central/rev/0249c123e74640ed91edeabba00649ef4d929372/layout/xul/nsMenuPopupFrame.cpp#1786 is returning a different screen that I think you're expecting.

I can't tell exactly without knowing what screen setup you have, but this code is calling ScreenForRect(1126, 832, 1718, 926) and returing a screen that has dimensions (x=1920, y=358, w=1600, h=900)

The left screen has resolution 1920x1080, the right one 1600x900.

Assuming you have two screens horizontal next to each other, and the first

"first" means the left one.

is 1920 pixels wide,

Yes.

the call to ScreenForRect passes coordinates for an anchor that is indeed mostly on the second screen. Note that you can't display popups on more than one screen at once,

That's interesting to know. Shouldn't be the problem for that Pernosco run. The popup was opened at the bottom right area of the screen, but there was sufficient space to show it on that screen.

so one of the screens has to be selected.

Is this a menu you are trying to open?

Yes, it's a new menu, containing one entry labeled "Paste".

Comment 4

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

When setting the priority and severity of this issue, please take into account that according to telemetry data, roughly 10% of all release users use two or more screens.

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Mirko Brodesser (:mbrodesser) from comment #4)

When setting the priority and severity of this issue, please take into account that according to telemetry data, roughly 10% of all release users use two or more screens.

Per telemetry, this issue looks important enough to block clipboard.readText(), which is a high priority for us.
Hi Neil, can we have your help to fix this here? Thank you.

Comment 6

[Neil Deakin]

I answered this in bug 1744524.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:enndeakin, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 8

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

For now changing the ni?-request to myself to follow up 1744524#c17.

Comment 9

[Neil Deakin]

Attachment: WIP: Bug 1766803, don't adjust anchor based on offset

Comment 10

[Neil Deakin]

I found two issues here:

1. Removing the code added by 798226. I'm not sure why the anchor rectangle should be adjusted based on the offset. Removing it doesn't cause any test failures including the tests modified by that patch. Bug 798226 mainly affected the arrow placement, and we don't show the arrow anymore anyway, so maybe it doesn't matter anymore. I'll look in more detail into what that bug was trying to accomplish. The patch here fixes the menu position for most of the screen.
2. However, there is still an issue when the popup should appear near the border of the two screens. The scale being returned by https://searchfox.org/mozilla-central/rev/4719c903af605c2caeda3bb1ce4edb9ca58bf68e/layout/xul/nsMenuPopupFrame.cpp#1791 is incorrect. It is returning the scale for the first screen even though the window appears on the second screen. I only tested this on Mac with two screens with differing scales.

Comment 11

[Neil Deakin]

Attachment: WIP: Bug 1766803, patch for testing scale factor

Comment 12

[Neil Deakin]

This test in the patch here shows the last issue described in point 2 of comment 10. To test it:

1. Have a setup with two monitors each with a different desktop scale. I have the main laptop screen (2x scale) below and the second one (1x scale) above.
2. Apply the two patches in this bug.
3. Open firefox and click on the 'Click Here' label, which appears around where the sidebar is. A menu should appear near the mouse pointer.
4. The bug occurs if the window is positioned so that it straddles the border between both monitors, when the window appears on screen 2 but is mostly offscreen.

The zoom calculation at https://searchfox.org/mozilla-central/rev/4719c903af605c2caeda3bb1ce4edb9ca58bf68e/layout/xul/nsMenuPopupFrame.cpp#1791 seems to return the scale value for the first screen instead. I would expect it to always return the scale for the screen that the window is drawn on.

It works fine if I move the window up a bit so that more of it is visible or move it to the first screen.

I only verified the problem on Mac. I can test on other platforms later tonight.

jkew, do you know why the scale might be wrong here?

Comment 13

[Jonathan Kew [:jfkthame]]

(In reply to Neil Deakin from comment #12)

jkew, do you know why the scale might be wrong here?

It's a long time since I looked at mixed-resolution scaling issues, but..... I guess this is being called before the nsMenuPopupFrame has been drawn anywhere? If that's so, then I think I recall that its prescontext/devicecontext may not be able to provide a correct desktopToDevice scale, because the macOS window that'll be backing it isn't ready and correctly placed yet.

Kind of a chicken-and-egg situation: IIUC, you want the desktopToDeviceScale in order to compute the constraints for positioning the popup window (before actually displaying it); but until it is displayed (at a particular location), the OS doesn't know what its scale factor will be, because that depends where it is.

So if it's being positioned in relation to (and on the same screen as) an existing window, I suspect you might need to get the scale factor from that window (as it's already known) rather than from the not-yet-displayed popup. Does that make any sense/help at all?

Hmm... re-reading the description above, there's another thing that worries me. You refer to "the window is positioned so that it straddles the border between both monitors, when the window appears on screen 2 but is mostly offscreen". That's a somewhat weird edge case (literally!), in that I presume the scale factor of a window is determined by which monitor it is being displayed on, which (because of how macOS manages windows across monitor boundaries) may not be the same as the monitor that would intersect the largest part of it. I wonder if we have code that assumes that a window's scale factor is determined by the screen with the largest intersecting area. That was originally the case, I think, when macOS actually displayed a window spanning multiple screens -- its scale factor would flip according to which screen had the larger part. But then in some OS update, the behavior changed such that the window is displayed only on the screen where the user was manipulating it, and the (possibly-larger) part that crosses to another screen was hidden.

Comment 14

[Neil Deakin]

It's a long time since I looked at mixed-resolution scaling issues, but..... I guess this is being called before the nsMenuPopupFrame has been drawn anywhere? If that's so, then I think I recall that its prescontext/devicecontext may not be able to provide a correct desktopToDevice scale, because the macOS window that'll be backing it isn't ready and correctly placed yet.

Menus don't have a different devicecontext than their parent window whether they are open or not, so I would expect the scale returned by PresContext()->DeviceContext()->GetDesktopToDeviceScale() to return the scale of the screen the window is displayed on.

I wonder if we have code that assumes that a window's scale factor is determined by the screen with the largest intersecting area. That was originally the case, I think, when macOS actually displayed a window spanning multiple screens -- its scale factor would flip according to which screen had the larger part. But then in some OS update, the behavior changed such that the window is displayed only on the screen where the user was manipulating it, and the (possibly-larger) part that crosses to another screen was hidden.

This sounds like it may be the case somewhere.

Comment 15

[Itiel]

Something similar to this happens in RTL, where the Paste popup appears on the left side of the screen. I have only one monitor though.

Comment 16

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

(In reply to Itiel from comment #15)

Something similar to this happens in RTL, where the Paste popup appears on the left side of the screen. I have only one monitor though.

@Itiel: could you please provide steps to reproduce or a screen recording?

Comment 17

[Itiel]

STR:

1. Have only one monitor
2. Set intl.l10n.pseudo to bidi and restart
3. Click the Paste button in https://jsfiddle.net/m1afgw9n/
4. See that the Paste button appears on the left-most side of the screen

I'm not able to share a screen recording atm, but lmk if you still need one and I'll attach here.

Comment 18

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

(In reply to Itiel from comment #17)

STR:

1. Have only one monitor
2. Set intl.l10n.pseudo to bidi and restart
3. Click the Paste button in https://jsfiddle.net/m1afgw9n/
4. See that the Paste button appears on the left-most side of the screen

I'm not able to share a screen recording atm, but lmk if you still need one and I'll attach here.

Thanks. Could reproduce that issue.