Turn tests at media/modern-media-controls/start-support back on
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Jul 2017 20:47:53 +0000 (20:47 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Jul 2017 20:47:53 +0000 (20:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174683

Patch by Antoine Quint <graouts@apple.com> on 2017-07-20
Reviewed by Dean Jackson.

Source/WebCore:

Turning those tests back on revealed a small bug that is unlikely to really affect content
on the Web. In the case where the size of the video is known right away, without the need
for loading its metadata, as is the case in the start-support-click-to-start.html test with
a local media resource, all queued layouts are flushed at once and we may call the layout()
method of the left ButtonsContainer which originally is set to contain the play/pause button,
which would remove the play/pause button from the center of the media. So before we potentially
set the play/pause button as the central button, we first assign the default button set for
the two ButtonsContainer instances and only add the play/pause button when we're not showing
the prominent play/pause button.

* Modules/modern-media-controls/controls/inline-media-controls.js:
(InlineMediaControls.prototype.layout):
(InlineMediaControls.prototype._leftContainerButtons):

LayoutTests:

* media/modern-media-controls/start-support/start-support-click-to-start-expected.txt:
* media/modern-media-controls/start-support/start-support-click-to-start.html:
* media/modern-media-controls/start-support/start-support-fullscreen.html:
* media/modern-media-controls/start-support/start-support-lowPowerMode-expected.txt:
* media/modern-media-controls/start-support/start-support-lowPowerMode.html:
* platform/ios-simulator/TestExpectations:
* platform/mac/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219700 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/media/modern-media-controls/start-support/start-support-click-to-start-expected.txt
LayoutTests/media/modern-media-controls/start-support/start-support-click-to-start.html
LayoutTests/media/modern-media-controls/start-support/start-support-fullscreen.html
LayoutTests/media/modern-media-controls/start-support/start-support-lowPowerMode-expected.txt
LayoutTests/media/modern-media-controls/start-support/start-support-lowPowerMode.html
LayoutTests/platform/ios-simulator/TestExpectations
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/Modules/modern-media-controls/controls/inline-media-controls.js

index 1806f77..63064d9 100644 (file)
@@ -1,3 +1,18 @@
+2017-07-20  Antoine Quint  <graouts@apple.com>
+
+        Turn tests at media/modern-media-controls/start-support back on
+        https://bugs.webkit.org/show_bug.cgi?id=174683
+
+        Reviewed by Dean Jackson.
+
+        * media/modern-media-controls/start-support/start-support-click-to-start-expected.txt:
+        * media/modern-media-controls/start-support/start-support-click-to-start.html:
+        * media/modern-media-controls/start-support/start-support-fullscreen.html:
+        * media/modern-media-controls/start-support/start-support-lowPowerMode-expected.txt:
+        * media/modern-media-controls/start-support/start-support-lowPowerMode.html:
+        * platform/ios-simulator/TestExpectations:
+        * platform/mac/TestExpectations:
+
 2017-07-20  Matt Lewis  <jlewis3@apple.com>
 
         Marked media/modern-media-controls/scrubber-support/scrubber-support-drag.html as flaky
index 0c45eec..2cd2783 100644 (file)
@@ -6,6 +6,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 PASS mediaController.controls.showsStartButton is true
 
 Pressing on the start button
+PASS mediaController.controls.playPauseButton.element.getBoundingClientRect().width became different from 0
 Media is playing
 PASS mediaController.controls.showsStartButton is false
 
index 958539c..dd8abd3 100644 (file)
@@ -2,30 +2,43 @@
 <script src="../resources/media-controls-loader.js" type="text/javascript"></script>
 <script src="../resources/media-controls-utils.js" type="text/javascript"></script>
 <body>
-<video src="../../content/test.mp4" style="width: 320px; height: 240px;" controls></video>
-<div id="shadow"></div>
+<style type="text/css" media="screen">
+    
+    video, #host {
+        position: absolute;
+        top: 0;
+        left: 0;
+        width: 800px;
+        height: 600px;
+    }
+    
+</style>
+<video src="../../content/test.mp4" controls></video>
+<div id="host"></div>
 <script type="text/javascript">
 
 window.jsTestIsAsync = true;
 
 description("Testing the <code>MediaController</code> click-to-start behavior.");
 
-const shadowRoot = document.querySelector("div#shadow").attachShadow({ mode: "open" });
+const host = document.querySelector("div#host");
 const media = document.querySelector("video");
-const mediaController = createControls(shadowRoot, media, null);
+const mediaController = createControls(host, media, null);
 
 media.addEventListener("loadedmetadata", function() {
     shouldBeTrue("mediaController.controls.showsStartButton");
     debug("");
     debug("Pressing on the start button");
-    window.requestAnimationFrame(() => pressOnElement(mediaController.controls.startButton.element));
+    shouldBecomeDifferent("mediaController.controls.playPauseButton.element.getBoundingClientRect().width", "0", () => {
+        pressOnElement(mediaController.controls.playPauseButton.element)
+    });
 });
 
 media.addEventListener("play", function() {
     debug("Media is playing");
     shouldBeFalse("mediaController.controls.showsStartButton");
     debug("");
-    shadowRoot.host.remove();
+    host.remove();
     media.remove();
     finishJSTest();
 });
index b2d999f..75e8731 100644 (file)
@@ -1,19 +1,32 @@
 <script src="../../../resources/js-test-pre.js"></script>
 <script src="../resources/media-controls-loader.js" type="text/javascript"></script>
+<script src="../resources/media-controls-utils.js" type="text/javascript"></script>
 <body>
-<video src="../../content/test.mp4" style="width: 320px; height: 240px;"></video>
-<div id="shadow"></div>
+<style type="text/css" media="screen">
+    
+    video, #host {
+        position: absolute;
+        top: 0;
+        left: 0;
+        width: 320px;
+        height: 240px;
+    }
+    
+</style>
+<video src="../../content/test.mp4"></video>
+<div id="host"></div>
 <script type="text/javascript">
 
 window.jsTestIsAsync = true;
 
 description("Testing the <code>StartSupport</code> behavior with no source.");
 
