Bug 203493 – [Picture-in-Picture Web API] Support picture-in-picture CSS pseudo-class

• Component: Media
• Status: RESOLVED FIXED

[Peng Liu]

Test case failure: LayoutTests/imported/w3c/web-platform-tests/picture-in-picture/css-selector.html.

[Radar WebKit Bug Importer]

<rdar://problem/56685826>

[Peng Liu]

Created attachment 383248 [details]
Patch

[Peng Liu]

Created attachment 383252 [details]
Fix GTK build failure

[Dean Jackson]

Comment on attachment 383252 [details]
Fix GTK build failure

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

> LayoutTests/media/picture-in-picture/picture-in-picture-api-css-selector.html:16
> +        /* illegal selector list */
> +        video, :picture-in-picture(*) {
> +            color: rgb(255, 0, 0);
> +        }

I'm not sure exactly what this is testing, but you don't actually test to make sure it didn't happen.

I also suggest you add another test (probably a separate file) that exercises the is<HTMLVideoElement>(element) part of the logic. i.e. something like

div:picture-in-picture {
  background-color: red;
}

[Dean Jackson]

Note that WPT also has a picture-in-picture directory - so maybe we should import those. It includes a CSS selector test.

[Dean Jackson]

http://wpt.live/picture-in-picture/

https://github.com/web-platform-tests/wpt/tree/master/picture-in-picture

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 383252 [details]:

inspector/model/remote-object-weak-collection.html bug 202932 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 383252 [details]
Fix GTK build failure

Clearing flags on attachment: 383252

Committed r252330: <https://trac.webkit.org/changeset/252330>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Peng Liu]

Comment on attachment 383252 [details]
Fix GTK build failure

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

>> LayoutTests/media/picture-in-picture/picture-in-picture-api-css-selector.html:16
>> +        }
> 
> I'm not sure exactly what this is testing, but you don't actually test to make sure it didn't happen.
> 
> I also suggest you add another test (probably a separate file) that exercises the is<HTMLVideoElement>(element) part of the logic. i.e. something like
> 
> div:picture-in-picture {
>   background-color: red;
> }

A follow up task is reported: https://bugs.webkit.org/show_bug.cgi?id=204088

[Peng Liu]

(In reply to Dean Jackson from comment #5)
> Note that WPT also has a picture-in-picture directory - so maybe we should
> import those. It includes a CSS selector test.

The WPT cases for the picture-in-picture API are already imported, but they are disabled for now.