[Modern Media Controls] Improve media documents across macOS, iPhone and iPad
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Mar 2017 07:34:18 +0000 (07:34 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Mar 2017 07:34:18 +0000 (07:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=169145
<rdar://problem/17048858>

Reviewed by Dean Jackson.

Source/WebCore:

There were a variety of issues with media documents, some longstanding, and some specifically
about modern media controls.

One issue was that fullscreen and picture-in-picture buttons would show for audio media documents,
due to using a <video> element to load the audio file. We now have additional logic in MediaController
to identify if the loaded media is really an audio file, and using this information to hide the
fullscreen and picture-in-picture buttons.

Another issue was that we would inject style in MediaDocument.cpp that was specific to modern media
controls when we could have the modern-media-controls module injected CSS handle this styling. We now
use the injected style in the shadow root to size media documents based on the device characteristics
and ensuring that page styles are overridden.

We also simplify how MediaDocument.cpp sets the source of the media element to simply use the "src"
attribute and not a <source> element.

Finally, we introduce a MediaDocumentController class that is instantiated when we're dealing with
a media document to hide the controls while we determine the type of media we're loading (audio vs.
video) in order to apply the appropriate styling without flashes.

As a result of the new styles applied by the modern-media-controls module, media documents have a
similar behavior on macOS and iPad, where we only enforce a min-width for video allowing them
to play at their natural size otherwise, and enforcing a fixed width for audio. On iPhone however,
we want to always play the media at full width, with some padding in the case of audio.

Tests: media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only.html
       media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing.html
       media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing.html
       media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html
       media/modern-media-controls/media-documents/media-document-audio-mac-sizing.html
       media/modern-media-controls/media-documents/media-document-video-ios-sizing.html
       media/modern-media-controls/media-documents/media-document-video-mac-sizing.html
       media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only.html

* Modules/modern-media-controls/controls/ios-inline-media-controls.css:
(:host(audio) .media-controls.ios.inline > .controls-bar:before,):
(:host(audio) .media-controls.ios.inline > .controls-bar:before): Deleted.
* Modules/modern-media-controls/controls/macos-media-controls.css:
(:host(audio) .media-controls.mac.inline > .controls-bar,):
(:host(audio) .media-controls.mac.inline > .controls-bar > .background-tint,):
(:host(audio) .media-controls.mac.inline > .controls-bar): Deleted.
(:host(audio) .media-controls.mac.inline > .controls-bar > .background-tint): Deleted.
* Modules/modern-media-controls/controls/media-document.css: Copied from Source/WebCore/Modules/modern-media-controls/controls/macos-media-controls.css.
(:host(.media-document)):
(:host(.media-document.ready)):
(:host(.media-document.audio.mac)):
(:host(.media-document.audio.ipad)):
(:host(.media-document.audio.iphone)):
(:host(.media-document.video.mac)):
(:host(.media-document.video.ipad)):
(:host(.media-document.video.iphone)):
* Modules/modern-media-controls/js-files:
* Modules/modern-media-controls/media/fullscreen-support.js:
(FullscreenSupport.prototype.syncControl):
(FullscreenSupport):
* Modules/modern-media-controls/media/media-controller.js:
(MediaController):
(MediaController.prototype.get isAudio):
* Modules/modern-media-controls/media/media-document-controller.js: Copied from Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js.
(MediaDocumentController):
(MediaDocumentController.prototype.handleEvent):
(MediaDocumentController.prototype._mediaDocumentHasMetadata):
(MediaDocumentController.prototype._mediaDocumentHasSize):
* Modules/modern-media-controls/media/pip-support.js:
(PiPSupport.prototype.syncControl):
(PiPSupport):
* html/MediaDocument.cpp:
(WebCore::MediaDocumentParser::createDocumentStructure):

LayoutTests:

We add new tests for media documents and related features that cover the following cases:

    - checking <video> with only audio tracks does not show the fullscreen button
    - checking <video> with only audio tracks does not show the picture-in-picture button
    - checking the size used in media documents for audio and video across macOS, iPhone and iPad

* media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only-expected.txt: Added.
* media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only.html: Added.
* media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing-expected.txt: Added.
* media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing.html: Added.
* media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing-expected.txt: Added.
* media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing.html: Added.
* media/modern-media-controls/media-documents/media-document-audio-ios-sizing-expected.txt: Added.
* media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html: Added.
* media/modern-media-controls/media-documents/media-document-audio-mac-sizing-expected.txt: Added.
* media/modern-media-controls/media-documents/media-document-audio-mac-sizing.html: Added.
* media/modern-media-controls/media-documents/media-document-video-ios-sizing-expected.txt: Added.
* media/modern-media-controls/media-documents/media-document-video-ios-sizing.html: Added.
* media/modern-media-controls/media-documents/media-document-video-mac-sizing-expected.txt: Added.
* media/modern-media-controls/media-documents/media-document-video-mac-sizing.html: Added.
* media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only-expected.txt: Added.
* media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only.html: Added.
* platform/ios-simulator/TestExpectations:
* platform/mac/TestExpectations:

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

29 files changed:
LayoutTests/ChangeLog
LayoutTests/media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only-expected.txt [new file with mode: 0644]
LayoutTests/media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only.html [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing-expected.txt [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing.html [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing-expected.txt [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing.html [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/media-document-audio-ios-sizing-expected.txt [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/media-document-audio-mac-sizing-expected.txt [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/media-document-audio-mac-sizing.html [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/media-document-video-ios-sizing-expected.txt [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/media-document-video-ios-sizing.html [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/media-document-video-mac-sizing-expected.txt [new file with mode: 0644]
LayoutTests/media/modern-media-controls/media-documents/media-document-video-mac-sizing.html [new file with mode: 0644]
LayoutTests/media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only-expected.txt [new file with mode: 0644]
LayoutTests/media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only.html [new file with mode: 0644]
LayoutTests/platform/ios-simulator/TestExpectations
LayoutTests/platform/mac/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/Modules/modern-media-controls/controls/ios-inline-media-controls.css
Source/WebCore/Modules/modern-media-controls/controls/macos-media-controls.css
Source/WebCore/Modules/modern-media-controls/controls/media-document.css [new file with mode: 0644]
Source/WebCore/Modules/modern-media-controls/js-files
Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js
Source/WebCore/Modules/modern-media-controls/media/media-controller.js
Source/WebCore/Modules/modern-media-controls/media/media-document-controller.js [new file with mode: 0644]
Source/WebCore/Modules/modern-media-controls/media/pip-support.js
Source/WebCore/html/MediaDocument.cpp

index 4ebb766..3409bcc 100644 (file)
@@ -1,3 +1,36 @@
+2017-03-28  Antoine Quint  <graouts@apple.com>
+
+        [Modern Media Controls] Improve media documents across macOS, iPhone and iPad
+        https://bugs.webkit.org/show_bug.cgi?id=169145
+        <rdar://problem/17048858>
+
+        Reviewed by Dean Jackson.
+
+        We add new tests for media documents and related features that cover the following cases:
+        
+            - checking <video> with only audio tracks does not show the fullscreen button
+            - checking <video> with only audio tracks does not show the picture-in-picture button
+            - checking the size used in media documents for audio and video across macOS, iPhone and iPad
+
+        * media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only-expected.txt: Added.
+        * media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only.html: Added.
+        * media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing-expected.txt: Added.
+        * media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing.html: Added.
+        * media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing-expected.txt: Added.
+        * media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing.html: Added.
+        * media/modern-media-controls/media-documents/media-document-audio-ios-sizing-expected.txt: Added.
+        * media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html: Added.
+        * media/modern-media-controls/media-documents/media-document-audio-mac-sizing-expected.txt: Added.
+        * media/modern-media-controls/media-documents/media-document-audio-mac-sizing.html: Added.
+        * media/modern-media-controls/media-documents/media-document-video-ios-sizing-expected.txt: Added.
+        * media/modern-media-controls/media-documents/media-document-video-ios-sizing.html: Added.
+        * media/modern-media-controls/media-documents/media-document-video-mac-sizing-expected.txt: Added.
+        * media/modern-media-controls/media-documents/media-document-video-mac-sizing.html: Added.
+        * media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only-expected.txt: Added.
+        * media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only.html: Added.
+        * platform/ios-simulator/TestExpectations:
+        * platform/mac/TestExpectations:
+
 2017-03-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         REGRESSION(213764): Large images should not be decoded asynchronously when they are drawn on a canvas
diff --git a/LayoutTests/media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only-expected.txt b/LayoutTests/media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only-expected.txt
new file mode 100644 (file)
index 0000000..4b71d75
--- /dev/null
@@ -0,0 +1,14 @@
+Testing that the fullscreen button is disabled for <video> with an audio resource.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Once media loads its metadata the fullscreen button should become disabled
+Obtained loadedmetadata event
+PASS mediaController.isAudio is true
+PASS mediaController.controls.fullscreenButton.enabled is false
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only.html b/LayoutTests/media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only.html
new file mode 100644 (file)
index 0000000..c49eaba
--- /dev/null
@@ -0,0 +1,44 @@
+<script src="../../../resources/js-test-pre.js"></script>
+<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
+<body>
+<style type="text/css" media="screen">
+    
+    video, #host {
+        position: absolute;
+        top: 0;
+        left: 0;
+    }
+
+    video {
+        width: 800px;
+        height: 240px;
+    }
+    
+</style>
+<video src="../../content/test.wav" controls autoplay></video>
+<div id="host"></div>
+<script type="text/javascript">
+
+window.jsTestIsAsync = true;
+
+description("Testing that the fullscreen button is disabled for &lt;video> with an audio resource.");
+
+const container = document.querySelector("div#host");
+const media = document.querySelector("video");
+const mediaController = createControls(container, media, null);
+
+debug("Once media loads its metadata the fullscreen button should become disabled");
+media.addEventListener("loadedmetadata", () => {
+    debug("Obtained loadedmetadata event");
+    shouldBeTrue("mediaController.isAudio");
+    shouldBeFalse("mediaController.controls.fullscreenButton.enabled");
+    
+    debug("");
+    media.remove();
+    container.remove();
+    finishJSTest();
+});
+
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
diff --git a/LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing-expected.txt b/LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing-expected.txt
new file mode 100644 (file)
index 0000000..f894be8
--- /dev/null
@@ -0,0 +1,12 @@
+Testing the size of the media element in an audio media document on iOS.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getComputedStyle(media).width became "650px"
+PASS getComputedStyle(media).height is "50px"
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing.html b/LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing.html
new file mode 100644 (file)
index 0000000..8025247
--- /dev/null
@@ -0,0 +1,34 @@
+<script src="../../../../resources/js-test-pre.js"></script>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<body>
+<iframe src="../../../content/silence.mp3" style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;"></iframe>
+<script type="text/javascript">
+
+description("Testing the size of the media element in an audio media document on iOS.");
+
+window.jsTestIsAsync = true;
+
+let media;
+
+(function runTestIfReady() {
+    const iframe = document.querySelector("iframe");
+    media = iframe.contentDocument.querySelector("video");
+
+    if (!media) {
+        setTimeout(runTestIfReady);
+        return;
+    }
+
+    shouldBecomeEqualToString("getComputedStyle(media).width", "650px", () => {
+        shouldBeEqualToString("getComputedStyle(media).height", "50px");
+
+        debug("");
+        iframe.remove();
+        finishJSTest();
+    });
+
+})();
+
+</script>
+<script src="../../../../resources/js-test-post.js"></script>
+</body>
diff --git a/LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing-expected.txt b/LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing-expected.txt
new file mode 100644 (file)
index 0000000..b3d6878
--- /dev/null
@@ -0,0 +1,12 @@
+Testing the size of the media element in a video media document on iOS.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getComputedStyle(media).width became "700px"
+PASS getComputedStyle(media).height is "525px"
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing.html b/LayoutTests/media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing.html
new file mode 100644 (file)
index 0000000..d6c5483
--- /dev/null
@@ -0,0 +1,34 @@
+<script src="../../../../resources/js-test-pre.js"></script>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<body>
+<iframe src="../../../content/test.mp4" style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;"></iframe>
+<script type="text/javascript">
+
+description("Testing the size of the media element in a video media document on iOS.");
+
+window.jsTestIsAsync = true;
+
+let media;
+
+(function runTestIfReady() {
+    const iframe = document.querySelector("iframe");
+    media = iframe.contentDocument.querySelector("video");
+
+    if (!media) {
+        setTimeout(runTestIfReady);
+        return;
+    }
+
+    shouldBecomeEqualToString("getComputedStyle(media).width", "700px", () => {
+        shouldBeEqualToString("getComputedStyle(media).height", "525px");
+
+        debug("");
+        iframe.remove();
+        finishJSTest();
+    });
+
+})();
+
+</script>
+<script src="../../../../resources/js-test-post.js"></script>
+</body>
diff --git a/LayoutTests/media/modern-media-controls/media-documents/media-document-audio-ios-sizing-expected.txt b/LayoutTests/media/modern-media-controls/media-documents/media-document-audio-ios-sizing-expected.txt
new file mode 100644 (file)
index 0000000..7db75b3
--- /dev/null
@@ -0,0 +1,12 @@
+Testing the size of the media element in an audio media document on iOS.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getComputedStyle(media).width became "320px"
+PASS getComputedStyle(media).height is "50px"
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html b/LayoutTests/media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html
new file mode 100644 (file)
index 0000000..ec4248a
--- /dev/null
@@ -0,0 +1,35 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<script src="../../../resources/js-test-pre.js"></script>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<body>
+<iframe src="../../content/silence.mp3" style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;"></iframe>
+<script type="text/javascript">
+
+description("Testing the size of the media element in an audio media document on iOS.");
+
+window.jsTestIsAsync = true;
+
+let media;
+
+(function runTestIfReady() {
+    const iframe = document.querySelector("iframe");
+    media = iframe.contentDocument.querySelector("video");
+
+    if (!media) {
+        setTimeout(runTestIfReady);
+        return;
+    }
+
+    shouldBecomeEqualToString("getComputedStyle(media).width", "320px", () => {
+        shouldBeEqualToString("getComputedStyle(media).height", "50px");
+
+        debug("");
+        iframe.remove();
+        finishJSTest();
+    });
+
+})();
+
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
diff --git a/LayoutTests/media/modern-media-controls/media-documents/media-document-audio-mac-sizing-expected.txt b/LayoutTests/media/modern-media-controls/media-documents/media-document-audio-mac-sizing-expected.txt
new file mode 100644 (file)
index 0000000..2c22fa4
--- /dev/null
@@ -0,0 +1,12 @@
+Testing the size of the media element in an audio media document on macOS.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getComputedStyle(media).width became "650px"
+PASS getComputedStyle(media).height is "25px"
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/media/modern-media-controls/media-documents/media-document-audio-mac-sizing.html b/LayoutTests/media/modern-media-controls/media-documents/media-document-audio-mac-sizing.html
new file mode 100644 (file)
index 0000000..d9bef53
--- /dev/null
@@ -0,0 +1,33 @@
+<script src="../../../resources/js-test-pre.js"></script>
+<body>
+<iframe src="../../content/silence.mp3" style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;"></iframe>
+<script type="text/javascript">
+
+description("Testing the size of the media element in an audio media document on macOS.");
+
+window.jsTestIsAsync = true;
+
+let media;
+
+(function runTestIfReady() {
+    const iframe = document.querySelector("iframe");
+    media = iframe.contentDocument.querySelector("video");
+
+    if (!media) {
+        setTimeout(runTestIfReady);
+        return;
+    }
+
+    shouldBecomeEqualToString("getComputedStyle(media).width", "650px", () => {
+        shouldBeEqualToString("getComputedStyle(media).height", "25px");
+
+        debug("");
+        iframe.remove();
+        finishJSTest();
+    });
+
+})();
+
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
diff --git a/LayoutTests/media/modern-media-controls/media-documents/media-document-video-ios-sizing-expected.txt b/LayoutTests/media/modern-media-controls/media-documents/media-document-video-ios-sizing-expected.txt
new file mode 100644 (file)
index 0000000..6c54dfc
--- /dev/null
@@ -0,0 +1,12 @@
+Testing the size of the media element in a video media document on iOS.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getComputedStyle(media).height became "240px"
+PASS getComputedStyle(media).width is "320px"
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/media/modern-media-controls/media-documents/media-document-video-ios-sizing.html b/LayoutTests/media/modern-media-controls/media-documents/media-document-video-ios-sizing.html
new file mode 100644 (file)
index 0000000..e1cb81a
--- /dev/null
@@ -0,0 +1,35 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<script src="../../../resources/js-test-pre.js"></script>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<body>
+<iframe src="../../content/test.mp4" style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;"></iframe>
+<script type="text/javascript">
+
+description("Testing the size of the media element in a video media document on iOS.");
+
+window.jsTestIsAsync = true;
+
+let media;
+
+(function runTestIfReady() {
+    const iframe = document.querySelector("iframe");
+    media = iframe.contentDocument.querySelector("video");
+
+    if (!media) {
+        setTimeout(runTestIfReady);
+        return;
+    }
+
+    shouldBecomeEqualToString("getComputedStyle(media).height", "240px", () => {
+        shouldBeEqualToString("getComputedStyle(media).width", "320px");
+
+        debug("");
+        iframe.remove();
+        finishJSTest();
+    });
+
+})();
+
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
diff --git a/LayoutTests/media/modern-media-controls/media-documents/media-document-video-mac-sizing-expected.txt b/LayoutTests/media/modern-media-controls/media-documents/media-document-video-mac-sizing-expected.txt
new file mode 100644 (file)
index 0000000..c301b07
--- /dev/null
@@ -0,0 +1,12 @@
+Testing the size of the media element in a video media document on macOS.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getComputedStyle(media).width became "700px"
+PASS getComputedStyle(media).height is "525px"
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/media/modern-media-controls/media-documents/media-document-video-mac-sizing.html b/LayoutTests/media/modern-media-controls/media-documents/media-document-video-mac-sizing.html
new file mode 100644 (file)
index 0000000..81e0bd7
--- /dev/null
@@ -0,0 +1,33 @@
+<script src="../../../resources/js-test-pre.js"></script>
+<body>
+<iframe src="../../content/test.mp4" style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;"></iframe>
+<script type="text/javascript">
+
+description("Testing the size of the media element in a video media document on macOS.");
+
+window.jsTestIsAsync = true;
+
+let media;
+
+(function runTestIfReady() {
+    const iframe = document.querySelector("iframe");
+    media = iframe.contentDocument.querySelector("video");
+
+    if (!media) {
+        setTimeout(runTestIfReady);
+        return;
+    }
+
+    shouldBecomeEqualToString("getComputedStyle(media).width", "700px", () => {
+        shouldBeEqualToString("getComputedStyle(media).height", "525px");
+
+        debug("");
+        iframe.remove();
+        finishJSTest();
+    });
+
+})();
+
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
diff --git a/LayoutTests/media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only-expected.txt b/LayoutTests/media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only-expected.txt
new file mode 100644 (file)
index 0000000..fe5de4a
--- /dev/null
@@ -0,0 +1,14 @@
+Testing that the picture-in-picture button is disabled for <video> with an audio resource.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Once media loads its metadata the picture-in-picture button should become disabled
+Obtained loadedmetadata event
+PASS mediaController.isAudio is true
+PASS mediaController.controls.pipButton.enabled is false
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only.html b/LayoutTests/media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only.html
new file mode 100644 (file)
index 0000000..5f593d4
--- /dev/null
@@ -0,0 +1,44 @@
+<script src="../../../resources/js-test-pre.js"></script>
+<script src="../resources/media-controls-loader.js" type="text/javascript"></script>
+<body>
+<style type="text/css" media="screen">
+    
+    video, #host {
+        position: absolute;
+        top: 0;
+        left: 0;
+    }
+
+    video {
+        width: 800px;
+        height: 240px;
+    }
+    
+</style>
+<video src="../../content/test.wav" controls autoplay></video>
+<div id="host"></div>
+<script type="text/javascript">
+
+window.jsTestIsAsync = true;
+
+description("Testing that the picture-in-picture button is disabled for &lt;video> with an audio resource.");
+
+const container = document.querySelector("div#host");
+const media = document.querySelector("video");
+const mediaController = createControls(container, media, null);
+
+debug("Once media loads its metadata the picture-in-picture button should become disabled");
+media.addEventListener("loadedmetadata", () => {
+    debug("Obtained loadedmetadata event");
+    shouldBeTrue("mediaController.isAudio");
+    shouldBeFalse("mediaController.controls.pipButton.enabled");
+    
+    debug("");
+    media.remove();
+    container.remove();
+    finishJSTest();
+});
+
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
index 8210e21..d66fc14 100644 (file)
@@ -2840,6 +2840,8 @@ media/modern-media-controls/pip-support/pip-support-enabled.html [ Skip ]
 media/modern-media-controls/placard-support/placard-support-pip.html [ Skip ]
 media/modern-media-controls/scrubber-support/scrubber-support-drag.html [ Skip ]
 media/modern-media-controls/scrubber-support/ipad/scrubber-support-drag.html [ Skip ]
+media/modern-media-controls/media-documents/media-document-audio-mac-sizing.html [ Skip ]
+media/modern-media-controls/media-documents/media-document-video-mac-sizing.html [ Skip ]
 
 # These tests use uiController and need to be skipped in open-source
 media/modern-media-controls/placard-support/ipad/placard-support-pip.html [ Skip ]
@@ -2851,6 +2853,10 @@ media/modern-media-controls/start-support/start-support-click-to-start.html [ Sk
 media/modern-media-controls/start-support/start-support-lowPowerMode.html [ Skip ]
 media/modern-media-controls/button/button.html [ Skip ]
 
+# iPad-specific tests that need to be skipped in open-source
+media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing.html [ Skip ]
+media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing.html [ Skip ]
+
 # AirPlay cannot be tested on iOS
 webkit.org/b/166062 media/modern-media-controls/airplay-placard/airplay-placard-text-section.html [ Skip ]
 webkit.org/b/166062 media/modern-media-controls/airplay-support/airplay-support.html [ Skip ]
@@ -2924,6 +2930,3 @@ fast/css-generated-content/initial-letter-pagination-raised.html [ Skip ]
 fast/css-generated-content/initial-letter-pagination-sunken.html [ Skip ]
 fast/css-generated-content/initial-letter-pagination-raised-rl.html [ Skip ]
 fast/css-generated-content/initial-letter-pagination-sunken-rl.html [ Skip ]
-
-webkit.org/b/170123 media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing.html [ Timeout ]
-webkit.org/b/170123 media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing.html [ Timeout ]
index ef49db5..c3a32ad 100644 (file)
@@ -1472,6 +1472,10 @@ media/modern-media-controls/ios-inline-media-controls/ios-inline-media-controls-
 media/modern-media-controls/media-controller/media-controller-ios-only-enable-tap-gesture-recognizer-with-fades-when-idle.html [ Skip ]
 media/modern-media-controls/media-controller/media-controller-ios-do-not-hide-controls-when-tapping-button.html [ Skip ]
 media/modern-media-controls/media-controller/media-controller-tight-padding.html [ Skip ]
+media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html [ Skip ]
+media/modern-media-controls/media-documents/media-document-video-ios-sizing.html [ Skip ]
+media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing.html [ Skip ]
+media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing.html [ Skip ]
 media/modern-media-controls/audio/audio-controls-styles.html [ Skip ]
 
 webkit.org/b/169118 [ Debug ] media/modern-media-controls/fullscreen-support/fullscreen-support-click.html [ Pass Timeout ]
index 8503948..5b679fc 100644 (file)
@@ -1,3 +1,80 @@
+2017-03-28  Antoine Quint  <graouts@apple.com>
+
+        [Modern Media Controls] Improve media documents across macOS, iPhone and iPad
+        https://bugs.webkit.org/show_bug.cgi?id=169145
+        <rdar://problem/17048858>
+
+        Reviewed by Dean Jackson.
+
+        There were a variety of issues with media documents, some longstanding, and some specifically
+        about modern media controls.
+
+        One issue was that fullscreen and picture-in-picture buttons would show for audio media documents,
+        due to using a <video> element to load the audio file. We now have additional logic in MediaController
+        to identify if the loaded media is really an audio file, and using this information to hide the
+        fullscreen and picture-in-picture buttons.
+
+        Another issue was that we would inject style in MediaDocument.cpp that was specific to modern media
+        controls when we could have the modern-media-controls module injected CSS handle this styling. We now
+        use the injected style in the shadow root to size media documents based on the device characteristics
+        and ensuring that page styles are overridden.
+
+        We also simplify how MediaDocument.cpp sets the source of the media element to simply use the "src"
+        attribute and not a <source> element.
+
+        Finally, we introduce a MediaDocumentController class that is instantiated when we're dealing with
+        a media document to hide the controls while we determine the type of media we're loading (audio vs.
+        video) in order to apply the appropriate styling without flashes.
+
+        As a result of the new styles applied by the modern-media-controls module, media documents have a
+        similar behavior on macOS and iPad, where we only enforce a min-width for video allowing them
+        to play at their natural size otherwise, and enforcing a fixed width for audio. On iPhone however,
+        we want to always play the media at full width, with some padding in the case of audio.
+
+        Tests: media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only.html
+               media/modern-media-controls/media-documents/ipad/media-document-audio-ios-sizing.html
+               media/modern-media-controls/media-documents/ipad/media-document-video-ios-sizing.html
+               media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html
+               media/modern-media-controls/media-documents/media-document-audio-mac-sizing.html
+               media/modern-media-controls/media-documents/media-document-video-ios-sizing.html
+               media/modern-media-controls/media-documents/media-document-video-mac-sizing.html
+               media/modern-media-controls/pip-support/pip-support-disabled-video-with-audio-tracks-only.html
+
+        * Modules/modern-media-controls/controls/ios-inline-media-controls.css:
+        (:host(audio) .media-controls.ios.inline > .controls-bar:before,):
+        (:host(audio) .media-controls.ios.inline > .controls-bar:before): Deleted.
+        * Modules/modern-media-controls/controls/macos-media-controls.css:
+        (:host(audio) .media-controls.mac.inline > .controls-bar,):
+        (:host(audio) .media-controls.mac.inline > .controls-bar > .background-tint,):
+        (:host(audio) .media-controls.mac.inline > .controls-bar): Deleted.
+        (:host(audio) .media-controls.mac.inline > .controls-bar > .background-tint): Deleted.
+        * Modules/modern-media-controls/controls/media-document.css: Copied from Source/WebCore/Modules/modern-media-controls/controls/macos-media-controls.css.
+        (:host(.media-document)):
+        (:host(.media-document.ready)):
+        (:host(.media-document.audio.mac)):
+        (:host(.media-document.audio.ipad)):
+        (:host(.media-document.audio.iphone)):
+        (:host(.media-document.video.mac)):
+        (:host(.media-document.video.ipad)):
+        (:host(.media-document.video.iphone)):
+        * Modules/modern-media-controls/js-files:
+        * Modules/modern-media-controls/media/fullscreen-support.js:
+        (FullscreenSupport.prototype.syncControl):
+        (FullscreenSupport):
+        * Modules/modern-media-controls/media/media-controller.js:
+        (MediaController):
+        (MediaController.prototype.get isAudio):
+        * Modules/modern-media-controls/media/media-document-controller.js: Copied from Source/WebCore/Modules/modern-media-controls/media/fullscreen-support.js.
+        (MediaDocumentController):
+        (MediaDocumentController.prototype.handleEvent):
+        (MediaDocumentController.prototype._mediaDocumentHasMetadata):
+        (MediaDocumentController.prototype._mediaDocumentHasSize):
+        * Modules/modern-media-controls/media/pip-support.js:
+        (PiPSupport.prototype.syncControl):
+        (PiPSupport):
+        * html/MediaDocument.cpp:
+        (WebCore::MediaDocumentParser::createDocumentStructure):
+
 2017-03-28  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Follow-up patch after r214364.
index e179125..ba3ffb7 100644 (file)
@@ -43,7 +43,8 @@
     will-change: transform;
 }
 
-:host(audio) .media-controls.ios.inline > .controls-bar:before {
+:host(audio) .media-controls.ios.inline > .controls-bar:before,
+:host(.media-document.audio) .media-controls.ios.inline > .controls-bar:before {
     -webkit-appearance: none;
     background-color: rgb(212, 212, 212);
 }
index 9dddcec..f190447 100644 (file)
     -webkit-appearance: none !important;
 }
 
-:host(audio) .media-controls.mac.inline > .controls-bar {
+:host(audio) .media-controls.mac.inline > .controls-bar,
+:host(.media-document.audio) .media-controls.mac.inline > .controls-bar {
     background-color: rgb(41, 41, 41);
 }
 
-:host(audio) .media-controls.mac.inline > .controls-bar > .background-tint {
+:host(audio) .media-controls.mac.inline > .controls-bar > .background-tint,
+:host(.media-document.audio) .media-controls.mac.inline > .controls-bar > .background-tint {
     display: none;
 }
diff --git a/Source/WebCore/Modules/modern-media-controls/controls/media-document.css b/Source/WebCore/Modules/modern-media-controls/controls/media-document.css
new file mode 100644 (file)
index 0000000..13ad79c
--- /dev/null
@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) 2017 Apple Inc. All Rights Reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+:host(.media-document) {
+    visibility: hidden !important;
+    max-width: 100% !important;
+    min-height: 50px !important;
+}
+
+:host(.media-document.ready) {
+    visibility: visible !important;
+}
+
+/* Audio */
+
+:host(.media-document.audio.mac) {
+    width: 650px !important;
+    min-height: 25px !important;
+}
+
+:host(.media-document.audio.ipad) {
+    width: 650px !important;
+}
+
+:host(.media-document.audio.iphone) {
+    padding: 0 10px;
+    width: 100% !important;
+    box-sizing: border-box;
+}
+
+/* Video */
+
+:host(.media-document.video.mac) {
+    min-width: 700px !important;
+}
+
+:host(.media-document.video.ipad) {
+    min-width: 700px !important;
+}
+
+:host(.media-document.video.iphone) {
+    width: 100% !important;
+}
index 54ce9a6..017b39c 100644 (file)
@@ -61,5 +61,6 @@ media/tracks-support.js
 media/volume-down-support.js
 media/volume-support.js
 media/volume-up-support.js
+media/media-document-controller.js
 media/media-controller.js
 
index 168da00..697e5dd 100644 (file)
@@ -69,7 +69,7 @@ class FullscreenSupport extends MediaControllerSupport
     {
         const control = this.control;
         const media = this.mediaController.media;
-        control.enabled = media.webkitSupportsFullscreen && media.videoTracks.length > 0;
+        control.enabled = !this.mediaController.isAudio && media.webkitSupportsFullscreen;
         control.isFullScreen = media.webkitDisplayingFullscreen;
     }
 
index e436d4c..af6074d 100644 (file)
@@ -40,6 +40,8 @@ class MediaController
         if (host) {
             host.controlsDependOnPageScaleFactor = this.layoutTraits & LayoutTraits.iOS;
             this.container.appendChild(host.textTrackContainer);
+            if (host.isInMediaDocument)
+                this.mediaDocumentController = new MediaDocumentController(this);
         }
 
         this._updateControlsIfNeeded();
@@ -60,7 +62,15 @@ class MediaController
 
     get isAudio()
     {
-        return this.media instanceof HTMLAudioElement || (this.media.readyState >= HTMLMediaElement.HAVE_METADATA && this.media.videoWidth === 0);
+        if (this.media instanceof HTMLAudioElement)
+            return true;
+
+        if (this.media.readyState < HTMLMediaElement.HAVE_METADATA)
+            return false;
+
+        const isLiveBroadcast = this.media.duration === Number.POSITIVE_INFINITY;
+        const hasVideoTracks = this.media.videoWidth != 0;
+        return !isLiveBroadcast && !hasVideoTracks;
     }
 
     get layoutTraits()
diff --git a/Source/WebCore/Modules/modern-media-controls/media/media-document-controller.js b/Source/WebCore/Modules/modern-media-controls/media/media-document-controller.js
new file mode 100644 (file)
index 0000000..25848c6
--- /dev/null
@@ -0,0 +1,75 @@
+/*
+ * Copyright (C) 2017 Apple Inc. All Rights Reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+class MediaDocumentController
+{
+
+    constructor(mediaController)
+    {
+        this.mediaController = mediaController;
+
+        const media = mediaController.media;
+        media.classList.add("media-document");
+
+        if (media.readyState >= HTMLMediaElement.HAVE_METADATA)
+            this._mediaDocumentHasMetadata();
+        else
+            media.addEventListener("loadedmetadata", this);
+    }
+
+    // Protected
+
+    handleEvent(event)
+    {
+        event.currentTarget.removeEventListener(event.type, this);
+
+        if (event.type === "loadedmetadata")
+            this._mediaDocumentHasMetadata();
+        else if (event.type === "resize")
+            this._mediaDocumentHasSize();
+    }
+
+    // Private
+
+    _mediaDocumentHasMetadata()
+    {
+        window.requestAnimationFrame(() => {
+            const media = this.mediaController.media;
+            media.classList.add(this.mediaController.isAudio ? "audio" : "video");
+            media.classList.add(window.navigator.platform === "MacIntel" ? "mac" : window.navigator.platform);
+
+            if (this.mediaController.isAudio)
+                this._mediaDocumentHasSize();
+            else
+                this.mediaController.shadowRoot.addEventListener("resize", this);
+        });
+    }
+
+    _mediaDocumentHasSize()
+    {
+        window.requestAnimationFrame(() => this.mediaController.media.classList.add("ready"));
+    }
+
+}
index 2a73a31..e1bfc7d 100644 (file)
@@ -56,7 +56,7 @@ class PiPSupport extends MediaControllerSupport
     {
         const media = this.mediaController.media;
         if (media.webkitSupportsPresentationMode)
-            this.control.enabled = media instanceof HTMLVideoElement && media.webkitSupportsPresentationMode(PiPMode) && !media.webkitCurrentPlaybackTargetIsWireless;
+            this.control.enabled = !this.mediaController.isAudio && media.webkitSupportsPresentationMode(PiPMode) && !media.webkitCurrentPlaybackTargetIsWireless;
         else
             this.control.enabled = false;
     }
index 8e95fac..4624168 100644 (file)
@@ -104,38 +104,26 @@ void MediaDocumentParser::createDocumentStructure()
 #endif
 
     auto body = HTMLBodyElement::create(document);
-    if (RuntimeEnabledFeatures::sharedFeatures().modernMediaControlsEnabled()) {
-        StringBuilder bodyStyle;
-        bodyStyle.appendLiteral("margin: 0; padding: 0;");
-        bodyStyle.appendLiteral("background-color: rgb(38, 38, 38);");
-        bodyStyle.appendLiteral("display: flex; justify-content: center; align-items: center;");
-        body->setAttribute(styleAttr, bodyStyle.toString());
-    }
     rootElement->appendChild(body);
 
     auto videoElement = HTMLVideoElement::create(document);
     m_mediaElement = videoElement.ptr();
     videoElement->setAttributeWithoutSynchronization(controlsAttr, emptyAtom);
     videoElement->setAttributeWithoutSynchronization(autoplayAttr, emptyAtom);
-    videoElement->setAttributeWithoutSynchronization(nameAttr, AtomicString("media", AtomicString::ConstructFromLiteral));
+    videoElement->setAttributeWithoutSynchronization(playsinlineAttr, emptyAtom);
+    videoElement->setSrc(document.url());
+    if (auto* loader = document.loader())
+        videoElement->setAttributeWithoutSynchronization(typeAttr, loader->responseMIMEType());
 
-    StringBuilder elementStyle;
-    elementStyle.appendLiteral("max-width: 100%; max-height: 100%;");
+    if (!RuntimeEnabledFeatures::sharedFeatures().modernMediaControlsEnabled()) {
+        StringBuilder elementStyle;
+        elementStyle.appendLiteral("max-width: 100%; max-height: 100%;");
 #if PLATFORM(IOS)
-    elementStyle.appendLiteral("width: 100%; height: auto;");
+        elementStyle.appendLiteral("width: 100%; height: auto;");
 #endif
-    if (RuntimeEnabledFeatures::sharedFeatures().modernMediaControlsEnabled()) {
-        elementStyle.appendLiteral("min-height: 50px;");
+        videoElement->setAttribute(styleAttr, elementStyle.toString());
     }
-    videoElement->setAttribute(styleAttr, elementStyle.toString());
-
-    auto sourceElement = HTMLSourceElement::create(document);
-    sourceElement->setSrc(document.url());
-
-    if (auto* loader = document.loader())
-        sourceElement->setType(loader->responseMIMEType());
 
-    videoElement->appendChild(sourceElement);
     body->appendChild(videoElement);
 
     Frame* frame = document.frame();