Closed Bug 1254026 Opened 4 years ago Closed 3 years ago

Save As and Open File dialogs are not scaled according to secondary display's resolution

Categories

(Core :: Widget: Win32, defect)

Unspecified
Windows
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox47 --- wontfix
firefox48 --- wontfix
firefox49 --- fixed
firefox50 --- fixed

People

(Reporter: aklotz, Assigned: jfkthame)

References

Details

Attachments

(2 files, 2 obsolete files)

Macbook Pro Retina, bootcamp Windows 10

Bring up a Save As dialog. The dimensions of the dialog box are fine, the dimensions of the pushbuttons are fine, but everything inside of it is too small.
Screenshot, please. Do you have an external display as well, or only the MBP's retina screen?
Flags: needinfo?(aklotz)
As well here... no problem for me.  Save as dialog is a windows widget.
On Win8.1, the Save As dialog is displayed according to the system's primary monitor's resolution. So if you have a lo-dpi primary display, and open Save As on a hi-dpi secondary display, it'll look very small; and in the reverse scenario, it appears awfully big.

This seems to be "expected" Windows behavior: I see the same thing with the Save As dialog in IE10 on my Win8.1 system. It matches the unscaled rendering of window borders, titlebars, controls, the system popup menu you get by right-clicking the title bar, etc. I think this is a misguided design choice in Windows, but it's how the system consistently behaves.

