| Summary: | The PiP button on the fullscreen youtube player disappears after starting a new video in a playlist | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Peng Liu <peng.liu6> | ||||||
| Component: | Media | Assignee: | Peng Liu <peng.liu6> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | darin, eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | iPhone / iPad | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=215526 | ||||||||
| Attachments: |
|
||||||||
|
Description
Peng Liu
2020-08-13 20:25:01 PDT
Created attachment 406570 [details]
Patch
Created attachment 406577 [details]
update the change log
Comment on attachment 406577 [details]
update the change log
Great ChangeLog!
Committed r265690: <https://trac.webkit.org/changeset/265690> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406577 [details]. Comment on attachment 406577 [details] update the change log View in context: https://bugs.webkit.org/attachment.cgi?id=406577&action=review > Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:73 > - if (m_mediaElement == mediaElement) > + if (m_mediaElement == mediaElement) { > + if (m_mediaElement) { > + for (auto client : m_clients) > + client->isPictureInPictureSupportedChanged(isPictureInPictureSupported()); > + } > return; > + } Adding this guaranteed side effect of this function seems like a difficult-to-maintain oblique way to fix this, especially since there is no regression test. I’d like us to find a way to make a regression test. I’d also like to make this not depend on side effects like this. Requiring that setting something to an existing value has a side effect means the function should probably be named differently and is not a simple setter. So one solution would be to rename the function. Comment on attachment 406577 [details] update the change log View in context: https://bugs.webkit.org/attachment.cgi?id=406577&action=review >> Source/WebCore/platform/cocoa/PlaybackSessionModelMediaElement.mm:73 >> + } > > Adding this guaranteed side effect of this function seems like a difficult-to-maintain oblique way to fix this, especially since there is no regression test. > > I’d like us to find a way to make a regression test. > > I’d also like to make this not depend on side effects like this. Requiring that setting something to an existing value has a side effect means the function should probably be named differently and is not a simple setter. So one solution would be to rename the function. Agree. Filed bug 215526 to track the task. |