Forcing fullscreen may choose wrong monitor on Win7

VERIFIED FIXED in Firefox 65

Status

()

defect
VERIFIED FIXED
5 months ago
4 months ago

People

(Reporter: agashlin, Assigned: agashlin)

Tracking

({regression})

66 Branch
mozilla66
Desktop
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox64 unaffected, firefox65+ verified, firefox66+ verified)

Details

Attachments

(1 attachment)

The fullscreen fix in bug 1482920 doesn't properly handle selecting the screen to use when forcing the window dimensions if DPI scaling is enabled in Windows 7. Bug 1513066 addresses scaling the size of the window but not scaling the coordinates used in selecting a monitor.
Assignee

Updated

5 months ago
OS: Unspecified → Windows 7
Hardware: Unspecified → Desktop
Steps to reproduce:

1. Configure 3 screens side-by-side, 1600x1200, 150% text size (DPI scaling), this is probably most easily done in VirtualBox.
2. Put a window on screen 2
3. Press F11 to fullscreen

Expected result:
Fullscreen on window 2

Actual result:
Fullscreen on window 3

This is because I'm passing the layout device pixels from Windows directly to ScreenForRect, which expects desktop pixels.

The fix is to divide by GetDesktopToDeviceScale() first.

---

While I'm in here, I noticed that the Shift + Windows + Left/Right Arrow hotkey (to move a window between screens) was behaving badly with DPI scaling on Win7 due to how we're rounding back and forth between layout device and desktop pixels (between setting Screen::mRectDisplayPix and nsWindow::Resize()), this causes fullscreen windows to be the wrong size by a pixel, and when it won't fit on the target screen (by some heuristic) Windows won't move the window with the hotkey:

1. Configure two 1600x1200 screens side-by-size, set text size to 150%
2. Open a window on screen 2, not maximized (in release 64.0 if you start on screen 1 it will fail in step 4, unless maximized in which case it will fail on step 5)
3. F11 to switch to fullscreen on screen 2
4. shift + windows key + left arrow to cycle to screen 1
5. repeat step 4 to switch back to screen 2

I'm including in this patch just using GetRect() directly instead of GetRectDisplayPix() and scaling to get the monitor size in layout device pixels, which was used in bug 1513066.
---

An old issue with the hotkey is bug 1475683, when maximized and fullscreen Shift+Win+Arrow will fail to move the window back to the primary monitor. No regression but I had hoped this would fix that, no such luck.
I needed to scale the layout device pixels coming from Windows before passing them to ScreenForRect(). Also, I'm using GetRect() directly instead of GetRectDisplayPix() * scale now, to avoid an unnecessary double scale & round which was making fullscreen windows off by one pixel in many cases.

Comment 3

4 months ago
Pushed by agashlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/741ded543228
Fix screen selection and scaling for fullscreen r=jmathies

Comment 4

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

Please nominate this for Beta approval when you get a chance. Let's get QA looking at this too.

Flags: qe-verify+
Flags: needinfo?(agashlin)

Comment on attachment 9034051 [details]
Bug 1514501 - Fix screen selection and scaling for fullscreen

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1482920

User impact if declined: On Win7, with multiple monitors (might need at least 3) and dpi scaling, when Firefox goes fullscreen it will do so on the wrong monitor.

Is this code covered by automated tests?: Unknown

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce:

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): It is a focused change which directly addresses the oversight that caused the regression. I've checked the issue on Nightly, reproducible in 20190109092644, fixed in 20190110893854.

String changes made/needed:

Flags: needinfo?(agashlin)
Attachment #9034051 - Flags: approval-mozilla-beta?

Comment on attachment 9034051 [details]
Bug 1514501 - Fix screen selection and scaling for fullscreen

[Triage Comment]
Fixes Firefox going fullscreen on the wrong monitor for some users under certain circumstances. Approved for 65.0b11.

Attachment #9034051 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

I have managed to reproduce this issue using Firefox 65.0b4 on Windows 7 32bit VM.

This issue is verified fixed using Firefox 66.0a1 (BuildId:20190114014511) and Firefox 65.0b11 (provided in comment 8) on Windows 7 32bit VM.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.