<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[211687] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/211687">211687</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2017-02-05 12:55:21 -0800 (Sun, 05 Feb 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>[Modern Media Controls] PiP button is not visible with a live broadcast video
https://bugs.webkit.org/show_bug.cgi?id=167794
<rdar://problem/30348790>
Patch by Antoine Quint <graouts@apple.com> on 2017-02-05
Reviewed by Dean Jackson.
Source/WebCore:
We were only listening to the "loadedmetadata", "error", "webkitpresentationmodechanged"
and "webkitcurrentplaybacktargetiswirelesschanged" events to invalidate the enabled state
for the PiP button. We also need to check availability of video tracks, which we already
did for fullscreen, which is quite similar.
So we now listen to "addtrack", "removetrack" and "change" events on the media.videoTracks
property, which correctly invalidates the PiP button when the first video track becomes
available or the last video track is removed.
Since a couple of other MediaControllerSupport subclasses (FullscreenSupport and TracksSupport)
would also listen to those events on various track types, we add a new "tracksToMonitor"
property on MediaControllerSupport which subclasses can override to provide a list of tracks
that should listen to those events. This removes the need for dedicated construction and
destruction time in MediaControllerSupport subclasses that need to listen to events on
tracks rather than the media itself.
Test: http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html
* Modules/modern-media-controls/media/fullscreen-support.js:
(FullscreenSupport):
(FullscreenSupport.prototype.get tracksToMonitor):
(FullscreenSupport.prototype.destroy): Deleted.
* Modules/modern-media-controls/media/media-controller-support.js:
(MediaControllerSupport):
(MediaControllerSupport.prototype.destroy):
(MediaControllerSupport.prototype.get tracksToMonitor):
* Modules/modern-media-controls/media/pip-support.js:
(PiPSupport.prototype.get tracksToMonitor):
* Modules/modern-media-controls/media/tracks-support.js:
(TracksSupport):
(TracksSupport.prototype.get tracksToMonitor):
(TracksSupport.prototype.destroy): Deleted.
LayoutTests:
Add a new test to check that a live broadcast video shows the picture-in-picture button.
* http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast-expected.txt: Added.
* http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html: Added.
* platform/mac/TestExpectations:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsplatformmacTestExpectations">trunk/LayoutTests/platform/mac/TestExpectations</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreModulesmodernmediacontrolsmediafullscreensupportjs">trunk/Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js</a></li>
<li><a href="#trunkSourceWebCoreModulesmodernmediacontrolsmediamediacontrollersupportjs">trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller-support.js</a></li>
<li><a href="#trunkSourceWebCoreModulesmodernmediacontrolsmediapipsupportjs">trunk/Source/WebCore/Modules/modern-media-controls/media/pip-support.js</a></li>
<li><a href="#trunkSourceWebCoreModulesmodernmediacontrolsmediatrackssupportjs">trunk/Source/WebCore/Modules/modern-media-controls/media/tracks-support.js</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li>trunk/LayoutTests/http/tests/media/modern-media-controls/pip-support/</li>
<li><a href="#trunkLayoutTestshttptestsmediamodernmediacontrolspipsupportpipsupportlivebroadcastexpectedtxt">trunk/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast-expected.txt</a></li>
<li><a href="#trunkLayoutTestshttptestsmediamodernmediacontrolspipsupportpipsupportlivebroadcasthtml">trunk/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (211686 => 211687)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2017-02-05 20:41:56 UTC (rev 211686)
+++ trunk/LayoutTests/ChangeLog 2017-02-05 20:55:21 UTC (rev 211687)
</span><span class="lines">@@ -1,5 +1,19 @@
</span><span class="cx"> 2017-02-05 Antoine Quint <graouts@apple.com>
</span><span class="cx">
</span><ins>+ [Modern Media Controls] PiP button is not visible with a live broadcast video
+ https://bugs.webkit.org/show_bug.cgi?id=167794
+ <rdar://problem/30348790>
+
+ Reviewed by Dean Jackson.
+
+ Add a new test to check that a live broadcast video shows the picture-in-picture button.
+
+ * http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast-expected.txt: Added.
+ * http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html: Added.
+ * platform/mac/TestExpectations:
+
+2017-02-05 Antoine Quint <graouts@apple.com>
+
</ins><span class="cx"> [Modern Media Controls] Time labels may wrap instead of displaying on a single line
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=167835
</span><span class="cx"> <rdar://problem/30340534>
</span></span></pre></div>
<a id="trunkLayoutTestshttptestsmediamodernmediacontrolspipsupportpipsupportlivebroadcastexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast-expected.txt (0 => 211687)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast-expected.txt 2017-02-05 20:55:21 UTC (rev 211687)
</span><span class="lines">@@ -0,0 +1,12 @@
</span><ins>+Testing the PiPSupport behavior with live broadcast.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS media.duration is Number.POSITIVE_INFINITY
+PASS mediaController.controls.pipButton.enabled is true
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestshttptestsmediamodernmediacontrolspipsupportpipsupportlivebroadcasthtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html (0 => 211687)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html (rev 0)
+++ trunk/LayoutTests/http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html 2017-02-05 20:55:21 UTC (rev 211687)
</span><span class="lines">@@ -0,0 +1,31 @@
</span><ins>+<script src="../../../../../resources/js-test-pre.js"></script>
+<script src="/media-resources/modern-media-controls/resources/media-controls-loader.js"></script>
+<body>
+<video src="../../resources/hls/test-live.php" style="width: 800px;" autoplay></video>
+<div id="shadow"></div>
+<script type="text/javascript">
+
+window.jsTestIsAsync = true;
+
+description("Testing the <code>PiPSupport</code> behavior with live broadcast.");
+
+const shadowRoot = document.querySelector("div#shadow").attachShadow({ mode: "open" });
+const media = document.querySelector("video");
+const mediaController = createControls(shadowRoot, media, null);
+
+if (window.internals)
+ window.internals.settings.setAllowsPictureInPictureMediaPlayback(true);
+
+media.addEventListener("play", event => {
+ shouldBe("media.duration", "Number.POSITIVE_INFINITY");
+ shouldBeTrue("mediaController.controls.pipButton.enabled");
+
+ debug("");
+ shadowRoot.host.remove();
+ media.remove();
+ finishJSTest();
+});
+
+</script>
+<script src="../../../../../resources/js-test-post.js"></script>
+</body>
</ins></span></pre></div>
<a id="trunkLayoutTestsplatformmacTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/mac/TestExpectations (211686 => 211687)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/platform/mac/TestExpectations 2017-02-05 20:41:56 UTC (rev 211686)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2017-02-05 20:55:21 UTC (rev 211687)
</span><span class="lines">@@ -1457,6 +1457,7 @@
</span><span class="cx">
</span><span class="cx"> # picture-in-picture is not supported prior to Sierra.
</span><span class="cx"> [ ElCapitan ] media/modern-media-controls/pip-support [ Skip ]
</span><ins>+[ ElCapitan ] http/tests/media/modern-media-controls/pip-support [ Skip ]
</ins><span class="cx"> [ ElCapitan ] media/modern-media-controls/media-controller/media-controller-inline-to-fullscreen-to-pip-to-inline.html [ Skip ]
</span><span class="cx"> [ ElCapitan ] media/modern-media-controls/placard-support/placard-support-pip.html [ Skip ]
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (211686 => 211687)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2017-02-05 20:41:56 UTC (rev 211686)
+++ trunk/Source/WebCore/ChangeLog 2017-02-05 20:55:21 UTC (rev 211687)
</span><span class="lines">@@ -1,5 +1,46 @@
</span><span class="cx"> 2017-02-05 Antoine Quint <graouts@apple.com>
</span><span class="cx">
</span><ins>+ [Modern Media Controls] PiP button is not visible with a live broadcast video
+ https://bugs.webkit.org/show_bug.cgi?id=167794
+ <rdar://problem/30348790>
+
+ Reviewed by Dean Jackson.
+
+ We were only listening to the "loadedmetadata", "error", "webkitpresentationmodechanged"
+ and "webkitcurrentplaybacktargetiswirelesschanged" events to invalidate the enabled state
+ for the PiP button. We also need to check availability of video tracks, which we already
+ did for fullscreen, which is quite similar.
+
+ So we now listen to "addtrack", "removetrack" and "change" events on the media.videoTracks
+ property, which correctly invalidates the PiP button when the first video track becomes
+ available or the last video track is removed.
+
+ Since a couple of other MediaControllerSupport subclasses (FullscreenSupport and TracksSupport)
+ would also listen to those events on various track types, we add a new "tracksToMonitor"
+ property on MediaControllerSupport which subclasses can override to provide a list of tracks
+ that should listen to those events. This removes the need for dedicated construction and
+ destruction time in MediaControllerSupport subclasses that need to listen to events on
+ tracks rather than the media itself.
+
+ Test: http/tests/media/modern-media-controls/pip-support/pip-support-live-broadcast.html
+
+ * Modules/modern-media-controls/media/fullscreen-support.js:
+ (FullscreenSupport):
+ (FullscreenSupport.prototype.get tracksToMonitor):
+ (FullscreenSupport.prototype.destroy): Deleted.
+ * Modules/modern-media-controls/media/media-controller-support.js:
+ (MediaControllerSupport):
+ (MediaControllerSupport.prototype.destroy):
+ (MediaControllerSupport.prototype.get tracksToMonitor):
+ * Modules/modern-media-controls/media/pip-support.js:
+ (PiPSupport.prototype.get tracksToMonitor):
+ * Modules/modern-media-controls/media/tracks-support.js:
+ (TracksSupport):
+ (TracksSupport.prototype.get tracksToMonitor):
+ (TracksSupport.prototype.destroy): Deleted.
+
+2017-02-05 Antoine Quint <graouts@apple.com>
+
</ins><span class="cx"> [Modern Media Controls] Time labels may wrap instead of displaying on a single line
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=167835
</span><span class="cx"> <rdar://problem/30340534>
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesmodernmediacontrolsmediafullscreensupportjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js (211686 => 211687)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js 2017-02-05 20:41:56 UTC (rev 211686)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js 2017-02-05 20:55:21 UTC (rev 211687)
</span><span class="lines">@@ -32,23 +32,8 @@
</span><span class="cx">
</span><span class="cx"> if (mediaController.controls instanceof IOSInlineMediaControls)
</span><span class="cx"> mediaController.controls.delegate = this;
</span><del>-
- const videoTracks = mediaController.media.videoTracks;
- for (let eventType of ["change", "addtrack", "removetrack"])
- videoTracks.addEventListener(eventType, this);
</del><span class="cx"> }
</span><span class="cx">
</span><del>- // Public
-
- destroy()
- {
- super.destroy();
-
- const videoTracks = this.mediaController.media.videoTracks;
- for (let eventType of ["change", "addtrack", "removetrack"])
- videoTracks.removeEventListener(eventType, this);
- }
-
</del><span class="cx"> // Protected
</span><span class="cx">
</span><span class="cx"> get control()
</span><span class="lines">@@ -61,6 +46,11 @@
</span><span class="cx"> return ["loadedmetadata", "error"];
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ get tracksToMonitor()
+ {
+ return [this.mediaController.media.videoTracks];
+ }
+
</ins><span class="cx"> buttonWasPressed(control)
</span><span class="cx"> {
</span><span class="cx"> const media = this.mediaController.media;
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesmodernmediacontrolsmediamediacontrollersupportjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller-support.js (211686 => 211687)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller-support.js 2017-02-05 20:41:56 UTC (rev 211686)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller-support.js 2017-02-05 20:55:21 UTC (rev 211687)
</span><span class="lines">@@ -33,6 +33,11 @@
</span><span class="cx"> for (let eventType of this.mediaEvents)
</span><span class="cx"> mediaController.media.addEventListener(eventType, this);
</span><span class="cx">
</span><ins>+ for (let tracks of this.tracksToMonitor) {
+ for (let eventType of ["change", "addtrack", "removetrack"])
+ tracks.addEventListener(eventType, this);
+ }
+
</ins><span class="cx"> if (!this.control)
</span><span class="cx"> return;
</span><span class="cx">
</span><span class="lines">@@ -49,6 +54,11 @@
</span><span class="cx"> for (let eventType of this.mediaEvents)
</span><span class="cx"> media.removeEventListener(eventType, this);
</span><span class="cx">
</span><ins>+ for (let tracks of this.tracksToMonitor) {
+ for (let eventType of ["change", "addtrack", "removetrack"])
+ tracks.removeEventListener(eventType, this);
+ }
+
</ins><span class="cx"> if (this.control)
</span><span class="cx"> this.control.uiDelegate = null;
</span><span class="cx"> }
</span><span class="lines">@@ -66,6 +76,12 @@
</span><span class="cx"> return [];
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ get tracksToMonitor()
+ {
+ // Implemented by subclasses.
+ return [];
+ }
+
</ins><span class="cx"> buttonWasPressed(control)
</span><span class="cx"> {
</span><span class="cx"> // Implemented by subclasses.
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesmodernmediacontrolsmediapipsupportjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/pip-support.js (211686 => 211687)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/modern-media-controls/media/pip-support.js 2017-02-05 20:41:56 UTC (rev 211686)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/pip-support.js 2017-02-05 20:55:21 UTC (rev 211687)
</span><span class="lines">@@ -41,6 +41,11 @@
</span><span class="cx"> return ["loadedmetadata", "error", "webkitpresentationmodechanged", "webkitcurrentplaybacktargetiswirelesschanged"];
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ get tracksToMonitor()
+ {
+ return [this.mediaController.media.videoTracks];
+ }
+
</ins><span class="cx"> buttonWasPressed(control)
</span><span class="cx"> {
</span><span class="cx"> const media = this.mediaController.media;
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesmodernmediacontrolsmediatrackssupportjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/tracks-support.js (211686 => 211687)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/modern-media-controls/media/tracks-support.js 2017-02-05 20:41:56 UTC (rev 211686)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/tracks-support.js 2017-02-05 20:55:21 UTC (rev 211687)
</span><span class="lines">@@ -35,27 +35,8 @@
</span><span class="cx">
</span><span class="cx"> this.mediaController.controls.tracksPanel.dataSource = this;
</span><span class="cx"> this.mediaController.controls.tracksPanel.uiDelegate = this;
</span><del>-
- const media = mediaController.media;
- for (let tracks of [media.audioTracks, media.textTracks]) {
- for (let eventType of ["addtrack", "change", "removetrack"])
- tracks.addEventListener(eventType, this);
- }
</del><span class="cx"> }
</span><span class="cx">
</span><del>- // Public
-
- destroy()
- {
- super.destroy();
-
- const media = this.mediaController.media;
- for (let tracks of [media.audioTracks, media.textTracks]) {
- for (let eventType of ["addtrack", "change", "removetrack"])
- tracks.removeEventListener(eventType, this);
- }
- }
-
</del><span class="cx"> // Protected
</span><span class="cx">
</span><span class="cx"> get control()
</span><span class="lines">@@ -68,6 +49,11 @@
</span><span class="cx"> return ["loadedmetadata"];
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ get tracksToMonitor()
+ {
+ return [this.mediaController.media.audioTracks, this.mediaController.media.textTracks];
+ }
+
</ins><span class="cx"> buttonWasPressed(control)
</span><span class="cx"> {
</span><span class="cx"> this.mediaController.controls.showTracksPanel();
</span></span></pre>
</div>
</div>
</body>
</html>