RESOLVED FIXED 238579
Add a heuristic to identify and extract the prominent video element in element fullscreen 
https://bugs.webkit.org/show_bug.cgi?id=238579
Summary Add a heuristic to identify and extract the prominent video element in elemen...
Wenson Hsieh
Reported 2022-03-30 16:10:03 PDT
.
Attachments
Patch (13.11 KB, patch)
2022-03-30 17:05 PDT, Wenson Hsieh 		no flags
DetailsFormatted DiffDiff
Patch (10.42 KB, patch)
2022-03-30 21:41 PDT, Wenson Hsieh 		no flags
DetailsFormatted DiffDiff
Patch (13.23 KB, patch)
2022-03-31 07:45 PDT, Wenson Hsieh 		jer.noble: review+
DetailsFormatted DiffDiff
Patch for landing (13.40 KB, patch)
2022-03-31 14:41 PDT, Wenson Hsieh 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Wenson Hsieh
Comment 1 2022-03-30 17:05:40 PDT Comment hidden (obsolete)
Wenson Hsieh
Comment 2 2022-03-30 21:41:31 PDT Comment hidden (obsolete)
Wenson Hsieh
Comment 3 2022-03-31 07:45:54 PDT
Created attachment 456235 [details] Patch
Jer Noble
Comment 4 2022-03-31 14:16:04 PDT
Comment on attachment 456235 [details] Patch Nit: it appears this heuristic depends on the video element already existing in the page when fullscreen mode is entered. You may want consider a mutation observer, or listening for events bubbled up to the fullscreen element itself (like 'loadstart', 'play' or 'pause') to trigger recalculation of this heuristic.
Wenson Hsieh
Comment 5 2022-03-31 14:27:32 PDT
Thanks for the review! (In reply to Jer Noble from comment #4) > Comment on attachment 456235 [details] > Patch > > Nit: it appears this heuristic depends on the video element already existing > in the page when fullscreen mode is entered. You may want consider a > mutation observer, or listening for events bubbled up to the fullscreen > element itself (like 'loadstart', 'play' or 'pause') to trigger > recalculation of this heuristic. Indeed! That is an omission from this iteration of the heuristic (as well as the case where m_mainVideoElement is unparented while in element fullscreen mode, and replaced with a new video element). I was initially planning to address this in a future patch by overriding HTMLMediaElement's tree insertion/removal method hooks (as well as other media-related methods), but I think the approach of listening for media-related events bubbled up to the fullscreen element is a lot more elegant — I'll go with that approach, and add a FIXME as well.
Wenson Hsieh
Comment 6 2022-03-31 14:41:22 PDT
Created attachment 456284 [details] Patch for landing
EWS
Comment 7 2022-03-31 22:53:59 PDT
Committed r292205 (249108@main): <https://commits.webkit.org/249108@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 456284 [details].
Radar WebKit Bug Importer
Comment 8 2022-03-31 22:55:15 PDT
<rdar://problem/91146510>
Note You need to log in before you can comment on or make changes to this bug.