1797170 - Firefox opens new windows on other displays at the wrong DPI

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect, P3)

Description

[Cory]

Attachment: Untitled Project.mp4

Steps to reproduce:

Two monitors hooked to my laptop. Both monitors are 1080p. Laptop is 4k.
Firefox is open full screen on 1080p monitor B.
On certain websites, clicking a link that spawns a new window places the new window on the laptop screen and not monitor B.

Prior to v104 (around there) Firefox would render the pop-up window as if it were opened on a 1080p monitor and not the 4k one. So you simply had to move the window if you wanted.

Actual results:

Now, the pop-up opens on the 4k main laptop screen with the DPI or resolution messed up and the entire contents of the pop-up are very small and not rendered within the full viewport of the pop-up. When this window with smaller content is dragged to the 1080p monitor it grows to the correct size of the viewport and stays that way no matter which screen it is then moved to after.

Expected results:

The pop-up should open on the same screen as the main Firefox window, however if it doesn't it should not render the contents of the pop-up incorrectly according to the resolution/DPI of the other monitor.

Comment 1

[Ray Kraesig [:rkraesig]]

Attachment: bug-1797170-testcase.html

Thanks! I can reproduce this on a current Nightly, and have attached a local test case.

The pop-up should open on the same screen as the main Firefox window

This is worth opening a separate bug for already filed as bug 1266206. We may not be able to fix it for backwards-compatibility reasons, but it's probably worth looking into.

Comment 2

[Ray Kraesig [:rkraesig]]

Attachment: bug-1797170-testcase.html

(Reduced margin in test case to prevent scrollbars from being applied a little too eagerly under some setups.)

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1757410

:hiro, since you are the author of the regressor, bug 1757410, could you take a look?

For more information, please visit auto_nag documentation.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

(In reply to Ray Kraesig [:rkraesig] from comment #1)

This is worth opening a separate bug for already filed as bug 1266206. We may not be able to fix it for backwards-compatibility reasons, but it's probably worth looking into.

Interesting. Bug 1266206 was originally opened for Linux, but with the test case in comment 2 (Thanks Ray for the test case!), the popup window is opened in the same monitor at least on Wayland. I will have to set up Windows.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

I am still struggling with finding out the place where we position newly created popup windows. What I've found so far is on Windows (11) we get two WM_DPICHANGED notifications, the first one is for the monitor where the original firefox window is positioned, the second one is for the monitor where the popup window will be appeared. Thus I am suspecting we (or Windows) tries to position the new window on the same monitor initially and move it to the other monitor. Of course there's definitely a flaw about DPI change notification in our code the second DPI change notification is ignored somewhere, the reason I am guessing is we consider the DPI isn't changed since the initial DPI value for the popup window is the monitor where the popup window appears.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #5)

Of course there's definitely a flaw about DPI change notification in our code the second DPI change notification is ignored somewhere, the reason I am guessing is we consider the DPI isn't changed since the initial DPI value for the popup window is the monitor where the popup window appears.

This flaw might be inevitable since our DPI change notification is async.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

I found the place where the problem is. The problem is here in ContentChild::ProvideWindowCommon. The code in question is;

    ParentShowInfo showInfo(                                                    
        u""_ns, /* fakeShowInfo = */ true, /* isTransparent = */ false,         
        aTabOpener->WebWidget()->GetDPI(),                                      
        aTabOpener->WebWidget()->RoundsWidgetCoordinatesTo(),                   
        aTabOpener->WebWidget()->GetDefaultScale().scale);

This aTabOpener->WebWidget() isn't the widget for the popup-ed content, it's rather for the original window. Thus the GetDPI() returns a wrong value. It will end up being used for DoFakeShow call in the same function and it will be propagated into the pres shell for the popup-ed content. And before bug 1757410, we did call UIResolutionChanged which is an async version of UIResolutionChangedSync which is now we use, thus the wrong DPI value had been clobbered by the async UIResolutionChanged call.

Note about the issue that newly popup window isn't opened in the same monitor where the original firefox window is positioned. TIL there's a spec text explaining where window.open-ed window should be positioned. In terms of the spec, the current Firefox behavior is correct. It's not an issue at all.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1797170 - Use the proper widget to obtain per-monitor metrics (e.g. DPI) for doing a fake show. r?emilio

Comment 9

[Ray Kraesig [:rkraesig]]

Is the above also expected to fix bug 1550421?

Comment 10

[Hiroyuki Ikezoe (:hiro)]

If I understand bug 1550421 correctly, the answer is yes. I confirmed D160875 fixes bug 1550421 on my macbook with an external monitor by using the Ray's test case in comment 2.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1e74216b2cc9
Use the proper widget to obtain per-monitor metrics (e.g. DPI) for doing a fake show. r=emilio

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/1e74216b2cc9

Comment 14

[Rares Doghi, Desktop QA]

I was able to reproduce this issue using the attached test case and Confirmed the Fix. This issue is Verified as fixed in our latest Nightly build 108.0a1 (2022-11-07).

Comment 15

[Rares Doghi, Desktop QA]

Updating the Status flags for this issue.