1598312 - It's hard to resize the Picture-in-Picture video window

Status: VERIFIED FIXED

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

Description

[(no longer active)]

It seems that the Picture-in-Picture window has a hit-area for resizing about one pixel wide around its edges, which makes it very tricky to resize it, especially diagonally. It would be nice if we could increase the hit area a bit to make it a bit easier to grab the window edges for resizing with a mouse/touchpad.

Comment 1

[tonank]

It is a lot easier in Chrome/Edge to do this.

Comment 2

[Micah [:mtigley] (she/her)]

To get started on this, we can look at this window utility called setChromeMargin. This allows us to adjust the chrome size on each side of the window.

This method utility should be available on the PiP window we open at: https://searchfox.org/mozilla-central/rev/a6db3bd67367aa9ddd9505690cab09b47e65a762/toolkit/components/pictureinpicture/PictureInPicture.jsm#410-416. I believe we'll want to call this setChromeMargin around this area of the code.

Comment 3

[Micah [:mtigley] (she/her)]

Updates on this:

It doesn't look like setChromeMargin is what we want here. This looks like it's going to require some fiddling with the platform code to get working:

We have 2 approaches for this:

1. The first looks at having a way to set the resizable margin from the WindowUtils. To do this, we'll need to do the following:

• Expose a method on WindowUtils called setResizableMargin. SetResizableMargin takes in margin values for changing the horizontal/vertical resizer margin: aHResizerSize and aVResizerSize.

• Create an implementation for SetResizableMargin like SetChromeMargin. This function will be responsible for calling a method on nsWindow that is responsible for modifying these values mHorResizeMargin and mVertResizeMargin with aHResizerSize and aVResizerSize, respectively.

• Create a method on nsWindow called SetWindowResizerSize. As described above, it sets mHorResizeMargin and mVertResizeMargin and then calls UpdateNonClientMargins to apply other "window-y" things it needs.

• Now we need a way to prevent UpdateNonClientMargins from overwriting what we set mHorResizeMargin and mVertResizeMargin to. Right now, we should do this by storing creating another member variables on nsWindow, perhaps called mUseResizeMarginOverrides, and use this to prevent executing this code.

After all this, we should be able to do something like: pipWindow.windowUtils.setWindowResizableMargin(x, y) to increase the resizer size.

2. The second approach looks at changing what min-resize border value we use.

• Create some local variable called "minResizableBorder" whose value is initialized to kResizableBorderMinSize.
• Do a check for whether or not the window is a PiP window. I believe this is currently available by checking mAlwaysOnTop in nsWindow.
  If we're using PiP, then reassigns minResizableBorder to some other pixel value that would increase the border size significantly.
• Replace uses of kResizableBorderMinSize with minResizableBorder here: https://searchfox.org/mozilla-central/rev/26330a08b1f9d06938faa0aa5e0f8c7a58064aa2/widget/windows/nsWindow.cpp#6419-6427

I believe the first approach is what we should be exploring first due to the fact we can reuse it for other feature windows and that it seems to be using the correct variables to ensure the resizer works on different desktop resolutions, plus any other calculations nsWindow needs.

Comment 4

[chenggu3]

Attachment: WIP: Bug 1598312 - increase resize margin for pip window. r=mtigley,mhowell

Comment 5

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

I poked at this a bit today, attempting approach #1 from Comment 3.

I ran into a bit of a roadblock after implementing SetWindowResizerSize on nsWindow.cpp: from nsDOMWindowUtils, it's not immediately clear how to get an nsWindow* in order to call nsWindow::SetWindowResizerSize. The companion method, SetNonClientMargins, is defined on nsIWidget, so maybe the right fix is to move SetWindowResizerSize down into the nsIWidget interface?

While I was thinking about all this, I took a look at the existing patch attached to this bug, and it looks like that's what the last person had tried--moving the SetWindowResizerSize declaration into nsIWidget, so that, in nsDOMWindowUtils, it would be possible to call it by using the same sequence used in nsDOMWindowUtils::SetChromeMargin. I noticed in the comments on the phabricator review, it was mentioned that this approach didn't work.

Should I maybe try the second suggested approach instead? Or I could try to debug exactly why the first approach might not be working?

Comment 6

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

:mtigley and I chatted about this on Slack today. Just mentioning here for visibility that I'm going to try to press on with the first approach, since that seems to be the best one, and I'll try to figure out why it isn't working.

Comment 7

[Molly Howell (she/her) (mostly inactive)]

Attachment: Bug 1598312 - Support setting a custom resize margin, and apply that to Picture-in-Picture windows. r=cmartin!,emilio!,kpatenio,mtigley

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/299312118fec
Support setting a custom resize margin, and apply that to Picture-in-Picture windows. r=cmartin,emilio

Comment 9

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/299312118fec

Comment 10

[Daniel Bodea [:danibodea]]

I can confirm this fix.
On Nighty v99.0a1 (2022-02-15) the edges and corners of the PiP window are much harde4r to grab than on Nightly v100.0a1 (2022-03-16)
The border that the user can grab to resize the PiP is considerably larger after the fix.

Furthermore, I believe this fix only applies to Windows since I could not observe a difference between the affected and fixed builds on Mac OS 11 and Ubuntu 20. Please revert the edit if incorrect.