1931747 - Fix some PiP code to not assume outer window size equals inner size.

Status: VERIFIED FIXED

(Toolkit :: Picture-in-Picture, defect)

Description

[Loïc Yhuel]

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

Steps to reproduce:

Updated to latest Nightly (20241116214436) on Windows 11.
Previous Nightly (20241115210754) was fine.

Actual results:

Windows are square with no border and shadow

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Win32' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Mayank Bansal]

I am on Windows11, and i didnt notice anything different. So' this may be something specific.

Can you
copy-paste the contents of "about:support" from your browser
Attach a screenshot of what is the issue?

Comment 3

[Loïc Yhuel]

Attachment: about:support text

Comment 4

[Loïc Yhuel]

Attachment: firefox window on a white background

notice the lack of rounded corners and shadow

Comment 5

[Mayank Bansal]

Thanks!

Bisection:
Bug 1930292 - Stop setting non-zero chromemargin values. r=win-reviewers,handyman
Differential Revision: https://phabricator.services.mozilla.com/D228528

Comment 6

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

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

:emilio, since you are the author of the regressor, bug 1930292, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Ugh, I think if I can repro this I couldn't notice because I had this applied locally:

diff --git a/widget/windows/nsWindow.cpp b/widget/windows/nsWindow.cpp
index 979eca7a443e5..d99092dae00fd 100644
--- a/widget/windows/nsWindow.cpp
+++ b/widget/windows/nsWindow.cpp
@@ -1078,6 +1078,9 @@ nsresult nsWindow::Create(nsIWidget* aParent, const LayoutDeviceIntRect& aRect,
     const DWM_SYSTEMBACKDROP_TYPE tabbedWindow = DWMSBT_TABBEDWINDOW;
     DwmSetWindowAttribute(mWnd, DWMWA_SYSTEMBACKDROP_TYPE, &tabbedWindow,
                           sizeof tabbedWindow);
+    const auto round = DWMWCP_ROUND;
+    DwmSetWindowAttribute(mWnd, DWMWA_WINDOW_CORNER_PREFERENCE, &round,
+                          sizeof round);
   }

   if (mOpeningAnimationSuppressed) {

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1931747 - Remove unnecessary condition in PreXULSkeletonUI. r=#win-reviewers

We always have Dwm{Get,Set}WindowAttribute because we fail if we can't
load them.

This in practice means that we release the DC and uncloak the window a
bit later (due to the MakeScopeExits), but I think that's fine.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1931747 - Ask DWM to always round toplevel windows. r=#win-reviewers

It seems bug 1930292 can cause DWM heuristics to not round our windows
some of the time. Explicitly ask it to do that. This has an extra
benefit which is that PiP windows are now rounded like on macOS.

Comment 10

[Mayank Bansal]

FWIW, if you enable mica from about:config, you dont see this bug (that is, window borders have both shadow and rounded corners)

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1931747 - Don't extend into the frame border area with chromemargin=0,0,0,0. r=#win-reviewers

Windows 10 and 11 don't have fat windows, so this is unnecessary and
breaks other effects like the accent-color border (if you have "Show
accent color on titlebar and window borders" enabled).

We need to make the custom resize margin thing we have for PiP not mess
with our non-client area, but that is trivial.

Comment 12

[Emilio Cobos Álvarez [:emilio]]

https://treeherder.mozilla.org/jobs?repo=try&revision=81e782a9407e0d8551c8a53a6efe28b79ff58679

Comment 13

[Phabricator Automation]

Comment on attachment 9438204 [details]
Bug 1931747 - Remove unnecessary condition in PreXULSkeletonUI. r=#win-reviewers

Revision D229267 was moved to bug 1931778. Setting attachment 9438204 [details] to obsolete.

Comment 14

[tabmix.onemen]

simple workaround for now:

#main-window[sizemode="normal"] {
  border-radius: 2px;
}

Comment 16

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1931747 - Make picture in picture deal with outer != inner sizes correctly. r=#pip-reviewers

The previous patch makes the PiP window have rounded corners and shadows
on Windows (like on macOS).

That uncovers some cases where PiP was using inner rather than outer
sizes incorrectly, mostly in test code. This wasn't a problem before my
patch because innerSize used to be the same as outerSize before my
patch.

I could restore the previous behavior (by using hidechrome="true" or
so), but I think this is nicer, and the code works with outer sizes
regardless (i.e., if we want to restore the old behavior we don't need
to touch any of this code, just add hidechrome, so that outer and inner
sizes match).

Note that all the code in PictureInPicture.sys.mjs already deals with
outer sizes (correctly so).

Comment 17

[Emilio Cobos Álvarez [:emilio]]

https://treeherder.mozilla.org/jobs?repo=try&revision=6ead823b1e6d3ef757a3ad6c633db8db1442c16b (https://treeherder.mozilla.org/jobs?repo=try&revision=937d167d28e7eb29241b11cd7f088d2d1c9607da for a previous try run with the PiP failures)

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5fea4129f696
 - Make picture in picture deal with outer != inner sizes correctly. r=pip-reviewers,mconley

Comment 21

[Ray Kraesig [:rkraesig]]

Please consider resolving the above test failures by reverting the relevant patch, and then (if you still think it can and should be committed separately) moving the PiP changes to their own bug. I'd much prefer to avoid another situation like bug 1891063.

Comment 22

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/5fea4129f696

Comment 23

[Emilio Cobos Álvarez [:emilio]]

Ok, let's repurpose this bug for the PiP issue fixed in https://hg.mozilla.org/mozilla-central/rev/5fea4129f696. I'll ask for backout of the regressor and will move the patch somewhere else.

Comment 24

[Phabricator Automation]

Comment on attachment 9438214 [details]
Bug 1931747 - Don't extend into the frame border area with chromemargin=0,0,0,0. r=#win-reviewers

Revision D229270 was moved to bug 1930292. Setting attachment 9438214 [details] to obsolete.

Comment 25

[Monica Chiorean]

I was able to reproduce the issue on Win10x64 using Firefox build 134.0a1(20241116214436).
Verified as fixed on Win10x64 using Firefox builds 135.0a1 and 134.0b4.