Bug 203493

Summary: [Picture-in-Picture Web API] Support picture-in-picture CSS pseudo-class
Product: WebKit Reporter: Peng Liu <peng.liu6>
Component: MediaAssignee: Peng Liu <peng.liu6>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, commit-queue, dino, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, jonlee, macpherson, menard, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 182688    
Attachments:
Description Flags
Patch
none
Fix GTK build failure none

Peng Liu
Reported 2019-10-28 10:49:33 PDT
Test case failure: LayoutTests/imported/w3c/web-platform-tests/picture-in-picture/css-selector.html.
Attachments
Patch (11.08 KB, patch)
2019-11-10 13:43 PST, Peng Liu 		no flags
DetailsFormatted DiffDiff
Fix GTK build failure (11.18 KB, patch)
2019-11-10 19:07 PST, Peng Liu 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2019-10-28 14:48:05 PDT
<rdar://problem/56685826>
Peng Liu
Comment 2 2019-11-10 13:43:18 PST
Created attachment 383248 [details] Patch
Peng Liu
Comment 3 2019-11-10 19:07:00 PST
Created attachment 383252 [details] Fix GTK build failure
Dean Jackson
Comment 4 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; }
Dean Jackson
Comment 5 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.
WebKit Commit Bot
Comment 7 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.
WebKit Commit Bot
Comment 8 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>
WebKit Commit Bot
Comment 9 2019-11-11 10:01:29 PST
All reviewed patches have been landed. Closing bug.
Peng Liu
Comment 10 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
Peng Liu
Comment 11 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.
Note You need to log in before you can comment on or make changes to this bug.