- Revision
- 228947
- Author
- grao...@webkit.org
- Date
- 2018-02-23 05:53:13 -0800 (Fri, 23 Feb 2018)
Log Message
REGRESSION (r228445): A big pause button shows over YouTube videos if you tap "Tap To Unmute" on iOS
https://bugs.webkit.org/show_bug.cgi?id=183074
<rdar://problem/37747028>
Reviewed by Eric Carlson.
Source/WebCore:
Test: media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html
In the fix for webkit.org/b/182668, we made it so that when the "controls" attribute is absent from a media
element we stop listening to the bulk of media events and prevent controls from updating any DOM properties
so as to minimize the amount of CPU usage by the Web process.
An unfortunate side effect was that, if the media controls were disabled at the time the video starts playing,
the StartSupport class would thus not catch the "play" event and would not be able to set the "hasPlayed"
property to "true" on the MediaController, which would then prevent the _shouldShowStartButton() from returning
"false". As a result, if the "controls" attribute was turned back on after the media started playing, they
would default to showing the start button, which would be then in the play state, ie. showing the pause icon.
We now set the "hasPlayed" property in the "play" event handler on MediaController, which is always registered
regardless of the "controls" attribute setting. We also ensure we invalidate the "showStartButton" property on
the media controls when StartSupport is enabled, which is the case when the "controls" attribute is toggled back
to "true" from a previous "false" value.
* Modules/modern-media-controls/media/media-controller.js:
(MediaController.prototype.handleEvent):
* Modules/modern-media-controls/media/start-support.js:
(StartSupport):
(StartSupport.prototype.enable):
(StartSupport.prototype.handleEvent):
(StartSupport.prototype._updateShowsStartButton):
LayoutTests:
Add a new test that set controls on the video, then immediately removes them, plays the video and turns the controls
back on as soon as the video starts to check that the "showsStartButton" property is false on the media controls.
Prior to this patch this test would fail.
* media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play-expected.txt: Added.
* media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html: Added.
* platform/ios/TestExpectations:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (228946 => 228947)
--- trunk/LayoutTests/ChangeLog 2018-02-23 12:53:04 UTC (rev 228946)
+++ trunk/LayoutTests/ChangeLog 2018-02-23 13:53:13 UTC (rev 228947)
@@ -1,3 +1,19 @@
+2018-02-22 Antoine Quint <grao...@apple.com>
+
+ REGRESSION (r228445): A big pause button shows over YouTube videos if you tap "Tap To Unmute" on iOS
+ https://bugs.webkit.org/show_bug.cgi?id=183074
+ <rdar://problem/37747028>
+
+ Reviewed by Eric Carlson.
+
+ Add a new test that set controls on the video, then immediately removes them, plays the video and turns the controls
+ back on as soon as the video starts to check that the "showsStartButton" property is false on the media controls.
+ Prior to this patch this test would fail.
+
+ * media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play-expected.txt: Added.
+ * media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html: Added.
+ * platform/ios/TestExpectations:
+
2018-02-22 Youenn Fablet <you...@apple.com>
CacheStorage::Engine::Caches::writeRecord is not always calling the completion handler
Added: trunk/LayoutTests/media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play-expected.txt (0 => 228947)
--- trunk/LayoutTests/media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play-expected.txt (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play-expected.txt 2018-02-23 13:53:13 UTC (rev 228947)
@@ -0,0 +1,18 @@
+Testing the StartSupport behavior when the media starts playing while controls are disabled and controls become active post-play.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Disabling controls.
+Starting the video.
+
+Obtained the 'play' event.
+PASS mediaController.controls.showsStartButton is true
+
+Turning controls back on.
+PASS mediaController.controls.showsStartButton became false
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html (0 => 228947)
--- trunk/LayoutTests/media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html 2018-02-23 13:53:13 UTC (rev 228947)
@@ -0,0 +1,52 @@
+<script src=""
+<script src="" type="text/_javascript_"></script>
+<script src="" type="text/_javascript_"></script>
+<body>
+<video src="" style="width: 320px; height: 240px;" controls></video>
+<div id="shadow"></div>
+<script type="text/_javascript_">
+
+window.jsTestIsAsync = true;
+
+description("Testing the <code>StartSupport</code> behavior when the media starts playing while controls are disabled and controls become active post-play.");
+
+const shadowRoot = document.querySelector("div#shadow");
+const media = document.querySelector("video");
+const mediaController = createControls(shadowRoot, media, null);
+
+media.addEventListener("createTouch" in document ? "touchstart" : "click", event => {
+ try {
+ media.play();
+ } catch(e) {
+ debug("Calling media.play() failed");
+ finishJSTest();
+ }
+});
+
+media.addEventListener("play", () => {
+ debug("");
+ debug("Obtained the 'play' event.");
+ shouldBeTrue("mediaController.controls.showsStartButton");
+
+ debug("");
+ debug("Turning controls back on.");
+ media.controls = true;
+ shouldBecomeEqual("mediaController.controls.showsStartButton", "false", () => {
+ debug("");
+ shadowRoot.remove();
+ media.remove();
+ finishJSTest();
+ });
+}, { once: true });
+
+media.addEventListener("canplay", () => {
+ debug("Starting the video.");
+ pressOnElement(media);
+}, { once: true });
+
+debug("Disabling controls.");
+media.controls = false;
+
+</script>
+<script src=""
+</body>
Modified: trunk/LayoutTests/platform/ios/TestExpectations (228946 => 228947)
--- trunk/LayoutTests/platform/ios/TestExpectations 2018-02-23 12:53:04 UTC (rev 228946)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2018-02-23 13:53:13 UTC (rev 228947)
@@ -3200,6 +3200,7 @@
media/modern-media-controls/seek-backward-support [ Skip ]
media/modern-media-controls/seek-forward-support [ Skip ]
media/modern-media-controls/start-support/start-support-click-to-start.html [ Skip ]
+media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html [ Skip ]
media/modern-media-controls/start-support/start-support-lowPowerMode.html [ Skip ]
media/modern-media-controls/volume-down-support [ Skip ]
media/modern-media-controls/volume-up-support [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (228946 => 228947)
--- trunk/Source/WebCore/ChangeLog 2018-02-23 12:53:04 UTC (rev 228946)
+++ trunk/Source/WebCore/ChangeLog 2018-02-23 13:53:13 UTC (rev 228947)
@@ -1,3 +1,36 @@
+2018-02-22 Antoine Quint <grao...@apple.com>
+
+ REGRESSION (r228445): A big pause button shows over YouTube videos if you tap "Tap To Unmute" on iOS
+ https://bugs.webkit.org/show_bug.cgi?id=183074
+ <rdar://problem/37747028>
+
+ Reviewed by Eric Carlson.
+
+ Test: media/modern-media-controls/start-support/start-support-disable-controls-and-re-enable-post-play.html
+
+ In the fix for webkit.org/b/182668, we made it so that when the "controls" attribute is absent from a media
+ element we stop listening to the bulk of media events and prevent controls from updating any DOM properties
+ so as to minimize the amount of CPU usage by the Web process.
+
+ An unfortunate side effect was that, if the media controls were disabled at the time the video starts playing,
+ the StartSupport class would thus not catch the "play" event and would not be able to set the "hasPlayed"
+ property to "true" on the MediaController, which would then prevent the _shouldShowStartButton() from returning
+ "false". As a result, if the "controls" attribute was turned back on after the media started playing, they
+ would default to showing the start button, which would be then in the play state, ie. showing the pause icon.
+
+ We now set the "hasPlayed" property in the "play" event handler on MediaController, which is always registered
+ regardless of the "controls" attribute setting. We also ensure we invalidate the "showStartButton" property on
+ the media controls when StartSupport is enabled, which is the case when the "controls" attribute is toggled back
+ to "true" from a previous "false" value.
+
+ * Modules/modern-media-controls/media/media-controller.js:
+ (MediaController.prototype.handleEvent):
+ * Modules/modern-media-controls/media/start-support.js:
+ (StartSupport):
+ (StartSupport.prototype.enable):
+ (StartSupport.prototype.handleEvent):
+ (StartSupport.prototype._updateShowsStartButton):
+
2018-02-23 Carlos Garcia Campos <cgar...@igalia.com>
[GStreamer][MiniBrowser] Honor GStreamer command line parameters in MiniBrowser
Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller.js (228946 => 228947)
--- trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller.js 2018-02-23 12:53:04 UTC (rev 228946)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/media-controller.js 2018-02-23 13:53:13 UTC (rev 228947)
@@ -169,6 +169,8 @@
// We must immediately perform layouts so that we don't lag behind the media layout size.
scheduler.flushScheduledLayoutCallbacks();
} else if (event.currentTarget === this.media) {
+ if (event.type === "play")
+ this.hasPlayed = true;
this._updateControlsIfNeeded();
this._updateControlsAvailability();
if (event.type === "webkitpresentationmodechanged")
Modified: trunk/Source/WebCore/Modules/modern-media-controls/media/start-support.js (228946 => 228947)
--- trunk/Source/WebCore/Modules/modern-media-controls/media/start-support.js 2018-02-23 12:53:04 UTC (rev 228946)
+++ trunk/Source/WebCore/Modules/modern-media-controls/media/start-support.js 2018-02-23 13:53:13 UTC (rev 228947)
@@ -30,7 +30,7 @@
{
super(mediaController);
- this.mediaController.controls.showsStartButton = this._shouldShowStartButton();
+ this._updateShowsStartButton();
}
// Protected
@@ -40,6 +40,13 @@
return ["loadedmetadata", "play", "error", this.mediaController.fullscreenChangeEventType];
}
+ enable()
+ {
+ super.enable();
+
+ this._updateShowsStartButton();
+ }
+
buttonWasPressed(control)
{
this.mediaController.media.play();
@@ -47,16 +54,18 @@
handleEvent(event)
{
- if (event.type === "play")
- this.mediaController.hasPlayed = true;
+ super.handleEvent(event);
- this.mediaController.controls.showsStartButton = this._shouldShowStartButton();
-
- super.handleEvent(event);
+ this._updateShowsStartButton();
}
// Private
+ _updateShowsStartButton()
+ {
+ this.mediaController.controls.showsStartButton = this._shouldShowStartButton();
+ }
+
_shouldShowStartButton()
{
const media = this.mediaController.media;