Not sure if there's anything we can currently do to work around this, short of implementing our own in-app Save As dialog instead of using the system-provided one.
Summary: Save As dialogs are not scaled on Retina → Save As and Open File dialogs are not scaled according to secondary display's resolution
The same issues apply to the Open File dialog.
FWIW, we're not the only people who don't currently know how to fix this:
http://stackoverflow.com/questions/35454712/make-windows-common-dialogs-per-monitor-dpi-aware
(In reply to Jonathan Kew (:jfkthame) from comment #5)
> FWIW, we're not the only people who don't currently know how to fix this:
> http://stackoverflow.com/questions/35454712/make-windows-common-dialogs-per-
> monitor-dpi-aware

Hmm, if that is any indication, we should be using an updated API when running on Vista or newer.
Flags: needinfo?(aklotz)
AFAICS, we -are- already using the "updated" APIs mentioned there -- see nsFilePicker::ShowFilePicker() in widget/windows/nsFilePicker.cpp, which uses IFileOpenDialog/IFileSaveDialog.
I've heard from contacts at MS confirming that there will be new APIs in a future version of Windows to help address this (but for now, it is what it is...)

As of now, this could arguably be closed as WONTFIX (or maybe INVALID), as it's not a Firefox bug, it's expected Windows behavior. But I'll leave it open because we'll want to adopt the new APIs as soon as they're available.
I'm not sure if this actually works, but would wrapping those APIs in ActivateActCtx/DeactivateActCtx with an activation context that does /not/ indicate multi-monitor DPI support help? My thought pattern is that maybe it would force Windows to scale just the dialog?

(I know that would work for other aspects such as sxs redirection, but I'm not sure if DPI scaling would work the same way)
I don't believe so. Per-monitor dpi awareness is a process-wide state declared in the application manifest, and not connected to activation contexts.
Duplicate of this bug: 1270589
The latest Windows Insider build includes new APIs that allow us to fix this issue; here's a patch that should do it.
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Duplicate of this bug: 1255058
Duplicate of this bug: 1279475
Comment on attachment 8757311 [details] [diff] [review]
When Win10 dpi-awareness-context APIs are available, run the Open File and Save As dialogs as dpi-unaware windows, so that they are auto-scaled by the system appropriately for the display's resolution

The Win10 Anniversary Update due to ship at the beginning of August will include these APIs, allowing a much improved user experience with the file dialogs. So we should get this into the tree and onto the trains soon...

(This patch sets up the EnableNonClientDpiScaling proc ptr as well, although it's not actually used here; we'll be needing that too, for bug 1270954, so I included it here for simplicity but we can split it off if you prefer.)
Attachment #8757311 - Flags: review?(VYV03354)
Attachment #8757318 - Flags: review?(VYV03354)
Comment on attachment 8757311 [details] [diff] [review]
When Win10 dpi-awareness-context APIs are available, run the Open File and Save As dialogs as dpi-unaware windows, so that they are auto-scaled by the system appropriately for the display's resolution

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

::: widget/windows/WinUtils.cpp
@@ +476,5 @@
>      }
>    }
> +
> +  if (IsWin10OrLater()) {
> +    HMODULE user32Dll = ::LoadLibraryEx(L"user32", NULL,

GetModuleHandle() is sufficient. We implicitly link to user32, so it will never be unloaded.

::: widget/windows/nsFilePicker.cpp
@@ +1050,5 @@
>    mFiles.Clear();
>  
> +  // On Win10, the picker doesn't support per-monitor DPI, so we open it
> +  // with our context set temporarily to non-dpi-aware
> +  WinUtils::AutoDpiUnaware dpiUnaware;

Why is this DPI-unaware instead of system-DPI-aware? System-DPI-aware will prevent blurry dialogs on single high-DPI monitor systems.
Attachment #8757311 - Attachment is obsolete: true
Attachment #8757311 - Flags: review?(VYV03354)
Attachment #8757318 - Attachment is obsolete: true
Attachment #8757318 - Flags: review?(VYV03354)
Attachment #8767356 - Flags: review?(VYV03354) → review+
Attachment #8767357 - Flags: review?(VYV03354) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/1b782dcb677abdbbeade064e3321a87505b7d0c9
Bug 1254026 - When Win10 dpi-awareness-context APIs are available, run the Open File and Save As dialogs as system-dpi-aware windows, so that they are auto-scaled by the system appropriately for any secondary display's resolution. r=emk

https://hg.mozilla.org/integration/mozilla-inbound/rev/5c98fa080b96a19d4ed2dc6947c78ba7240fe19b
Bug 1254026 - followup - Apply the same fix to the Print dialog. r=emk
https://hg.mozilla.org/mozilla-central/rev/1b782dcb677a
https://hg.mozilla.org/mozilla-central/rev/5c98fa080b96
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Comment on attachment 8767356 [details] [diff] [review]
When Win10 dpi-awareness-context APIs are available, run the Open File and Save As dialogs as system-dpi-aware windows, so that they are auto-scaled by the system appropriately for any secondary display's resolution

With the Windows 10 Anniversary Update due to ship very soon (early August, IIUC), it would be good to get this into FF48/49 rather than waiting for FF50 to improve the user experience.

Approval Request Comment
[Feature/regressing bug #]: Win10 hi-dpi support

[User impact if declined]: file open/save dialogs mis-sized on secondary monitor

[Describe test coverage new/current, TreeHerder]: tested manually (requires mixed-dpi config)

[Risks and why]: minimal, no effect except on mixed-dpi Windows systems

[String/UUID change made/needed]: n/a
Attachment #8767356 - Flags: approval-mozilla-beta?
Attachment #8767356 - Flags: approval-mozilla-aurora?
Hi Jonathan,
Since it's late beta, is this patch critical for user? I would like this patch ride the train on 49/50 and bake more time.
Flags: needinfo?(jfkthame)
(Yes, I figured it might be too late to take this on beta.)

I don't think we can regard this as "critical". The undesired behavior (Open/Save/Print dialogs don't scale appropriately on secondary displays with a different DPI setting) already exists for Windows 8.1 and 10 users; it's ugly but not crippling. (Note that the behavior is a result of limitations in the Windows APIs for supporting mixed resolutions, it's not really a Firefox bug.)

The Win10 Anniversary Update will introduce new APIs (currently in Insider builds) that can solve the issue, and will ship to end users next month, but our users won't get the benefit until we ship an update of Firefox that actually uses the new APIs. So that's what we're doing here.

But I'd agree that it's not "critical" to have this as soon as Win10AU goes out; we can deliver the fix somewhat later. In the meantime, things just continue as-is (ugly, but functional) for those users.
Flags: needinfo?(jfkthame)
Comment on attachment 8767357 [details] [diff] [review]
followup - Apply the same fix to the Print dialog

See approval request for the main patch (above).
Attachment #8767357 - Flags: approval-mozilla-aurora?
Comment on attachment 8767356 [details] [diff] [review]
When Win10 dpi-awareness-context APIs are available, run the Open File and Save As dialogs as system-dpi-aware windows, so that they are auto-scaled by the system appropriately for any secondary display's resolution

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

Since this is not critical for user, let it ride the train on 49/50.
Attachment #8767356 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Blocks: 1270954
Comment on attachment 8767356 [details] [diff] [review]
When Win10 dpi-awareness-context APIs are available, run the Open File and Save As dialogs as system-dpi-aware windows, so that they are auto-scaled by the system appropriately for any secondary display's resolution

The patch fixes a UI issue in secondary display's resolution. Take it in 49 aurora.
Attachment #8767356 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8767357 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Depends on: 1354020
You need to log in before you can comment on or make changes to this bug.