-const shadowRoot = document.querySelector("div#shadow").attachShadow({ mode: "open" });
+const host = document.querySelector("div#host");
 const media = document.querySelector("video");
-const mediaController = createControls(shadowRoot, media, null);
+const mediaController = createControls(host, media, null);
 
-const button = document.body.appendChild(document.createElement("button"));
+const button = document.body.appendChild(document.createElement("div"));
+button.textContent = "Enter Fullscreen";
 button.addEventListener("click", event => {
     try {
         media.webkitEnterFullscreen();
@@ -27,21 +40,13 @@ media.addEventListener("webkitfullscreenchange", function() {
     debug("Media entered fullscreen");
     shouldBeFalse("mediaController.controls.showsStartButton");
     debug("");
-    shadowRoot.host.remove();
+    host.remove();
     media.remove();
     button.remove();
     finishJSTest();
 });
 
-media.addEventListener("loadedmetadata", event => {
-    if ("eventSender" in window) {
-        // Click the button so that we may enter fullscreen.
-        eventSender.mouseMoveTo(button.offsetLeft + 1, button.offsetTop + 1);
-        eventSender.mouseDown();
-        eventSender.mouseUp();
-    } else
-        debug("This test is designed to run in DRT");
-});
+media.addEventListener("loadedmetadata", event => pressOnElement(button));
 
 </script>
 <script src="../../../resources/js-test-post.js"></script>
index 88886c5..9d31466 100644 (file)
@@ -4,12 +4,12 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 Received 'canplaythrough' event
-PASS !!internals.shadowRoot(media).querySelector('button.start') became true
+PASS !!internals.shadowRoot(media).querySelector('button.play-pause.center') became true
 PASS media.controls is false
 Pressing on the start button
 Received 'play' event
 PASS media.controls is false
-PASS internals.shadowRoot(media).querySelector('button.start') became null
+PASS internals.shadowRoot(media).querySelector('button.play-pause.center') became null
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 6e6b898..58e17f1 100644 (file)
@@ -26,12 +26,12 @@ function endTest()
 media.addEventListener("canplaythrough", function() {
     debug("Received 'canplaythrough' event");
     // We should display the start button since we denied autoplay and the user needs a way to start playback.
-    shouldBecomeEqual("!!internals.shadowRoot(media).querySelector('button.start')", "true", function() {
+    shouldBecomeEqual("!!internals.shadowRoot(media).querySelector('button.play-pause.center')", "true", function() {
         shouldBeFalse("media.controls");
 
         debug("Pressing on the start button");
         hasUserGesture = true;
-        pressOnElement(internals.shadowRoot(media).querySelector('button.start'));
+        pressOnElement(internals.shadowRoot(media).querySelector('button.play-pause.center'));
     });
 });
 
@@ -39,7 +39,7 @@ media.addEventListener("play", function() {
     debug("Received 'play' event");
     shouldBeFalse("media.controls");
     if (hasUserGesture) {
-        shouldBecomeEqual("internals.shadowRoot(media).querySelector('button.start')", "null", endTest);
+        shouldBecomeEqual("internals.shadowRoot(media).querySelector('button.play-pause.center')", "null", endTest);
     } else {
         testFailed("Media started playing without user interaction");
         endTest();
index da4e8b9..78a7e1d 100644 (file)
@@ -100,6 +100,7 @@ media/modern-media-controls/scrubber-support [ Pass ]
 media/modern-media-controls/skip-back-button [ Pass ]
 media/modern-media-controls/skip-forward-button [ Pass ]
 media/modern-media-controls/slider [ Pass ]
+media/modern-media-controls/start-support [ Pass ]
 media/modern-media-controls/status-label [ Pass ]
 media/modern-media-controls/time-control [ Pass ]
 media/modern-media-controls/time-label [ Pass ]
@@ -118,12 +119,14 @@ media/modern-media-controls/mute-support/mute-support-press-on-button.html [ Ski
 media/modern-media-controls/pip-support/ipad/pip-support-tap.html [ Skip ]
 media/modern-media-controls/placard-support/ipad [ Skip ]
 media/modern-media-controls/scrubber-support/ipad/scrubber-support-drag.html [ Skip ]
+media/modern-media-controls/start-support/start-support-click-to-start.html [ Skip ]
 
 # These tests rely on fullscreen which do not use the WebKit media controls on iOS
 media/modern-media-controls/controls-visibility-support/controls-visibility-support-fullscreen-on-parent-element.html [ Skip ]
 media/modern-media-controls/controls-visibility-support/controls-visibility-support-fullscreen-on-video.html [ Skip ]
 media/modern-media-controls/css/webkit-cursor-visibility-auto-hide.html [ Skip ]
 media/modern-media-controls/placard-support/placard-support-airplay-fullscreen.html [ Skip ]
+media/modern-media-controls/start-support/start-support-fullscreen.html [ Skip ]
 
 # These tests specifically test iOS-only media controls features
 media/modern-media-controls/ios-inline-media-controls/ios-inline-media-controls-button-padding.html [ Pass ]
index d31a94f..38f1472 100644 (file)
@@ -1581,6 +1581,7 @@ media/modern-media-controls/scrubber-support [ Pass ]
 media/modern-media-controls/skip-back-button [ Pass ]
 media/modern-media-controls/skip-forward-button [ Pass ]
 media/modern-media-controls/slider [ Pass ]
+media/modern-media-controls/start-support [ Pass ]
 media/modern-media-controls/status-label [ Pass ]
 media/modern-media-controls/time-control [ Pass ]
 media/modern-media-controls/time-label [ Pass ]
index f424a37..e8db0bb 100644 (file)
@@ -1,3 +1,24 @@
+2017-07-20  Antoine Quint  <graouts@apple.com>
+
+        Turn tests at media/modern-media-controls/start-support back on
+        https://bugs.webkit.org/show_bug.cgi?id=174683
+
+        Reviewed by Dean Jackson.
+
+        Turning those tests back on revealed a small bug that is unlikely to really affect content
+        on the Web. In the case where the size of the video is known right away, without the need
+        for loading its metadata, as is the case in the start-support-click-to-start.html test with
+        a local media resource, all queued layouts are flushed at once and we may call the layout()
+        method of the left ButtonsContainer which originally is set to contain the play/pause button,
+        which would remove the play/pause button from the center of the media. So before we potentially
+        set the play/pause button as the central button, we first assign the default button set for
+        the two ButtonsContainer instances and only add the play/pause button when we're not showing
+        the prominent play/pause button.
+
+        * Modules/modern-media-controls/controls/inline-media-controls.js:
+        (InlineMediaControls.prototype.layout):
+        (InlineMediaControls.prototype._leftContainerButtons):
+
 2017-07-20  Chris Dumez  <cdumez@apple.com>
 
         Drop legacy FileException type
index 3bae7a9..22530c7 100644 (file)
@@ -118,6 +118,12 @@ class InlineMediaControls extends MediaControls
             return;
         }
 
+        if (!this.bottomControlsBar)
+            return;
+
+        this.leftContainer.buttons = this._leftContainerButtons();
+        this.rightContainer.buttons = this._rightContainerButtons();
+
         // If we should show the start button, then only show that button.
         if (this._showsStartButton) {
             this.playPauseButton.style = this.width <= MaximumSizeToShowSmallProminentControl || this.height <= MaximumSizeToShowSmallProminentControl ? Button.Styles.SmallCenter : Button.Styles.Center;
@@ -125,9 +131,6 @@ class InlineMediaControls extends MediaControls
             return;
         }
 
-        if (!this.bottomControlsBar)
-            return;
-
         // Update the top left controls bar.
         this._topLeftControlsBarContainer.buttons = this._topLeftContainerButtons();
         this._topLeftControlsBarContainer.layout();
@@ -160,8 +163,6 @@ class InlineMediaControls extends MediaControls
         // Iterate through controls to see if we need to drop any of them. Reset all default states before we proceed.
         this.bottomControlsBar.visible = true;
         this.playPauseButton.style = Button.Styles.Bar;
-        this.leftContainer.buttons = this._leftContainerButtons();
-        this.rightContainer.buttons = this._rightContainerButtons();
         this.rightContainer.buttons.concat(this.leftContainer.buttons).forEach(button => delete button.dropped);
         this.muteButton.style = this.preferredMuteButtonStyle;
         this.muteButton.usesRTLIconVariant = false;
@@ -247,6 +248,8 @@ class InlineMediaControls extends MediaControls
 
     _leftContainerButtons()
     {
+        if (this._showsStartButton)
+            return [this.skipBackButton, this.skipForwardButton];
         return [this.skipBackButton, this.playPauseButton, this.skipForwardButton];
     }