1287734 - incorrect window placement when dragging a tab to the desktop across monitors with mixed resolution

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Jonathan Kew [:jfkthame]]

STR, using a Retina MacBook with a 1920x1200 lo-dpi external monitor in extended-desktop mode:

1. Set the Retina screen as the primary display, and the external lo-dpi screen placed to its left.

2. Open a Firefox window with several tabs on the Retina screen, and size the window fairly small (e.g. about half the screen) so that there's room to move it around freely.

3. Drag one of the tabs over to the desktop on the external monitor, dropping it fairly close to the left-hand edge of the screen (to move the tab to a new window at that location).

Result: the new window is placed on the right-hand side of the external monitor, instead of at the drop location.


STR, v2:

1. As above, except configure the external lo-dpi screen to the right of the main (retina) display.

2. As above.

3. As above.

Result: the new window is opened on the internal Retina screen instead of the external monitor.

Comment 1

[Jonathan Kew [:jfkthame]]

AFAICT, this happens because nsWindowWatcher::SizeOpenedDocShellItem is mishandling coordinates when it tries to apply the SizeSpec it is given.

Its fatal mistake is that it tries to place the window using treeOwnerAsWin->SetPosition(), which takes device coordinates; but on a retina Mac, scaling window positions to device coordinates may result in ambiguous values. E.g. in STR v2 above, we have a retina screen which the OS considers to have Cocoa coordinates (0,0)..(1440,900), and then an external monitor at (1440,0)..(3360,1200). But when we work with LayoutDevicePixel coordinates, the retina screen is (0,0)..(2880,1800) -- so a device-pixel position of (2000, 100), for example, exists on both screens and cannot be converted to a unique location on the extended desktop unless we *also* know ahead of time which screen we want to use.

To reliably position windows across a multi-screen, mixed-resolution desktop, we should instead use the DesktopPix APIs (i.e. call treeOwnerAsWin->SetPositionDesktopPix), passing the coordinates as (system-dependent) "global desktop pixel" values that are unambiguous across all screens. These are the values that will eventually be passed to the system APIs that actually position the window.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Make nsWindowWatcher::SizeOpenedDocShellItem use global desktop coordinates to position the window, for reliable placement on mixed-resolution Mac systems

With this patch, the tab-drop operation is consistently resulting in a window at the expected position in my testing with the STR above.

Comment 3

[Masatoshi Kimura [:emk]]

Comment on attachment 8772367 [details] [diff] [review]
Make nsWindowWatcher::SizeOpenedDocShellItem use global desktop coordinates to position the window, for reliable placement on mixed-resolution Mac systems

Review of attachment 8772367 [details] [diff] [review]:
-----------------------------------------------------------------

::: embedding/components/windowwatcher/nsWindowWatcher.cpp
@@ +2258,2 @@
>        int32_t screenLeft, screenTop, screenWd, screenHt;
>        screen->GetRectDisplayPix(&screenLeft, &screenTop, &screenWd, &screenHt);

GetRectDisplayPix is supposed to return desktop pixels[1]. Why is this calculation needed at all?

[1] https://dxr.mozilla.org/mozilla-central/rev/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/widget/nsIScreen.idl#47

Comment 4

[Masatoshi Kimura [:emk]]

Comment on attachment 8772367 [details] [diff] [review]
Make nsWindowWatcher::SizeOpenedDocShellItem use global desktop coordinates to position the window, for reliable placement on mixed-resolution Mac systems

Ah, |left| and |top| were device pixels. Please disregard the previous comment. LGTM.

Comment 5

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6f91113b1bc9e3b3c0a2c4d0e91ec4d7159d84d8
Bug 1287734 - Make nsWindowWatcher::SizeOpenedDocShellItem use global desktop coordinates to position the window, for reliable placement on mixed-resolution Mac systems. r=emk

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6f91113b1bc9