[Modern Media Controls] Improve handling of <video> with only audio tracks
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Feb 2017 13:48:10 +0000 (13:48 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Feb 2017 13:48:10 +0000 (13:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=167836
<rdar://problem/30255812>

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

Source/WebCore:

We now check for the availability of video tracks before considering a <video>
element is displaying an actual video file and turning auto-hide on. We also
check that we have video tracks before enabling the fullscreen button. This
brings the behavior of a <video> pointing to a resource with no video tracks
to be the same as an <audio> element.

Test: media/modern-media-controls/media-controller/media-controller-video-with-only-audio.html

* Modules/modern-media-controls/media/controls-visibility-support.js:
(ControlsVisibilitySupport.prototype.get tracksToMonitor):
(ControlsVisibilitySupport.prototype._updateControls):
(ControlsVisibilitySupport):
* Modules/modern-media-controls/media/fullscreen-support.js:
(FullscreenSupport.prototype.syncControl):
(FullscreenSupport):

LayoutTests:

Add a new test to check that a <video> with a resource that only has audio tracks
does not auto-hide nor show the fullscreen button. We also rebaseline a few existing
tests for this change of behavior.

* fast/regions/inline-block-inside-anonymous-overflow-with-covered-controls-expected.html:
* fast/regions/inline-block-inside-anonymous-overflow-with-covered-controls.html:
* media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-on-expected.txt:
* media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-on.html:
* media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-toggle-expected.txt:
* media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-toggle.html:
* media/modern-media-controls/media-controller/media-controller-video-with-only-audio-expected.txt: Added.
* media/modern-media-controls/media-controller/media-controller-video-with-only-audio.html: Added.
* media/modern-media-controls/time-label/time-label-white-space-nowrap.html:

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/regions/inline-block-inside-anonymous-overflow-with-covered-controls-expected.html
LayoutTests/fast/regions/inline-block-inside-anonymous-overflow-with-covered-controls.html
LayoutTests/media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-on-expected.txt
LayoutTests/media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-on.html
LayoutTests/media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-toggle-expected.txt
LayoutTests/media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-toggle.html
LayoutTests/media/modern-media-controls/media-controller/media-controller-video-with-only-audio-expected.txt [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-controller/media-controller-video-with-only-audio.html [new file with mode: 0644]
LayoutTests/media/modern-media-controls/time-label/time-label-white-space-nowrap.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/modern-media-controls/media/controls-visibility-support.js
Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js

index a30671e..53cdf73 100644 (file)
@@ -1,3 +1,25 @@
+2017-02-06  Antoine Quint  <graouts@apple.com>
+
+        [Modern Media Controls] Improve handling of <video> with only audio tracks
+        https://bugs.webkit.org/show_bug.cgi?id=167836
+        <rdar://problem/30255812>
+
+        Reviewed by Dean Jackson.
+
+        Add a new test to check that a <video> with a resource that only has audio tracks
+        does not auto-hide nor show the fullscreen button. We also rebaseline a few existing
+        tests for this change of behavior.
+
+        * fast/regions/inline-block-inside-anonymous-overflow-with-covered-controls-expected.html:
+        * fast/regions/inline-block-inside-anonymous-overflow-with-covered-controls.html:
+        * media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-on-expected.txt:
+        * media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-on.html:
+        * media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-toggle-expected.txt:
+        * media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-toggle.html:
+        * media/modern-media-controls/media-controller/media-controller-video-with-only-audio-expected.txt: Added.
+        * media/modern-media-controls/media-controller/media-controller-video-with-only-audio.html: Added.
+        * media/modern-media-controls/time-label/time-label-white-space-nowrap.html:
+
 2017-02-05  Antoine Quint  <graouts@apple.com>
 
         [Modern Media Controls] PiP button is not visible with a live broadcast video
index f0c8003..9e7fcd3 100644 (file)
@@ -30,8 +30,8 @@
                 position: absolute;
                 display: block;
                 width: 350px;
-                height: 40px;
-                top: 350px;
+                height: 50px;
+                top: 335px;
                 left: 5px;
                 z-index: 1000;
                 background-color: black;
index 39590b4..f2bac65 100644 (file)
@@ -29,8 +29,8 @@
                 position: absolute;
                 display: block;
                 width: 350px;
-                height: 40px;
-                top: 350px;
+                height: 50px;
+                top: 335px;
                 left: 5px;
                 z-index: 1000;
                 background-color: black;
index ec2fb0b..0673eb3 100644 (file)
@@ -3,11 +3,9 @@ Testing the ControlsVisibilitySupport behavior without controls.
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-Media has not begun loading yet
-PASS mediaController.controls.controlsBar.visible is false
-PASS mediaController.controls.startButton.visible is false
 
 Media has loaded metadata
+PASS media.videoTracks.length is 1
 PASS mediaController.controls.controlsBar.visible is true
 PASS mediaController.controls.startButton.visible is true
 
index e42372f..e96aad4 100644 (file)
@@ -13,13 +13,10 @@ const shadowRoot = document.querySelector("div#shadow").attachShadow({ mode: "op
 const media = document.querySelector("video");
 const mediaController = createControls(shadowRoot, media, null);
 
-debug("Media has not begun loading yet");
-shouldBeFalse("mediaController.controls.controlsBar.visible");
-shouldBeFalse("mediaController.controls.startButton.visible");
-
 media.addEventListener("loadedmetadata", function() {
     debug("");
     debug("Media has loaded metadata");
+    shouldBe("media.videoTracks.length", "1");
     shouldBeTrue("mediaController.controls.controlsBar.visible");
     shouldBeTrue("mediaController.controls.startButton.visible");
     debug("");
index 6dc8b91..df827f6 100644 (file)
@@ -3,11 +3,9 @@ Testing the ControlsVisibilitySupport behavior when the controls attribute chang
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-Media has not begun loading yet
-PASS mediaController.controls.controlsBar.visible is false
-PASS mediaController.controls.startButton.visible is false
 
 Media has loaded metadata
+PASS media.videoTracks.length is 1
 PASS mediaController.controls.controlsBar.visible is true
 PASS mediaController.controls.startButton.visible is true
 
index efb1c50..b85fc79 100644 (file)
@@ -35,13 +35,10 @@ new MutationObserver(() => {
     }
 }).observe(media, { attributes: true, attributeFilter: ["controls"] });
 
-debug("Media has not begun loading yet");
-shouldBeFalse("mediaController.controls.controlsBar.visible");
-shouldBeFalse("mediaController.controls.startButton.visible");
-
 media.addEventListener("loadedmetadata", function() {
     debug("");
     debug("Media has loaded metadata");
+    shouldBe("media.videoTracks.length", "1");
     shouldBeTrue("mediaController.controls.controlsBar.visible");
     shouldBeTrue("mediaController.controls.startButton.visible");
 
diff --git a/LayoutTests/media/modern-media-controls/media-controller/media-controller-video-with-only-audio-expected.txt b/LayoutTests/media/modern-media-controls/media-controller/media-controller-video-with-only-audio-expected.txt
new file mode 100644 (file)
index 0000000..c9bf24c
--- /dev/null
@@ -0,0 +1,12 @@
+Testing that we only show appropriate controls for a video element with only audio tracks.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS mediaControls.querySelector('button.fullscreen') is null
+PASS mediaControls.querySelector('.controls-bar').classList.contains('faded') is false
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/media/modern-media-controls/media-controller/media-controller-video-with-only-audio.html b/LayoutTests/media/modern-media-controls/media-controller/media-controller-video-with-only-audio.html
new file mode 100644 (file)
index 0000000..a3ac4eb
--- /dev/null
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<script src="../../../resources/js-test-pre.js"></script>
+<body>
+<video src="../../content/test.wav" style="width: 800px; height: 240px;" controls autoplay data-auto-hide-delay="100"></video>
+<script type="text/javascript">
+
+window.jsTestIsAsync = true;
+
+description("Testing that we only show appropriate controls for a video element with only audio tracks.");
+
+const media = document.querySelector("video");
+const shadowRoot = window.internals.shadowRoot(media);
+let mediaControls;
+
+media.addEventListener("play", event => {
+    mediaControls = shadowRoot.lastElementChild.lastElementChild;
+
+    // We should not show the fullscreen button if we have a <video> with no audio tracks.
+    window.requestAnimationFrame(() => shouldBeNull("mediaControls.querySelector('button.fullscreen')"));
+
+    setTimeout(() => {
+        // Controls should not auto-hide if we have a <video> with no audio tracks.
+        shouldBeFalse("mediaControls.querySelector('.controls-bar').classList.contains('faded')");
+
+        debug("");
+        media.remove();
+        finishJSTest();
+    }, 250);
+});
+
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
index bf96da7..57e656f 100644 (file)
@@ -14,12 +14,14 @@ document.querySelector("video").addEventListener("play", (event) => {
     const media = event.target;
     media.pause();
 
-    timeLabel = window.internals.shadowRoot(media).querySelector(".time-label");
-    shouldBeEqualToString("timeLabel.ownerDocument.defaultView.getComputedStyle(timeLabel).whiteSpace", "nowrap");
+    window.requestAnimationFrame(() => {
+        timeLabel = window.internals.shadowRoot(media).querySelector(".time-label");
+        shouldBeEqualToString("timeLabel.ownerDocument.defaultView.getComputedStyle(timeLabel).whiteSpace", "nowrap");
 
-    debug("");
-    media.remove();
-    finishJSTest();
+        debug("");
+        media.remove();
+        finishJSTest();
+    });
 });
 
 </script>
index eda3953..00c49c6 100644 (file)
@@ -1,3 +1,27 @@
+2017-02-06  Antoine Quint  <graouts@apple.com>
+
+        [Modern Media Controls] Improve handling of <video> with only audio tracks
+        https://bugs.webkit.org/show_bug.cgi?id=167836
+        <rdar://problem/30255812>
+
+        Reviewed by Dean Jackson.
+
+        We now check for the availability of video tracks before considering a <video>
+        element is displaying an actual video file and turning auto-hide on. We also
+        check that we have video tracks before enabling the fullscreen button. This
+        brings the behavior of a <video> pointing to a resource with no video tracks
+        to be the same as an <audio> element.
+
+        Test: media/modern-media-controls/media-controller/media-controller-video-with-only-audio.html
+
+        * Modules/modern-media-controls/media/controls-visibility-support.js:
+        (ControlsVisibilitySupport.prototype.get tracksToMonitor):
+        (ControlsVisibilitySupport.prototype._updateControls):
+        (ControlsVisibilitySupport):
+        * Modules/modern-media-controls/media/fullscreen-support.js:
+        (FullscreenSupport.prototype.syncControl):
+        (FullscreenSupport):
+
 2017-02-06  Alex Christensen  <achristensen@webkit.org>
 
         Fix WinCairo build after r211681
index 64d39d5..b6e81f2 100644 (file)
@@ -48,6 +48,11 @@ class ControlsVisibilitySupport extends MediaControllerSupport
         return ["loadedmetadata", "play", "pause"];
     }
 
+    get tracksToMonitor()
+    {
+        return [this.mediaController.media.videoTracks];
+    }
+
     handleEvent()
     {
         this._updateControls();
@@ -58,7 +63,7 @@ class ControlsVisibilitySupport extends MediaControllerSupport
     _updateControls()
     {
         const media = this.mediaController.media;
-        const isVideo = media instanceof HTMLVideoElement;
+        const isVideo = media instanceof HTMLVideoElement && media.videoTracks.length > 0;
         let shouldShowControls = this.mediaController.media.controls;
         if (isVideo)
             shouldShowControls = shouldShowControls && media.readyState > HTMLMediaElement.HAVE_NOTHING;
index a18ae7a..5102401 100644 (file)
@@ -69,7 +69,7 @@ class FullscreenSupport extends MediaControllerSupport
     {
         const control = this.control;
         const media = this.mediaController.media;
-        control.enabled = media.webkitSupportsFullscreen;
+        control.enabled = media.webkitSupportsFullscreen && media.videoTracks.length > 0;
         control.isFullScreen = media.webkitDisplayingFullscreen;
     }