Bug 167794 - [Modern Media Controls] PiP button is not visible with a live broadcast video
Summary: [Modern Media Controls] PiP button is not visible with a live broadcast video
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-02-03 08:29 PST by Antoine Quint
Modified: 2017-02-05 12:56 PST (History)
2 users (show)

See Also:


Attachments
Testcase (257 bytes, text/html)
2017-02-03 08:29 PST, Antoine Quint
no flags Details
Patch (11.89 KB, patch)
2017-02-04 05:15 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch for landing (11.89 KB, patch)
2017-02-05 12:17 PST, Antoine Quint
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2017-02-03 08:29:16 PST
Created attachment 300534 [details]
Testcase

We're not showing the PiP button for a live broadcast video.
Comment 1 Radar WebKit Bug Importer 2017-02-03 08:29:38 PST
<rdar://problem/30348790>
Comment 2 Antoine Quint 2017-02-03 09:11:57 PST
The PiPSupport class (Source/WebCore/Modules/modern-media-controls/media/pip-support.js) listens to several events to update the enabled state of the PiP control: "loadedmetadata", "error", "webkitpresentationmodechanged" and "webkitcurrentplaybacktargetiswirelesschanged". As those events are dispatched, the following method is called:

    syncControl()
    {
        const media = this.mediaController.media;
        if (media.webkitSupportsPresentationMode)
            this.control.enabled = media instanceof HTMLVideoElement && media.webkitSupportsPresentationMode(PiPMode) && !media.webkitCurrentPlaybackTargetIsWireless;
        else
            this.control.enabled = false;
    }

As the testcase loads, we get three calls:

1. one from the constructor (this happens to set some initial state prior to any event firing):
    media instanceof HTMLVideoElement = true
    media.webkitSupportsPresentationMode(PiPMode) = false
    media.webkitCurrentPlaybackTargetIsWireless = false

2. the "loadedmetadata" event fires:
    media instanceof HTMLVideoElement = true
    media.webkitSupportsPresentationMode(PiPMode) = false
    media.webkitCurrentPlaybackTargetIsWireless = false

3. the "webkitcurrentplaybacktargetiswirelesschanged" event fires:
    media instanceof HTMLVideoElement = true
    media.webkitSupportsPresentationMode(PiPMode) = false
    media.webkitCurrentPlaybackTargetIsWireless = false

So in all calls we eventually set "enabled" to false due to "media.webkitSupportsPresentationMode(PiPMode)" being false. Is there some other event we should be using that would determine that a live broadcast video has PiP enabled?
Comment 3 Antoine Quint 2017-02-03 09:14:10 PST
By contrast, loading a local video yields the same call sequence but "media.webkitSupportsPresentationMode(PiPMode)" is true in both the "loadedmetadata" and "webkitcurrentplaybacktargetiswirelesschanged" event handlers.
Comment 4 Antoine Quint 2017-02-04 03:48:52 PST
The issue is that we're not taking into account addition (and removal) of video tracks. We do this for FullscreenSupport, we'll just do the same with PiPSupport, likely with a shared superclass.
Comment 5 Antoine Quint 2017-02-04 05:15:57 PST
Created attachment 300621 [details]
Patch
Comment 6 Dean Jackson 2017-02-05 12:07:31 PST
Comment on attachment 300621 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        and "webkitcurrentplaybacktargetiswirelesschanged" events to invaidate the enabled state

Typo: invalidate

> Source/WebCore/ChangeLog:18
> +        Since a couple other MediaControllerSupport subclasses (FullscreenSupport and TracksSupport)

Nit: a couple of other
Comment 7 Antoine Quint 2017-02-05 12:17:37 PST
Created attachment 300674 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2017-02-05 12:56:51 PST
Comment on attachment 300674 [details]
Patch for landing

Clearing flags on attachment: 300674

Committed r211687: <http://trac.webkit.org/changeset/211687>
Comment 9 WebKit Commit Bot 2017-02-05 12:56:54 PST
All reviewed patches have been landed.  Closing bug.