Bug 221083 – EnterPictureInPictureEvent event was renamed to PictureInPictureEvent in spec

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Philip Jägenstedt]

https://github.com/w3c/picture-in-picture/pull/189 renamed the EnterPictureInPictureEvent interface to just PictureInPictureEvent.

The event type is still "enterpictureinpicture" so doing this renaming should be very low risk.

There are some failures in WPT due to this:
https://wpt.fyi/results/picture-in-picture/idlharness.window.html?run_id=5142493001678848&run_id=5150525664264192&run_id=5705922615705600

[Peng Liu]

We also need to update "leavepictureinpicture" event based on the discussion in https://github.com/w3c/picture-in-picture/issues/188.

[Peng Liu]

https://github.com/w3c/picture-in-picture/issues/145

[Radar WebKit Bug Importer]

<rdar://problem/73971131>

[Peng Liu]

Created attachment 450256 [details]
Patch

[Peng Liu]

Created attachment 450269 [details]
Patch

[youenn fablet]

Comment on attachment 450269 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=450269&action=review

r=me, worth checking API test results though.


> Source/WebCore/ChangeLog:11
> +        (https://github.com/w3c/picture-in-picture/issues/188)

Not sure how much of this is tested in WPT tests but maybe it is already covered and we should just resync the corresponding WPT tests.

> Source/WebCore/ChangeLog:13
> +        Covered by media/picture-in-picture/picture-in-picture-api-events.html.

PictureInPictureEvent new name is not covered though.
Can we update LayoutTests/imported/w3c/web-platform-tests/picture-in-picture/ to take benefit of the IDL harness coverage?
I would hope the IDL harness coverage test to be runnable without any issue.

[youenn fablet]

Hum, I see webkit.org/b/202617 now.
Might be worth the effort.
I still wonder whether some of the tests can already be run today, say imported/w3c/web-platform-tests/picture-in-picture/idlharness.window.html for instance. Can you try removing some of the Skip test expectations and update the expected.txt files in this patch?

[Peng Liu]

(In reply to youenn fablet from comment #7)
> Hum, I see webkit.org/b/202617 now.
> Might be worth the effort.
> I still wonder whether some of the tests can already be run today, say
> imported/w3c/web-platform-tests/picture-in-picture/idlharness.window.html
> for instance. Can you try removing some of the Skip test expectations and
> update the expected.txt files in this patch?

Probably we can enable some tests now, but not all of them. The reason is that those tests "shares" the system picture-in-picture implementation and cannot execute in parallel. One possible solution would be enabling the MockVideoPresentationMode of the WKTR by default.

[Peng Liu]

Created attachment 454145 [details]
Rebase the patch

[Peng Liu]

Created attachment 454289 [details]
[fast-cq] Patch for landing

[EWS]

Committed r291144 (248304@main): <https://commits.webkit.org/248304@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454289 [details].