1513066 - Fullscreen mode doesn't cover the full screen on win 7

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[Krzysztof]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

The bug looks like win7-specific.

1. Run Firefox Nightly. 
2. Open any site, youtube or about:newtab
3. Hit F11 or press the fullscreen button on youtube.


Actual results:

The window getting smaller actually, covers about 3/4 of the window. 

There's at least one more report of this issue on SUMO: https://support.mozilla.org/en-US/questions/1243053



Expected results:

The browser should go into the fullscreen mode.

Comment 1

[Krzysztof]

Liz, Mozregression points to this pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=7d539c01dad83dd9d6d2c0f9e3ab6efc390f971d&tochange=bcc079816150ebbb4e20c2cb7c68560b8cdfb075

Obvious reason is the patch in bug 1482920, which I already commented.

Comment 2

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Thank you for the report Krzysztof, you're right that my patch in bug 1482920 caused this. I can reproduce this in a Win 7 VM with DPI scaling at 125%, I overlooked that I need to scale by GetDesktopToDeviceScale. With per-monitor DPI scaling that's always 1.0, but per-monitor isn't available until Win 8.1. I'll have a patch shortly, this will need to be uplifted to beta as well.

Comment 3

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Attachment: Bug 1513066 - Scale monitor size when forcing fullscreen. r?jmathies

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/794798205610
Scale monitor size when forcing fullscreen. r=jmathies

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/794798205610

Comment 6

[Krzysztof]

Confirming, the bug is Fixed on my side. Thanks!

Comment 7

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

I realized when writing up the beta uplift request that the method I was using to get the screen was susceptible to the same scaling issue (which for instance breaks Shift+Win+Arrow monitor switching more than it was previously). I'm simplifying things to just use the same GetWidgetScreen() that InfallibleMakeFullScreen() does.

Comment 8

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

I'll address the monitor selection issue in followup bug 1514501.

Comment 9

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

Comment on attachment 9030905 [details]
Bug 1513066 - Scale monitor size when forcing fullscreen. r?jmathies

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1482920

User impact if declined: On Windows 7 with dpi scaling > 100%, a fullscreen window will not fill the whole screen. There is no user workaround other than changing scaling to 100%.

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): This addresses an oversight in scaling with force fullscreen, bringing the code in line with the scaling method used for window sizing in general. It is clearly more correct now and any remaining unanticipated risk would have been present in bug 1482920 to begin with.

String changes made/needed:

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9030905 [details]
Bug 1513066 - Scale monitor size when forcing fullscreen. r?jmathies

[Triage Comment]
Fixes incorrect fullscreen video playback on Windows 7 with dpi scaling > 100%. Approved for 65.0b5.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/cfbe2471b45a

Comment 12

[Anca Soncutean, Desktop QA]

I’ve reproduced this issue on Fx 65.0a1 (20181210095504) with DPI scaling at 125% on Windows 7 x86.
The issue is no longer reproducible with Fx 65.0b5 (20181217101316) and on Fx 66.0a1 (20181217093726) on the same platform.