Bug 215488

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: MediaAssignee: 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 Flags
Patch
none
update the change log none

Peng Liu
Reported 2020-08-13 20:25:01 PDT
The PiP button on the fullscreen youtube player disappears after starting a new video in a playlist
Attachments
Patch (3.30 KB, patch)
2020-08-13 21:13 PDT, Peng Liu 		no flags
DetailsFormatted DiffDiff
update the change log (3.34 KB, patch)
2020-08-14 00:15 PDT, Peng Liu 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-08-13 20:25:51 PDT
<rdar://problem/67039225>
Peng Liu
Comment 2 2020-08-13 21:13:57 PDT
Created attachment 406570 [details] Patch
Peng Liu
Comment 3 2020-08-14 00:15:54 PDT
Created attachment 406577 [details] update the change log
Eric Carlson
Comment 4 2020-08-14 11:33:28 PDT
Comment on attachment 406577 [details] update the change log Great ChangeLog!
EWS
Comment 5 2020-08-14 11:57:30 PDT
Committed r265690: <https://trac.webkit.org/changeset/265690> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406577 [details].
Darin Adler
Comment 6 2020-08-14 15:37:18 PDT
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.
Peng Liu
Comment 7 2020-08-14 16:36:07 PDT
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.
Note You need to log in before you can comment on or make changes to this bug.