1568316 - Picture-in-Picture window is missing the OS drop shadow

Status: NEW

(Toolkit :: Picture-in-Picture, enhancement, P4)

Description

[Mike Conley (:mconley) (:⚙️)]

See screenshots - the Picture-in-Picture player window is missing the OS-provided dropshadow on Windows. Same seems to be the case on macOS.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Dropshadow on a normal window

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Attachment: No dropshadow on the Picture-in-Picture player window

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Maybe relevant: https://stackoverflow.com/questions/43818022/borderless-window-with-drop-shadow

Comment 4

[John]

This also appears to be missing on Linux. At least on Fedora 31 with Gnome 3.34, running the latest nightly on Wayland.

Comment 6

[Ania]

Attachment: macOS dropshadow

It seems like macOS drop shadow is there now.

The lack of it on Windows doesn't look beautiful, so I'm going to add this to our potential PiP TLC effort.

Comment 7

[Molly Howell (she/her) (no longer active)]

There would be a couple of ways we could handle this, either opt in to Windows's default drop shadow provided by the DWM, or render our own. Experience trying to add our own shadows to our context menus for Proton makes me think that rendering them ourselves is unlikely to work out, so we'll go with the OS one.

That means what we need to do is:

1. Find a way for nsWindow to detect when it has a window that should get a shadow and needs to opt in to it. I don't immediately see anything obvious that wouldn't hit some number of false positives, so we may need to just add a flag for this (perhaps as a new feature string?).
2. In that case, use DwmExtendFrameIntoClientArea to add any amount of frame, a single pixel on all sides would work fine. This tells the DWM that it's allowed to do things to that window, which will enable the default shadow.

One other thing to note here; this will also opt us in to the Windows 11 rounded corners. We can turn that off if we want to, but it isn't immediately obvious to me whether we do; that's a decision that needs to be made. There would also be another complication, which is that setting that attribute causes the one-pixel frame to become visible around the video. We would probably want to extend the video over that pixel rather than leave it visible, but I'm not immediately sure how to do that; it would probably involve something very similar to whatever ends up working for bug 1598312.