<!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>[204534] 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/204534">204534</a></dd>
<dt>Author</dt> <dd>adachan@apple.com</dd>
<dt>Date</dt> <dd>2016-08-16 15:48:58 -0700 (Tue, 16 Aug 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Placeholder does not show the first time going into picture-in-picture on video without controls
https://bugs.webkit.org/show_bug.cgi?id=160868
Reviewed by Eric Carlson.
Source/WebCore:
Test: media/controls/pip-placeholder-without-video-controls.html
If the media controls script is not injected by the time webkitpresentationmodechanged
event fires, the script that updates the stylesheet to show the placeholder won't execute.
To fix this, doing one of the following would work:
- Ensure the shadow dom for the video is set up when we schedule the
webkitpresentationmodechanged event.
- Make sure the styles are set up correctly to show the placeholder if needed
when the Controller object (in mediaControlsApple.js) is created.
Doing both here following what we did for the wireless playback status.
* Modules/mediacontrols/mediaControlsApple.js:
(Controller):
Call updatePictureInPicturePlaceholder() so it updates the styles to show the
placeholder if needed.
(Controller.prototype.updatePictureInPicturePlaceholder):
Extract the logic that updates the placeholder into a separate method so it can
be called when we initialize Controller.
(Controller.prototype.handlePresentationModeChange):
Call updatePictureInPicturePlaceholder().
* Modules/mediacontrols/mediaControlsiOS.js:
(ControllerIOS.prototype.updatePictureInPicturePlaceholder):
Renamed from handlePresentationModeChange().
(ControllerIOS.prototype.handlePresentationModeChange): Deleted.
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::enterFullscreen):
Call configureMediaControls() which ensures the shadow root gets added if needed.
LayoutTests:
Test that a video without controls attribute does show after going into picture-in-picture.
* TestExpectations:
* media/controls/pip-placeholder-without-video-controls-expected.txt: Added.
* media/controls/pip-placeholder-without-video-controls.html: Added.
* platform/mac-wk2/TestExpectations:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsTestExpectations">trunk/LayoutTests/TestExpectations</a></li>
<li><a href="#trunkLayoutTestsplatformmacwk2TestExpectations">trunk/LayoutTests/platform/mac-wk2/TestExpectations</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreModulesmediacontrolsmediaControlsApplejs">trunk/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js</a></li>
<li><a href="#trunkSourceWebCoreModulesmediacontrolsmediaControlsiOSjs">trunk/Source/WebCore/Modules/mediacontrols/mediaControlsiOS.js</a></li>
<li><a href="#trunkSourceWebCorehtmlHTMLMediaElementcpp">trunk/Source/WebCore/html/HTMLMediaElement.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsmediacontrolspipplaceholderwithoutvideocontrolsexpectedtxt">trunk/LayoutTests/media/controls/pip-placeholder-without-video-controls-expected.txt</a></li>
<li><a href="#trunkLayoutTestsmediacontrolspipplaceholderwithoutvideocontrolshtml">trunk/LayoutTests/media/controls/pip-placeholder-without-video-controls.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (204533 => 204534)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-08-16 22:44:21 UTC (rev 204533)
+++ trunk/LayoutTests/ChangeLog 2016-08-16 22:48:58 UTC (rev 204534)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2016-08-15 Ada Chan <adachan@apple.com>
+
+ Placeholder does not show the first time going into picture-in-picture on video without controls
+ https://bugs.webkit.org/show_bug.cgi?id=160868
+
+ Reviewed by Eric Carlson.
+
+ Test that a video without controls attribute does show after going into picture-in-picture.
+
+ * TestExpectations:
+ * media/controls/pip-placeholder-without-video-controls-expected.txt: Added.
+ * media/controls/pip-placeholder-without-video-controls.html: Added.
+ * platform/mac-wk2/TestExpectations:
+
</ins><span class="cx"> 2016-08-16 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><span class="cx"> DOM4: getElementsByClassName should include non StyledElements
</span></span></pre></div>
<a id="trunkLayoutTestsTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/TestExpectations (204533 => 204534)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/TestExpectations 2016-08-16 22:44:21 UTC (rev 204533)
+++ trunk/LayoutTests/TestExpectations 2016-08-16 22:48:58 UTC (rev 204534)
</span><span class="lines">@@ -1002,6 +1002,7 @@
</span><span class="cx"> # PiP tests are only relevant on macOS Sierra and newer.
</span><span class="cx"> media/click-placeholder-not-pausing.html [ WontFix ]
</span><span class="cx"> media/controls/picture-in-picture.html [ WontFix ]
</span><ins>+media/controls/pip-placeholder-without-video-controls.html [ WontFix ]
</ins><span class="cx"> media/element-containing-pip-video-going-into-fullscreen.html [ WontFix ]
</span><span class="cx"> media/fullscreen-api-enabled-media-with-presentation-mode.html [ WontFix ]
</span><span class="cx"> media/fullscreen-video-going-into-pip.html [ WontFix ]
</span></span></pre></div>
<a id="trunkLayoutTestsmediacontrolspipplaceholderwithoutvideocontrolsexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/media/controls/pip-placeholder-without-video-controls-expected.txt (0 => 204534)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/media/controls/pip-placeholder-without-video-controls-expected.txt (rev 0)
+++ trunk/LayoutTests/media/controls/pip-placeholder-without-video-controls-expected.txt 2016-08-16 22:48:58 UTC (rev 204534)
</span><span class="lines">@@ -0,0 +1,16 @@
</span><ins>+This tests the picture-in-picture placeholder when the video has no controls.
+
+This test only runs in DRT!
+
+
+EVENT: canplaythrough
+
+Test the picture-in-picture button with valid video
+
+EVENT: webkitpresentationmodechanged
+PASS: Should be in pip mode
+PASS: Inline placeholder should be visible at this point
+PASS: Inline placeholder should have the 'picture-in-picture' class
+
+Testing finished.
+
</ins></span></pre></div>
<a id="trunkLayoutTestsmediacontrolspipplaceholderwithoutvideocontrolshtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/media/controls/pip-placeholder-without-video-controls.html (0 => 204534)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/media/controls/pip-placeholder-without-video-controls.html (rev 0)
+++ trunk/LayoutTests/media/controls/pip-placeholder-without-video-controls.html 2016-08-16 22:48:58 UTC (rev 204534)
</span><span class="lines">@@ -0,0 +1,78 @@
</span><ins>+<!DOCTYPE html>
+<html>
+ <head>
+ <script src="../media-file.js"></script>
+ <script src="controls-test-helpers.js"></script>
+ <script>
+ var maxAttemptsToTestPlaceholderVisibility = 10;
+ var attemptsToTestPlaceholderVisibility = 0;
+
+ if (window.internals)
+ window.internals.settings.setAllowsPictureInPictureMediaPlayback(true);
+
+ const tester = new ControlsTest("../content/test", "canplaythrough")
+ .whenReady(runTestsWithVideo)
+ .start();
+
+ function runTestsWithVideo()
+ {
+ tester.startNewSection("Test the picture-in-picture button with valid video");
+
+ internals.setMediaElementRestrictions(tester.media, "NoRestrictions");
+ tester.resetEventTrigger("webkitpresentationmodechanged")
+ .whenReady(presentationModeDidChange)
+ .media.webkitSetPresentationMode("picture-in-picture");
+ }
+
+ function presentationModeDidChange()
+ {
+ tester.test("Should be in pip mode")
+ .value(tester.media.webkitPresentationMode)
+ .isEqualTo("picture-in-picture");
+
+ tester.media.removeEventListener("webkitpresentationmodechanged", tester, false);
+
+ pollPIPPlaceholderVisibilityChange();
+ }
+
+ function pollPIPPlaceholderVisibilityChange()
+ {
+ // Intentionally using internals.shadowRoot instead of tester.stateForControlsElement()
+ // since the latter has the side effect of ensuring the shadow root and hence injecting
+ // the media controls script. The bug we are testing here is caused by the media controls
+ // script not being injected early enough.
+ var shadowRoot = internals.shadowRoot(tester.media);
+ var placeholder = shadowRoot ? shadowRoot.querySelector("div[pseudo='-webkit-media-controls-wireless-playback-status']") : null;
+ if (!placeholder || placeholder.className.indexOf("hidden") >= 0) {
+ if (attemptsToTestPlaceholderVisibility > maxAttemptsToTestPlaceholderVisibility) {
+ tester.logFailure("Inline placeholder did not become visible after video enters picture-in-picture.");
+ tester.end();
+ return;
+ }
+
+ // Use 33 to match PlaceholderPollingDelay in mediaControlsApple.js.
+ setTimeout(pollPIPPlaceholderVisibilityChange, 33);
+ attemptsToTestPlaceholderVisibility++;
+ return;
+ }
+
+ tester.test("Inline placeholder should be visible at this point")
+ .value(placeholder.className)
+ .doesNotContain("hidden");
+
+ tester.test("Inline placeholder should have the 'picture-in-picture' class")
+ .value(placeholder.className)
+ .contains("picture-in-picture");
+
+ tester.media.webkitSetPresentationMode("inline");
+
+ tester.end();
+ }
+ </script>
+ </head>
+ <body>
+ <p>This tests the picture-in-picture placeholder when the video has no controls.</p>
+ <p>This test only runs in DRT!</p>
+ <video></video>
+ </body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsplatformmacwk2TestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (204533 => 204534)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/platform/mac-wk2/TestExpectations 2016-08-16 22:44:21 UTC (rev 204533)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations 2016-08-16 22:48:58 UTC (rev 204534)
</span><span class="lines">@@ -473,6 +473,7 @@
</span><span class="cx"> # PiP tests are only enabled for Sierra WebKit2.
</span><span class="cx"> # rdar://problem/27574303
</span><span class="cx"> [ Sierra+ ] media/controls/picture-in-picture.html [ Pass ]
</span><ins>+[ Sierra+ ] media/controls/pip-placeholder-without-video-controls.html [ Pass ]
</ins><span class="cx"> [ Sierra+ ] media/element-containing-pip-video-going-into-fullscreen.html [ Pass ]
</span><span class="cx"> [ Sierra+ ] media/fullscreen-api-enabled-media-with-presentation-mode.html [ Pass ]
</span><span class="cx"> [ Sierra+ ] media/fullscreen-video-going-into-pip.html [ Pass ]
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (204533 => 204534)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-08-16 22:44:21 UTC (rev 204533)
+++ trunk/Source/WebCore/ChangeLog 2016-08-16 22:48:58 UTC (rev 204534)
</span><span class="lines">@@ -1,3 +1,40 @@
</span><ins>+2016-08-15 Ada Chan <adachan@apple.com>
+
+ Placeholder does not show the first time going into picture-in-picture on video without controls
+ https://bugs.webkit.org/show_bug.cgi?id=160868
+
+ Reviewed by Eric Carlson.
+
+ Test: media/controls/pip-placeholder-without-video-controls.html
+
+ If the media controls script is not injected by the time webkitpresentationmodechanged
+ event fires, the script that updates the stylesheet to show the placeholder won't execute.
+
+ To fix this, doing one of the following would work:
+ - Ensure the shadow dom for the video is set up when we schedule the
+ webkitpresentationmodechanged event.
+ - Make sure the styles are set up correctly to show the placeholder if needed
+ when the Controller object (in mediaControlsApple.js) is created.
+
+ Doing both here following what we did for the wireless playback status.
+
+ * Modules/mediacontrols/mediaControlsApple.js:
+ (Controller):
+ Call updatePictureInPicturePlaceholder() so it updates the styles to show the
+ placeholder if needed.
+ (Controller.prototype.updatePictureInPicturePlaceholder):
+ Extract the logic that updates the placeholder into a separate method so it can
+ be called when we initialize Controller.
+ (Controller.prototype.handlePresentationModeChange):
+ Call updatePictureInPicturePlaceholder().
+ * Modules/mediacontrols/mediaControlsiOS.js:
+ (ControllerIOS.prototype.updatePictureInPicturePlaceholder):
+ Renamed from handlePresentationModeChange().
+ (ControllerIOS.prototype.handlePresentationModeChange): Deleted.
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::enterFullscreen):
+ Call configureMediaControls() which ensures the shadow root gets added if needed.
+
</ins><span class="cx"> 2016-08-16 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Rename FrameView::m_layoutDisallowed to m_layoutDisallowedCount
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesmediacontrolsmediaControlsApplejs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js (204533 => 204534)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js 2016-08-16 22:44:21 UTC (rev 204533)
+++ trunk/Source/WebCore/Modules/mediacontrols/mediaControlsApple.js 2016-08-16 22:48:58 UTC (rev 204534)
</span><span class="lines">@@ -42,6 +42,7 @@
</span><span class="cx"> this.updateHasVideo();
</span><span class="cx"> this.updateWirelessTargetAvailable();
</span><span class="cx"> this.updateWirelessPlaybackStatus();
</span><ins>+ this.updatePictureInPicturePlaceholder();
</ins><span class="cx"> this.scheduleUpdateLayoutForDisplayedWidth();
</span><span class="cx">
</span><span class="cx"> this.listenFor(this.root, 'resize', this.handleRootResize);
</span><span class="lines">@@ -915,7 +916,7 @@
</span><span class="cx"> return presentationMode === 'inline' || presentationMode === 'fullscreen';
</span><span class="cx"> },
</span><span class="cx">
</span><del>- handlePresentationModeChange: function(event)
</del><ins>+ updatePictureInPicturePlaceholder: function()
</ins><span class="cx"> {
</span><span class="cx"> var presentationMode = this.presentationMode();
</span><span class="cx">
</span><span class="lines">@@ -951,11 +952,15 @@
</span><span class="cx"> this.controls.pictureInPictureButton.classList.remove(this.ClassNames.returnFromPictureInPicture);
</span><span class="cx"> break;
</span><span class="cx"> }
</span><ins>+ },
</ins><span class="cx">
</span><ins>+ handlePresentationModeChange: function(event)
+ {
+ this.updatePictureInPicturePlaceholder();
</ins><span class="cx"> this.updateControls();
</span><span class="cx"> this.updateCaptionContainer();
</span><span class="cx"> this.resetHideControlsTimer();
</span><del>- if (presentationMode != 'fullscreen' && this.video.paused && this.controlsAreHidden())
</del><ins>+ if (this.presentationMode() != 'fullscreen' && this.video.paused && this.controlsAreHidden())
</ins><span class="cx"> this.showControls();
</span><span class="cx"> this.host.setPreparedToReturnVideoLayerToInline(this.shouldReturnVideoLayerToInline());
</span><span class="cx"> },
</span></span></pre></div>
<a id="trunkSourceWebCoreModulesmediacontrolsmediaControlsiOSjs"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/mediacontrols/mediaControlsiOS.js (204533 => 204534)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/mediacontrols/mediaControlsiOS.js 2016-08-16 22:44:21 UTC (rev 204533)
+++ trunk/Source/WebCore/Modules/mediacontrols/mediaControlsiOS.js 2016-08-16 22:48:58 UTC (rev 204534)
</span><span class="lines">@@ -541,7 +541,7 @@
</span><span class="cx"> return this.presentationMode() === 'inline';
</span><span class="cx"> },
</span><span class="cx">
</span><del>- handlePresentationModeChange: function(event)
</del><ins>+ updatePictureInPicturePlaceholder: function(event)
</ins><span class="cx"> {
</span><span class="cx"> var presentationMode = this.presentationMode();
</span><span class="cx">
</span><span class="lines">@@ -557,7 +557,7 @@
</span><span class="cx"> break;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- Controller.prototype.handlePresentationModeChange.call(this, event);
</del><ins>+ Controller.prototype.updatePictureInPicturePlaceholder.call(this, event);
</ins><span class="cx"> },
</span><span class="cx">
</span><span class="cx"> // Due to the bad way we are faking inheritance here, in particular the extends method
</span></span></pre></div>
<a id="trunkSourceWebCorehtmlHTMLMediaElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (204533 => 204534)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/html/HTMLMediaElement.cpp 2016-08-16 22:44:21 UTC (rev 204533)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp 2016-08-16 22:48:58 UTC (rev 204534)
</span><span class="lines">@@ -5415,6 +5415,7 @@
</span><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx"> fullscreenModeChanged(mode);
</span><ins>+ configureMediaControls();
</ins><span class="cx"> if (hasMediaControls())
</span><span class="cx"> mediaControls()->enteredFullscreen();
</span><span class="cx"> if (document().page() && is<HTMLVideoElement>(*this)) {
</span></span></pre>
</div>
</div>
</body>
</html>