| Summary: | A PiP window is closed when the video element is removed from DOM | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Peng Liu <peng.liu6> | ||||||||||
| Component: | Media | Assignee: | Peng Liu <peng.liu6> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | calvaris, cdumez, changseok, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, kondapallykalyan, pdr, philipj, sergio, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=232125 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Peng Liu
2020-08-17 16:31:41 PDT
Created attachment 406854 [details]
Patch
Created attachment 406858 [details]
fix build failures
Created attachment 406859 [details]
fix build failures again
Comment on attachment 406859 [details] fix build failures again View in context: https://bugs.webkit.org/attachment.cgi?id=406859&action=review > Source/WebCore/html/HTMLMediaElement.cpp:4931 > + if (m_videoFullscreenMode == VideoFullscreenModePictureInPicture) > + return true; > + It took me a minute to figure out why this is necessary, so I think it would be useful to have a short comment here. > LayoutTests/media/remove-video-element-in-pip-from-document.html:31 > + setTimeout(endTest, 4000); We shouldn't use a timeout to end tests if at all possible. Four seconds is a very long time to wait for most test runs, but will probably be too short on a very heavily loaded bot. Luckily, I think you can do what you need with a MutationObserver instead. Created attachment 406871 [details]
patch for landing
Comment on attachment 406859 [details] fix build failures again View in context: https://bugs.webkit.org/attachment.cgi?id=406859&action=review >> Source/WebCore/html/HTMLMediaElement.cpp:4931 >> + > > It took me a minute to figure out why this is necessary, so I think it would be useful to have a short comment here. Good suggestion! Fixed. >> LayoutTests/media/remove-video-element-in-pip-from-document.html:31 >> + setTimeout(endTest, 4000); > > We shouldn't use a timeout to end tests if at all possible. Four seconds is a very long time to wait for most test runs, but will probably be too short on a very heavily loaded bot. > > Luckily, I think you can do what you need with a MutationObserver instead. Fixed. The new patch uses a MutationObserver to observe the event that the video element has been removed from DOM, and set up a one-second timer to end the test. Committed r265904: <https://trac.webkit.org/changeset/265904> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406871 [details]. |