1832154 - Enable media.videocontrols.picture-in-picture.respect-disablePictureInPicture

Status: RESOLVED FIXED

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

Description

[Niklas Baumgardner [:niklas]]

Currently (when bug 1811321 lands) media.videocontrols.picture-in-picture.respect-disablePictureInPicture is enabled for Nightly only. We will want to enable for all

Comment 1

[Niklas Baumgardner [:niklas]]

Attachment: Bug 1832154 - Enable media.videocontrols.picture-in-picture.respect-disablePictureInPicture. r=#pip-reviewers

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/acb746bb1801
Enable media.videocontrols.picture-in-picture.respect-disablePictureInPicture. r=pip-reviewers,kpatenio

Comment 3

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/acb746bb1801

Comment 4

[Hamish Willee]

Hi, The property HTMLVideoElement.disablePictureInPicture is not showing up/passing the WPT live tests or the bcd collector test: https://mdn-bcd-collector.gooborg.com/tests/api/HTMLVideoElement/disablePictureInPicture

There is a comment here "While the flag mentioned in that PR is indeed enabled by default, the property is not defined in the WebIDL.". I can see in https://searchfox.org/mozilla-central/source/dom/webidl/HTMLVideoElement.webidl it appears as an "InstrumentedProps".

[Exposed=Window,
 InstrumentedProps=(cancelVideoFrameCallback,
               >>>>>>>>>>  disablePictureInPicture,
                    onenterpictureinpicture,
                    onleavepictureinpicture,
                    playsInline,
                    requestPictureInPicture,
                    requestVideoFrameCallback)]
interface HTMLVideoElement : HTMLMediaElement {
  [HTMLConstructor] constructor();
...

So the question is "should this work" and if so, what do we need to get our tests to show that it works?

Discussion follows from https://github.com/mdn/browser-compat-data/pull/21029

Comment 5

[Hamish Willee]

Ping! Can we please have a response to this ^^^. As far as we can tell disablePictureInPicture is NOT supported - according to WPT and bcd collector tests. If you can't confirm, the intent is to mark this as not supported.

Comment 6

[Hamish Willee]

OK. We have undone the indication that this feature is supported in compatibility data.

Comment 7

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

Hi, sorry about the delay, I was out for a couple days. As far as I can tell, we do intend to be passing this test. We'll file a separate bug to update the appropriate WebIDL, and I think that should take care of this.

Note though that we provide an implementation of the attribute behavior that is deliberately not spec-compliant, and we have no plans to change that. I would assume that the compatibility data needs to reflect this fact.

Comment 8

[Hamish Willee]

Note though that we provide an implementation of the attribute behavior that is deliberately not spec-compliant, and we have no plans to change that. I would assume that the compatibility data needs to reflect this fact.

Hi Molly,
The compatibility data was reverted to mention the version this was provided in. However it doesn't capture that the behaviour is non-spec-compliant. We could add this as a note if you can provide information about what it actually does that differs?

Comment 9

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

So, there's two things:

1. We implement the disablePictureInPicture attribute, but none of the rest of the PIP spec.
2. Our implementation of disablePictureInPicture does not fully disable picture-in-picture; we'll hide our usual button that overlays the video, but still allow the user to override.

Thanks!

Comment 10

[Hamish Willee]

Thanks @molly. The BCD team are asking the hard questions in https://github.com/mdn/browser-compat-data/pull/21263 about how this can work if it is undefined:

"HTMLVideoElement.prototype.disablePictureInPicture is undefined, so how is it supported? Is it simply that setting the property to true has an effect?"

Is that the case? i.e. setting the property, even though it is undefined initially, hides the overlay button?

That is what I assumed, but would be good to have it "said".

Comment 11

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

(In reply to Hamish Willee from comment #10)

Is that the case? i.e. setting the property, even though it is undefined initially, hides the overlay button?

That is exactly correct, yes.