- Revision
- 218600
- Author
- grao...@webkit.org
- Date
- 2017-06-20 10:38:35 -0700 (Tue, 20 Jun 2017)
Log Message
Media document experience with long-loading files is poor
https://bugs.webkit.org/show_bug.cgi?id=173575
<rdar://problem/32178119>
Reviewed by Dean Jackson.
Source/WebCore:
In order to avoid showing media controls at a different size than that of the video when we've
received enough information to determine whether it's audio or video and what the video frame size
is, we do not show any UI until we have enough information to show the controls in their correct
initial state. This works well with local files and fast-loading files, but does not work well with
invalid files, which never load and fail to ever show any UI, and files that load slowly where there
is no visible feedback that content will be visible.
Instead, we now default to showing audio controls in their loading state, which provides a seamless
transition if we will be loading an audio file since the controls are initially in the correct state,
and at least provide feedback that data is loading even if we eventually transition to a video layout.
Additionally, we remove the invalid placard background in case the media is invalid, showing only the
crossed-out play icon in the center of the page in that state.
Tests: media/modern-media-controls/media-documents/media-document-invalid.html
media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout.html
* Modules/modern-media-controls/controls/media-document.css:
(:host(.media-document)): Remove "visibility: hidden" since we want the media controls to be visible
at all times.
(:host(.media-document.audio)): Add a little padding on the x-axis to ensure audio controls never snap
directly to the edges of the window.
(:host(.media-document.audio.iphone)): Remove the iPhone-specific styling since we moved it to the
general case.
(:host(.media-document.video.invalid) .placard): Remove the background from the invalid placard when
showing invalid media.
(:host(.media-document.ready)): Deleted.
* Modules/modern-media-controls/media/audio-support.js:
(AudioSupport.prototype.syncControl): Make sure we invalidate the media document layout when a media
document's media type changes.
* Modules/modern-media-controls/media/media-controller.js:
(MediaController): Instantiate the controls prior to creating the MediaDocumentController since the
MediaDocumentController will need to access the controls.
* Modules/modern-media-controls/media/media-document-controller.js:
(MediaDocumentController): Set the default layout for media controls for a media document to be audio
and in the waiting state.
(MediaDocumentController.prototype.layout): Toggle the "invalid", "audio" and "video" CSS classes for
the next possible commit to the DOM, provided we have established the media document's media type.
(MediaDocumentController.prototype.handleEvent): Deal with the "play" and "error" events to trigger
a layout.
(MediaDocumentController.prototype._mediaDocumentHasMetadata): Deleted.
(MediaDocumentController.prototype._mediaDocumentHasSize): Deleted.
LayoutTests:
Fix a test that started failing with this patch and add two new tests that check we are adding
the expected CSS classes when loading video and invalid media documents.
* media/modern-media-controls/media-documents/media-document-invalid-expected.txt: Added.
* media/modern-media-controls/media-documents/media-document-invalid.html: Added.
* media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout-expected.txt: Added.
* media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout.html: Added.
* media/modern-media-controls/volume-up-support/volume-up-support-expected.txt:
* media/modern-media-controls/volume-up-support/volume-up-support.html:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (218599 => 218600)
--- trunk/LayoutTests/ChangeLog 2017-06-20 17:21:47 UTC (rev 218599)
+++ trunk/LayoutTests/ChangeLog 2017-06-20 17:38:35 UTC (rev 218600)
@@ -1,3 +1,21 @@
+2017-06-19 Antoine Quint <grao...@apple.com>
+
+ Media document experience with long-loading files is poor
+ https://bugs.webkit.org/show_bug.cgi?id=173575
+ <rdar://problem/32178119>
+
+ Reviewed by Dean Jackson.
+
+ Fix a test that started failing with this patch and add two new tests that check we are adding
+ the expected CSS classes when loading video and invalid media documents.
+
+ * media/modern-media-controls/media-documents/media-document-invalid-expected.txt: Added.
+ * media/modern-media-controls/media-documents/media-document-invalid.html: Added.
+ * media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout-expected.txt: Added.
+ * media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout.html: Added.
+ * media/modern-media-controls/volume-up-support/volume-up-support-expected.txt:
+ * media/modern-media-controls/volume-up-support/volume-up-support.html:
+
2017-06-20 Claudio Saavedra <csaave...@igalia.com>
[WPE] Enable appcache tests
Added: trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-invalid-expected.txt (0 => 218600)
--- trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-invalid-expected.txt (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-invalid-expected.txt 2017-06-20 17:38:35 UTC (rev 218600)
@@ -0,0 +1,12 @@
+Testing that a media document initially starts with an audio layout and then transitions to an invalid layout with no source.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS media.classList.contains('audio') is true
+PASS media.classList.contains('invalid') became true
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-invalid.html (0 => 218600)
--- trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-invalid.html (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-invalid.html 2017-06-20 17:38:35 UTC (rev 218600)
@@ -0,0 +1,32 @@
+<script src=""
+<body>
+<iframe src="" style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;"></iframe>
+<script type="text/_javascript_">
+
+description("Testing that a media document initially starts with an audio layout and then transitions to an invalid layout with no source.");
+
+window.jsTestIsAsync = true;
+
+let media;
+
+(function runTestIfReady() {
+ const iframe = document.querySelector("iframe");
+ media = iframe.contentDocument.querySelector("video");
+
+ if (!media) {
+ setTimeout(runTestIfReady);
+ return;
+ }
+
+ // Media initially is laid out as an audio element but then becomes a video.
+ shouldBeTrue("media.classList.contains('audio')");
+ shouldBecomeEqual("media.classList.contains('invalid')", "true", () => {
+ debug("");
+ iframe.remove();
+ finishJSTest();
+ });
+})();
+
+</script>
+<script src=""
+</body>
Added: trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout-expected.txt (0 => 218600)
--- trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout-expected.txt (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout-expected.txt 2017-06-20 17:38:35 UTC (rev 218600)
@@ -0,0 +1,12 @@
+Testing that a media document initially starts with an audio layout and then transitions to a video layout.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS media.classList.contains('audio') is true
+PASS media.classList.contains('video') became true
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout.html (0 => 218600)
--- trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout.html (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout.html 2017-06-20 17:38:35 UTC (rev 218600)
@@ -0,0 +1,32 @@
+<script src=""
+<body>
+<iframe src="" style="position: absolute; top: 0; left: 0; width: 100%; height: 100%;"></iframe>
+<script type="text/_javascript_">
+
+description("Testing that a media document initially starts with an audio layout and then transitions to a video layout.");
+
+window.jsTestIsAsync = true;
+
+let media;
+
+(function runTestIfReady() {
+ const iframe = document.querySelector("iframe");
+ media = iframe.contentDocument.querySelector("video");
+
+ if (!media) {
+ setTimeout(runTestIfReady);
+ return;
+ }
+
+ // Media initially is laid out as an audio element but then becomes a video.
+ shouldBeTrue("media.classList.contains('audio')");
+ shouldBecomeEqual("media.classList.contains('video')", "true", () => {
+ debug("");
+ iframe.remove();
+ finishJSTest();
+ });
+})();
+
+</script>
+<script src=""
+</body>
Modified: trunk/LayoutTests/media/modern-media-controls/volume-up-support/volume-up-support-expected.txt (218599 => 218600)
--- trunk/LayoutTests/media/modern-media-controls/volume-up-support/volume-up-support-expected.txt 2017-06-20 17:21:47 UTC (rev 218599)
+++ trunk/LayoutTests/media/modern-media-controls/volume-up-support/volume-up-support-expected.txt 2017-06-20 17:38:35 UTC (rev 218600)
@@ -3,6 +3,7 @@
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+PASS !!window.internals.shadowRoot(media).querySelector('button.volume-up') became true
PASS media.volume is 0.5
Pressing on the volume up button
Modified: trunk/LayoutTests/media/modern-media-controls/volume-up-support/volume-up-support.html (218599 => 218600)
--- trunk/LayoutTests/media/modern-media-controls/volume-up-support/volume-up-support.html 2017-06-20 17:21:47 UTC (rev 218599)
+++ trunk/LayoutTests/media/modern-media-controls/volume-up-support/volume-up-support.html 2017-06-20 17:38:35 UTC (rev 218600)
@@ -17,22 +17,20 @@
media.addEventListener("webkitfullscreenchange", function() {
if (media.webkitDisplayingFullscreen) {
- window.requestAnimationFrame(() => {
- window.requestAnimationFrame(() => {
- shouldBe("media.volume", "0.5");
+ shouldBecomeEqual("!!window.internals.shadowRoot(media).querySelector('button.volume-up')", "true", () => {
+ shouldBe("media.volume", "0.5");
- media.addEventListener("volumechange", () => {
- shouldBe("media.volume", "1");
- debug("");
- media.remove();
- button.remove();
- finishJSTest();
- });
-
+ media.addEventListener("volumechange", () => {
+ shouldBe("media.volume", "1");
debug("");
- debug("Pressing on the volume up button");
- pressOnElement(window.internals.shadowRoot(media).lastElementChild.lastElementChild.querySelector("button.volume-up"));
+ media.remove();
+ button.remove();
+ finishJSTest();
});
+
+ debug("");
+ debug("Pressing on the volume up button");
+ pressOnElement(window.internals.shadowRoot(media).querySelector("button.volume-up"));
});
}
});
Modified: trunk/Source/WebCore/ChangeLog (218599 => 218600)
--- trunk/Source/WebCore/ChangeLog 2017-06-20 17:21:47 UTC (rev 218599)
+++ trunk/Source/WebCore/ChangeLog 2017-06-20 17:38:35 UTC (rev 218600)
@@ -1,3 +1,54 @@
+2017-06-19 Antoine Quint <grao...@apple.com>
+
+ Media document experience with long-loading files is poor
+ https://bugs.webkit.org/show_bug.cgi?id=173575
+ <rdar://problem/32178119>
+
+ Reviewed by Dean Jackson.
+
+ In order to avoid showing media controls at a different size than that of the video when we've
+ received enough information to determine whether it's audio or video and what the video frame size
+ is, we do not show any UI until we have enough information to show the controls in their correct
+ initial state. This works well with local files and fast-loading files, but does not work well with
+ invalid files, which never load and fail to ever show any UI, and files that load slowly where there
+ is no visible feedback that content will be visible.
+
+ Instead, we now default to showing audio controls in their loading state, which provides a seamless
+ transition if we will be loading an audio file since the controls are initially in the correct state,
+ and at least provide feedback that data is loading even if we eventually transition to a video layout.
+
+ Additionally, we remove the invalid placard background in case the media is invalid, showing only the
+ crossed-out play icon in the center of the page in that state.
+
+ Tests: media/modern-media-controls/media-documents/media-document-invalid.html
+ media/modern-media-controls/media-documents/media-document-video-with-initial-audio-layout.html
+
+ * Modules/modern-media-controls/controls/media-document.css:
+ (:host(.media-document)): Remove "visibility: hidden" since we want the media controls to be visible
+ at all times.
+ (:host(.media-document.audio)): Add a little padding on the x-axis to ensure audio controls never snap
+ directly to the edges of the window.
+ (:host(.media-document.audio.iphone)): Remove the iPhone-specific styling since we moved it to the
+ general case.
+ (:host(.media-document.video.invalid) .placard): Remove the background from the invalid placard when
+ showing invalid media.
+ (:host(.media-document.ready)): Deleted.
+ * Modules/modern-media-controls/media/audio-support.js:
+ (AudioSupport.prototype.syncControl): Make sure we invalidate the media document layout when a media
+ document's media type changes.
+ * Modules/modern-media-controls/media/media-controller.js:
+ (MediaController): Instantiate the controls prior to creating the MediaDocumentController since the
+ MediaDocumentController will need to access the controls.
+ * Modules/modern-media-controls/media/media-document-controller.js:
+ (MediaDocumentController): Set the default layout for media controls for a media document to be audio
+ and in the waiting state.
+ (MediaDocumentController.prototype.layout): Toggle the "invalid", "audio" and "video" CSS classes for
+ the next possible commit to the DOM, provided we have established the media document's media type.
+ (MediaDocumentController.prototype.handleEvent): Deal with the "play" and "error" events to trigger
+ a layout.
+ (MediaDocumentController.prototype._mediaDocumentHasMetadata): Deleted.
+ (MediaDocumentController.prototype._mediaDocumentHasSize): Deleted.
+
2017-06-20 Daniel Bates <daba...@apple.com>
NavigationAction has too many constructors
Modified: trunk/Source/WebCore/Modules/modern-media-controls/controls/media-document.css (218599 => 218600)
--- trunk/Source/WebCore/Modules/modern-media-controls/controls/media-document.css 2017-06-20 17:21:47 UTC (rev 218599)
+++ trunk/Source/WebCore/Modules/modern-media-controls/controls/media-document.css 2017-06-20 17:38:35 UTC (rev 218600)
@@ -24,18 +24,19 @@
*/
:host(.media-document) {
- visibility: hidden !important;
max-width: 100% !important;
max-height: 100% !important;
min-height: 50px !important;
}
-:host(.media-document.ready) {
- visibility: visible !important;
+/* Audio */
+
+:host(.media-document.audio) {
+ padding: 0 10px;
+ box-sizing: border-box;
+ height: var(--inline-controls-bar-height) !important;
}
-/* Audio */
-
:host(.media-document.audio.mac) {
width: 650px !important;
min-height: var(--inline-controls-bar-height) !important;
@@ -46,9 +47,7 @@
}
:host(.media-document.audio.iphone) {
- padding: 0 10px;
width: 100% !important;
- box-sizing: border-box;
}
/* Video */
@@ -64,3 +63,9 @@
:host(.media-document.video.iphone) {
width: 100% !important;
}
+
+/* Invalid state */
+
+:host(.media-document.video.invalid) .placard {
+ background-color: transparent;
+}
Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/audio-support.js (218599 => 218600)
--- trunk/Source/WebCore/Modules/modern-media-controls/media/audio-support.js 2017-06-20 17:21:47 UTC (rev 218599)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/audio-support.js 2017-06-20 17:38:35 UTC (rev 218600)
@@ -46,6 +46,8 @@
syncControl()
{
this.control.shouldUseAudioLayout = this.mediaController.isAudio;
+ if (this.mediaController.mediaDocumentController)
+ this.mediaController.mediaDocumentController.layout();
}
}
Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller.js (218599 => 218600)
--- trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller.js 2017-06-20 17:21:47 UTC (rev 218599)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller.js 2017-06-20 17:38:35 UTC (rev 218600)
@@ -37,6 +37,8 @@
this.container = shadowRoot.appendChild(document.createElement("div"));
this.container.className = "media-controls-container";
+ this._updateControlsIfNeeded();
+
if (host) {
host.controlsDependOnPageScaleFactor = this.layoutTraits & LayoutTraits.iOS;
this.container.appendChild(host.textTrackContainer);
@@ -44,7 +46,6 @@
this.mediaDocumentController = new MediaDocumentController(this);
}
- this._updateControlsIfNeeded();
scheduler.flushScheduledLayoutCallbacks();
shadowRoot.addEventListener("resize", this);
Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/media-document-controller.js (218599 => 218600)
--- trunk/Source/WebCore/Modules/modern-media-controls/media/media-document-controller.js 2017-06-20 17:21:47 UTC (rev 218599)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/media-document-controller.js 2017-06-20 17:38:35 UTC (rev 218600)
@@ -30,46 +30,50 @@
{
this.mediaController = mediaController;
+ // Force the controls to look like we're loading an audio file by default.
+ mediaController.controls.shouldUseAudioLayout = true;
+ mediaController.controls.timeControl.loading = true;
+
+ this._hasDeterminedMediaType = false;
+
const media = mediaController.media;
media.classList.add("media-document");
+ media.classList.add("audio");
+ media.classList.add(window.navigator.platform === "MacIntel" ? "mac" : window.navigator.platform);
- if (media.readyState >= HTMLMediaElement.HAVE_METADATA)
- this._mediaDocumentHasMetadata();
- else
- media.addEventListener("loadedmetadata", this);
+ media.addEventListener("error", this);
+ media.addEventListener("play", this);
}
- // Protected
+ // Public
- handleEvent(event)
+ layout()
{
- event.currentTarget.removeEventListener(event.type, this);
+ if (!this._hasDeterminedMediaType)
+ return;
- if (event.type === "loadedmetadata")
- this._mediaDocumentHasMetadata();
- else if (event.type === "resize")
- this._mediaDocumentHasSize();
- }
-
- // Private
-
- _mediaDocumentHasMetadata()
- {
- window.requestAnimationFrame(() => {
+ scheduler.scheduleLayout(() => {
const media = this.mediaController.media;
- media.classList.add(this.mediaController.isAudio ? "audio" : "video");
- media.classList.add(window.navigator.platform === "MacIntel" ? "mac" : window.navigator.platform);
+ const isInvalid = media.error !== null && media.played.length === 0;
+ const useVideoLayout = isInvalid || (media.readyState >= HTMLMediaElement.HAVE_METADATA && !this.mediaController.isAudio);
- if (this.mediaController.isAudio || (media.videoWidth && media.videoHeight))
- this._mediaDocumentHasSize();
- else
- this.mediaController.shadowRoot.addEventListener("resize", this);
+ const classList = media.classList;
+ classList.toggle("invalid", isInvalid);
+ classList.toggle("video", useVideoLayout);
+ classList.toggle("audio", !useVideoLayout);
});
}
- _mediaDocumentHasSize()
+ // Protected
+
+ handleEvent(event)
{
- window.requestAnimationFrame(() => this.mediaController.media.classList.add("ready"));
+ event.currentTarget.removeEventListener(event.type, this);
+
+ if (event.type === "play" || event.type === "error") {
+ this._hasDeterminedMediaType = true;
+ this.layout();
+ }
}
}
