Bug 190359 - Enable external playback for video in element fullscreen.
Summary: Enable external playback for video in element fullscreen.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jeremy Jones
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-08 10:42 PDT by Jeremy Jones
Modified: 2018-11-16 16:49 PST (History)
4 users (show)

See Also:


Attachments
Patch (6.93 KB, patch)
2018-10-08 10:47 PDT, Jeremy Jones
no flags Details | Formatted Diff | Diff
Patch (7.77 KB, patch)
2018-10-08 12:38 PDT, Jeremy Jones
no flags Details | Formatted Diff | Diff
Patch (7.77 KB, patch)
2018-10-08 12:46 PDT, Jeremy Jones
no flags Details | Formatted Diff | Diff
Patch (7.85 KB, patch)
2018-10-08 13:00 PDT, Jeremy Jones
no flags Details | Formatted Diff | Diff
Patch (8.70 KB, patch)
2018-11-02 11:13 PDT, Jeremy Jones
no flags Details | Formatted Diff | Diff
Patch For Landing (8.82 KB, patch)
2018-11-05 09:50 PST, Jeremy Jones
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Jones 2018-10-08 10:42:01 PDT
Enable external playback for video in element fullscreen.
Comment 1 Jeremy Jones 2018-10-08 10:47:46 PDT
Created attachment 351791 [details]
Patch
Comment 2 Jeremy Jones 2018-10-08 10:48:26 PDT
rdar://problem/42560085
Comment 3 Jeremy Jones 2018-10-08 12:38:18 PDT
Created attachment 351802 [details]
Patch
Comment 4 Jeremy Jones 2018-10-08 12:46:35 PDT
Created attachment 351804 [details]
Patch
Comment 5 Jeremy Jones 2018-10-08 13:00:21 PDT
Created attachment 351808 [details]
Patch
Comment 6 Jeremy Jones 2018-11-02 11:13:45 PDT
Created attachment 353716 [details]
Patch
Comment 7 Jer Noble 2018-11-02 11:17:33 PDT
Comment on attachment 353716 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=353716&action=review

r=me with nits

> Source/WebCore/html/HTMLMediaElement.h:737
> +    bool mediaPlayerVideoFullscreenStandby() const final { return m_videoFullscreenStandby; }

This is a verb that wants to be a noun. Could we do mediaPlayerIsVideoFullscreenStandby()? (and in all the related declarations)

> Source/WebCore/platform/graphics/MediaPlayer.cpp:724
> +void MediaPlayer::setVideoFullscreenStandby(bool isStandby)
> +{
> +    m_private->setVideoFullscreenStandby(isStandby);
> +}

We pass isStandby in here but never use it. Could we rename this "videoFullscreenStandbyChanged()"?
Comment 8 Jeremy Jones 2018-11-02 13:32:36 PDT
Comment on attachment 353716 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=353716&action=review

>> Source/WebCore/html/HTMLMediaElement.h:737
>> +    bool mediaPlayerVideoFullscreenStandby() const final { return m_videoFullscreenStandby; }
> 
> This is a verb that wants to be a noun. Could we do mediaPlayerIsVideoFullscreenStandby()? (and in all the related declarations)

Done.

>> Source/WebCore/platform/graphics/MediaPlayer.cpp:724
>> +}
> 
> We pass isStandby in here but never use it. Could we rename this "videoFullscreenStandbyChanged()"?

Done.
Comment 9 Jeremy Jones 2018-11-05 09:50:29 PST
Created attachment 353882 [details]
Patch For Landing
Comment 10 WebKit Commit Bot 2018-11-16 16:49:58 PST
Comment on attachment 353882 [details]
Patch For Landing

Clearing flags on attachment: 353882

Committed r238327: <https://trac.webkit.org/changeset/238327>
Comment 11 WebKit Commit Bot 2018-11-16 16:49:59 PST
All reviewed patches have been landed.  Closing bug.