1251624 - The menu appears on the second monitor with Nightly

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Šime Vidas]

Attachment: Capture.PNG

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:44.0) Gecko/20100101 Firefox/44.0
Build ID: 20160210153822

Steps to reproduce:

On Windows 10, when Firefox Nightly (currently 47) is maximized on my primary monitor and I press the main menu button (hamburger icon in the upper right)…


Actual results:

…the menu shows up on my secondary montiro. I’ve made a video of this issue here: https://www.youtube.com/watch?v=NpDqfPYaMy0 (first window is Firefox, second window is Firefox Nightly).

I’m attaching a screenshot of my current multi-monitor settings on Windows 10.


Expected results:

The menu should appear on my primary monitor (inside the Firefox window).

Comment 1

[Loic]

Jonathan, as you  worked on some patches about per-monitor DPI recently, might it be related?

Comment 2

[Jonathan Kew [:jfkthame]]

This is probably a duplicate of bug 1240533. If so, a fix landed recently. Please update to the latest Nightly ("Build ID 20160210153822" suggests it hasn't been updated for a couple of weeks) and see if it still occurs.

Comment 3

[Šime Vidas]

I’ve found the culprit. I use a custom layout.css.devPixelsPerPx value of 1.35 for my Firefoxes. After resetting the value, the issue disappears.

My Nightly’s build ID is 20160226030256, which should be up to date. It’s just that I always report bugs from Stable, hence the older ID. Is there a way I can check if that bug fix has landed in my version?

Comment 4

[Jonathan Kew [:jfkthame]]

20160226030256 includes the fix from bug 1240533 (landed 2016-02-22). So apparently there's still a remaining issue if you set a custom devPixelsPerPx value, although for the default setting it should now be fixed.

I have a guess as to what may be going wrong here... will test and follow up shortly.

Comment 5

[Jonathan Kew [:jfkthame]]

OK, I was able to reproduce and confirm the problem here. (Though we should note that custom devPixelsPerPx isn't really a supported thing, so a few glitches are likely.)

Anyway, this is simple to fix. The problem is that WinUtils::MonitorFromRect is given a rect in desktop pixel units (not CSS px), so it should NOT make use of the devPixelsPerPx value when converting to device pixels; that's what causes it to map some click locations to the wrong monitor, so that we then pop up the menu in the wrong place. Patch coming....

Meanwhile, I observed a second problem when using a custom devPixelsPerPx value, too: desktop notification windows (e.g. from the demo https://people.mozilla.org/~mnoorenberghe/w3c_notifications.htm) are incorrectly positioned. This is basically the opposite problem: in nsScreenWin::GetDefaultCSSScaleFactor, we fail to respect the devPixelsPerPx setting, and that's a problem because the front-end JS code that manages the notification window goes through CSS-px APIs. In this case we do need to include a check for a custom scaling factor to get the correct behavior. So let's fix that at the same time.

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: patch 1 - The desktop to device scaling in WinUtils::MonitorFromRect should not depend on custom CSS pixel scaling (devPixelsPerPx setting)

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: patch 2 - Check for scaling override (devPixelsPerPx setting) in nsScreenWin::GetDefaultCSSScaleFactor, for proper window positioning when a custom scale factor is used

Comment 8

[Jonathan Kew [:jfkthame]]

Try build including the two patches above:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b38297ac6af

Comment 9

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/319960f0619827640e3eee44562bad9f19450444
Bug 1251624 - patch 1 - The desktop to device scaling in WinUtils::MonitorFromRect should not depend on custom CSS pixel scaling (devPixelsPerPx setting). r=emk

https://hg.mozilla.org/integration/mozilla-inbound/rev/0b8286662a04cb6b71054a4926b6d0642da75c0d
Bug 1251624 - patch 2 - Check for scaling override (devPixelsPerPx setting) in nsScreenWin::GetDefaultCSSScaleFactor, for proper window positioning when a custom scale factor is used. r=emk

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/319960f06198
https://hg.mozilla.org/mozilla-central/rev/0b8286662a04

Comment 11

[Šime Vidas]

Looks good now. Thanks for fixing this, Jonathan ^_^