Bug 203493 - [Picture-in-Picture Web API] Support picture-in-picture CSS pseudo-class
Summary: [Picture-in-Picture Web API] Support picture-in-picture CSS pseudo-class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Peng Liu
URL:
Keywords: InRadar
Depends on:
Blocks: 182688
  Show dependency treegraph
 
Reported: 2019-10-28 10:49 PDT by Peng Liu
Modified: 2019-11-11 16:00 PST (History)
15 users (show)

See Also:


Attachments
Patch (11.08 KB, patch)
2019-11-10 13:43 PST, Peng Liu
no flags Details | Formatted Diff | Diff
Fix GTK build failure (11.18 KB, patch)
2019-11-10 19:07 PST, Peng Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peng Liu 2019-10-28 10:49:33 PDT
Test case failure: LayoutTests/imported/w3c/web-platform-tests/picture-in-picture/css-selector.html.
Comment 1 Radar WebKit Bug Importer 2019-10-28 14:48:05 PDT
<rdar://problem/56685826>
Comment 2 Peng Liu 2019-11-10 13:43:18 PST
Created attachment 383248 [details]
Patch
Comment 3 Peng Liu 2019-11-10 19:07:00 PST
Created attachment 383252 [details]
Fix GTK build failure
Comment 4 Dean Jackson 2019-11-11 09:23:37 PST
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;
}
Comment 5 Dean Jackson 2019-11-11 09:25:09 PST
Note that WPT also has a picture-in-picture directory - so maybe we should import those. It includes a CSS selector test.
Comment 7 WebKit Commit Bot 2019-11-11 10:00:34 PST
The commit-queue encountered the following flaky tests while processing attachment 383252 [details]:

inspector/model/remote-object-weak-collection.html bug 202932 (authors: drousso@apple.com and joepeck@webkit.org)
The commit-queue is continuing to process your patch.
Comment 8 WebKit Commit Bot 2019-11-11 10:01:27 PST
Comment on attachment 383252 [details]
Fix GTK build failure

Clearing flags on attachment: 383252

Committed r252330: <https://trac.webkit.org/changeset/252330>
Comment 9 WebKit Commit Bot 2019-11-11 10:01:29 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Peng Liu 2019-11-11 15:57:46 PST
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
Comment 11 Peng Liu 2019-11-11 16:00:35 PST
(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.