- Revision
- 247612
- Author
- kocsen_ch...@apple.com
- Date
- 2019-07-18 13:25:01 -0700 (Thu, 18 Jul 2019)
Log Message
Cherry-pick r247544. rdar://problem/53230040
Unable to bring up custom media controls on iOS for video.sina.cn
https://bugs.webkit.org/show_bug.cgi?id=199889
<rdar://problem/51883919>
Reviewed by Dean Jackson.
Source/WebCore:
Videos on video.sina.cn by default have the "controls" attribute and are set not to autoplay. This means that the original state
of the media controls are set to show the built-in media controls and also show the prominent play button to begin playback. The
display of the play button also requires a tap gesture recognizer, which calls preventDefault() when the "touchend" is received
to prevent double-tap-to-zoom, but also has the side-effect of preventing a "click" event from being dispatched for a tap.
The video.sina.cn code would eventually remove the "controls" attribute, which would make the built-in media controls not visible,
but still participate in hit-testing because we keep the shadow DOM around in order to potentially show the Airplay or picture-in-picture
placards. Additionally, we wouldn't disable the tap gesture recognizer when the "controls" attribute was removed.
We now ensure that both gesture recognizers used by iOS inline media controls are only enabled when media controls are visible.
Test: media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls.html
* Modules/modern-media-controls/controls/ios-inline-media-controls.js:
(IOSInlineMediaControls.prototype.set showsStartButton):
(IOSInlineMediaControls.prototype.get visible):
(IOSInlineMediaControls.prototype.set visible):
(IOSInlineMediaControls.prototype._updateGestureRecognizers):
(IOSInlineMediaControls.prototype._tapGestureRecognizerStateDidChange):
(IOSInlineMediaControls.prototype._pinchGestureRecognizerStateDidChange):
LayoutTests:
This test replicates the scenario found on video.sina.cn that caused the issue: a <video> element originally has the "controls"
attribute and is not set to autoplay. This means the tap gesture recognizer is created to track a tap for the video to play. Then
the "controls" attribute is removed and we dispatch a tap on the video which would previously have *not* caused a "click" event to
eventually be dispatched on the <video> element since the tap gesture recognizer would call preventDefault(). With this patch applied,
we get the "click" event because the tap gesture recognizer is disabled once the controls are no longer visible.
* media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls-expected.txt: Added.
* media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247544 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-608-branch/LayoutTests/ChangeLog (247611 => 247612)
--- branches/safari-608-branch/LayoutTests/ChangeLog 2019-07-18 20:24:58 UTC (rev 247611)
+++ branches/safari-608-branch/LayoutTests/ChangeLog 2019-07-18 20:25:01 UTC (rev 247612)
@@ -1,5 +1,68 @@
2019-07-17 Kocsen Chung <kocsen_ch...@apple.com>
+ Cherry-pick r247544. rdar://problem/53230040
+
+ Unable to bring up custom media controls on iOS for video.sina.cn
+ https://bugs.webkit.org/show_bug.cgi?id=199889
+ <rdar://problem/51883919>
+
+ Reviewed by Dean Jackson.
+
+ Source/WebCore:
+
+ Videos on video.sina.cn by default have the "controls" attribute and are set not to autoplay. This means that the original state
+ of the media controls are set to show the built-in media controls and also show the prominent play button to begin playback. The
+ display of the play button also requires a tap gesture recognizer, which calls preventDefault() when the "touchend" is received
+ to prevent double-tap-to-zoom, but also has the side-effect of preventing a "click" event from being dispatched for a tap.
+
+ The video.sina.cn code would eventually remove the "controls" attribute, which would make the built-in media controls not visible,
+ but still participate in hit-testing because we keep the shadow DOM around in order to potentially show the Airplay or picture-in-picture
+ placards. Additionally, we wouldn't disable the tap gesture recognizer when the "controls" attribute was removed.
+
+ We now ensure that both gesture recognizers used by iOS inline media controls are only enabled when media controls are visible.
+
+ Test: media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls.html
+
+ * Modules/modern-media-controls/controls/ios-inline-media-controls.js:
+ (IOSInlineMediaControls.prototype.set showsStartButton):
+ (IOSInlineMediaControls.prototype.get visible):
+ (IOSInlineMediaControls.prototype.set visible):
+ (IOSInlineMediaControls.prototype._updateGestureRecognizers):
+ (IOSInlineMediaControls.prototype._tapGestureRecognizerStateDidChange):
+ (IOSInlineMediaControls.prototype._pinchGestureRecognizerStateDidChange):
+
+ LayoutTests:
+
+ This test replicates the scenario found on video.sina.cn that caused the issue: a <video> element originally has the "controls"
+ attribute and is not set to autoplay. This means the tap gesture recognizer is created to track a tap for the video to play. Then
+ the "controls" attribute is removed and we dispatch a tap on the video which would previously have *not* caused a "click" event to
+ eventually be dispatched on the <video> element since the tap gesture recognizer would call preventDefault(). With this patch applied,
+ we get the "click" event because the tap gesture recognizer is disabled once the controls are no longer visible.
+
+ * media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls-expected.txt: Added.
+ * media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls.html: Added.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247544 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-07-17 Antoine Quint <grao...@apple.com>
+
+ Unable to bring up custom media controls on iOS for video.sina.cn
+ https://bugs.webkit.org/show_bug.cgi?id=199889
+ <rdar://problem/51883919>
+
+ Reviewed by Dean Jackson.
+
+ This test replicates the scenario found on video.sina.cn that caused the issue: a <video> element originally has the "controls"
+ attribute and is not set to autoplay. This means the tap gesture recognizer is created to track a tap for the video to play. Then
+ the "controls" attribute is removed and we dispatch a tap on the video which would previously have *not* caused a "click" event to
+ eventually be dispatched on the <video> element since the tap gesture recognizer would call preventDefault(). With this patch applied,
+ we get the "click" event because the tap gesture recognizer is disabled once the controls are no longer visible.
+
+ * media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls-expected.txt: Added.
+ * media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls.html: Added.
+
+2019-07-17 Kocsen Chung <kocsen_ch...@apple.com>
+
Cherry-pick r247540. rdar://problem/53230036
[iOS WK2] Avoid lots of compositing backing store for offscreen position:fixed descendants
Added: branches/safari-608-branch/LayoutTests/media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls-expected.txt (0 => 247612)
--- branches/safari-608-branch/LayoutTests/media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls-expected.txt (rev 0)
+++ branches/safari-608-branch/LayoutTests/media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls-expected.txt 2019-07-18 20:25:01 UTC (rev 247612)
@@ -0,0 +1,13 @@
+Testing that a click event is correctly dispatched when tapping over a <video> element with default controls disabled after being originally enabled.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Media loaded, disabling controls.
+Tapping on media.
+Obtained click event.
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: branches/safari-608-branch/LayoutTests/media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls.html (0 => 247612)
--- branches/safari-608-branch/LayoutTests/media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls.html (rev 0)
+++ branches/safari-608-branch/LayoutTests/media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls.html 2019-07-18 20:25:01 UTC (rev 247612)
@@ -0,0 +1,29 @@
+<script src=""
+<script src="" type="text/_javascript_"></script>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<body>
+<video src="" style="width: 320px; position: absolute; left: 0; top: 0;" type="video/mp4" playsinline muted controls></video>
+<script type="text/_javascript_">
+
+window.jsTestIsAsync = true;
+
+description("Testing that a click event is correctly dispatched when tapping over a <video> element with default controls disabled after being originally enabled.");
+
+const media = document.querySelector("video");
+
+media.addEventListener("click", event => {
+ debug("Obtained click event.");
+ debug("");
+ finishJSTest();
+});
+
+media.addEventListener("loadedmetadata", event => {
+ debug("Media loaded, disabling controls.");
+ media.controls = false;
+ debug("Tapping on media.");
+ pressOnElement(media);
+});
+
+</script>
+<script src=""
+</body>
Modified: branches/safari-608-branch/Source/WebCore/ChangeLog (247611 => 247612)
--- branches/safari-608-branch/Source/WebCore/ChangeLog 2019-07-18 20:24:58 UTC (rev 247611)
+++ branches/safari-608-branch/Source/WebCore/ChangeLog 2019-07-18 20:25:01 UTC (rev 247612)
@@ -1,5 +1,80 @@
2019-07-17 Kocsen Chung <kocsen_ch...@apple.com>
+ Cherry-pick r247544. rdar://problem/53230040
+
+ Unable to bring up custom media controls on iOS for video.sina.cn
+ https://bugs.webkit.org/show_bug.cgi?id=199889
+ <rdar://problem/51883919>
+
+ Reviewed by Dean Jackson.
+
+ Source/WebCore:
+
+ Videos on video.sina.cn by default have the "controls" attribute and are set not to autoplay. This means that the original state
+ of the media controls are set to show the built-in media controls and also show the prominent play button to begin playback. The
+ display of the play button also requires a tap gesture recognizer, which calls preventDefault() when the "touchend" is received
+ to prevent double-tap-to-zoom, but also has the side-effect of preventing a "click" event from being dispatched for a tap.
+
+ The video.sina.cn code would eventually remove the "controls" attribute, which would make the built-in media controls not visible,
+ but still participate in hit-testing because we keep the shadow DOM around in order to potentially show the Airplay or picture-in-picture
+ placards. Additionally, we wouldn't disable the tap gesture recognizer when the "controls" attribute was removed.
+
+ We now ensure that both gesture recognizers used by iOS inline media controls are only enabled when media controls are visible.
+
+ Test: media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls.html
+
+ * Modules/modern-media-controls/controls/ios-inline-media-controls.js:
+ (IOSInlineMediaControls.prototype.set showsStartButton):
+ (IOSInlineMediaControls.prototype.get visible):
+ (IOSInlineMediaControls.prototype.set visible):
+ (IOSInlineMediaControls.prototype._updateGestureRecognizers):
+ (IOSInlineMediaControls.prototype._tapGestureRecognizerStateDidChange):
+ (IOSInlineMediaControls.prototype._pinchGestureRecognizerStateDidChange):
+
+ LayoutTests:
+
+ This test replicates the scenario found on video.sina.cn that caused the issue: a <video> element originally has the "controls"
+ attribute and is not set to autoplay. This means the tap gesture recognizer is created to track a tap for the video to play. Then
+ the "controls" attribute is removed and we dispatch a tap on the video which would previously have *not* caused a "click" event to
+ eventually be dispatched on the <video> element since the tap gesture recognizer would call preventDefault(). With this patch applied,
+ we get the "click" event because the tap gesture recognizer is disabled once the controls are no longer visible.
+
+ * media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls-expected.txt: Added.
+ * media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls.html: Added.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247544 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-07-17 Antoine Quint <grao...@apple.com>
+
+ Unable to bring up custom media controls on iOS for video.sina.cn
+ https://bugs.webkit.org/show_bug.cgi?id=199889
+ <rdar://problem/51883919>
+
+ Reviewed by Dean Jackson.
+
+ Videos on video.sina.cn by default have the "controls" attribute and are set not to autoplay. This means that the original state
+ of the media controls are set to show the built-in media controls and also show the prominent play button to begin playback. The
+ display of the play button also requires a tap gesture recognizer, which calls preventDefault() when the "touchend" is received
+ to prevent double-tap-to-zoom, but also has the side-effect of preventing a "click" event from being dispatched for a tap.
+
+ The video.sina.cn code would eventually remove the "controls" attribute, which would make the built-in media controls not visible,
+ but still participate in hit-testing because we keep the shadow DOM around in order to potentially show the Airplay or picture-in-picture
+ placards. Additionally, we wouldn't disable the tap gesture recognizer when the "controls" attribute was removed.
+
+ We now ensure that both gesture recognizers used by iOS inline media controls are only enabled when media controls are visible.
+
+ Test: media/modern-media-controls/media-controller/ios/media-controller-allows-click-over-video-with-no-controls.html
+
+ * Modules/modern-media-controls/controls/ios-inline-media-controls.js:
+ (IOSInlineMediaControls.prototype.set showsStartButton):
+ (IOSInlineMediaControls.prototype.get visible):
+ (IOSInlineMediaControls.prototype.set visible):
+ (IOSInlineMediaControls.prototype._updateGestureRecognizers):
+ (IOSInlineMediaControls.prototype._tapGestureRecognizerStateDidChange):
+ (IOSInlineMediaControls.prototype._pinchGestureRecognizerStateDidChange):
+
+2019-07-17 Kocsen Chung <kocsen_ch...@apple.com>
+
Cherry-pick r247541. rdar://problem/53230029
Unable to tap buttons at top of Wells Fargo app’s Payees screen
Modified: branches/safari-608-branch/Source/WebCore/Modules/modern-media-controls/controls/ios-inline-media-controls.js (247611 => 247612)
--- branches/safari-608-branch/Source/WebCore/Modules/modern-media-controls/controls/ios-inline-media-controls.js 2019-07-18 20:24:58 UTC (rev 247611)
+++ branches/safari-608-branch/Source/WebCore/Modules/modern-media-controls/controls/ios-inline-media-controls.js 2019-07-18 20:25:01 UTC (rev 247612)
@@ -34,7 +34,7 @@
this.element.classList.add("ios");
- this._pinchGestureRecognizer = new PinchGestureRecognizer(this.element, this);
+ this._updateGestureRecognizers();
}
// Public
@@ -47,13 +47,20 @@
set showsStartButton(flag)
{
super.showsStartButton = flag;
+ this._updateGestureRecognizers();
+ }
- if (!flag)
- delete this._tapGestureRecognizer;
- else if (!this._tapGestureRecognizer)
- this._tapGestureRecognizer = new TapGestureRecognizer(this.element, this);
+ get visible()
+ {
+ return super.visible;
}
+ set visible(flag)
+ {
+ super.visible = flag;
+ this._updateGestureRecognizers();
+ }
+
// Protected
gestureRecognizerStateDidChange(recognizer)
@@ -66,8 +73,29 @@
// Private
+ _updateGestureRecognizers()
+ {
+ const shouldListenToPinches = this.visible;
+ const shouldListenToTaps = this.visible && this.showsStartButton;
+
+ if (shouldListenToPinches && !this._pinchGestureRecognizer)
+ this._pinchGestureRecognizer = new PinchGestureRecognizer(this.element, this);
+ else if (!shouldListenToPinches && this._pinchGestureRecognizer) {
+ this._pinchGestureRecognizer.enabled = false;
+ delete this._pinchGestureRecognizer;
+ }
+
+ if (shouldListenToTaps && !this._tapGestureRecognizer)
+ this._tapGestureRecognizer = new TapGestureRecognizer(this.element, this);
+ else if (!shouldListenToTaps && this._tapGestureRecognizer) {
+ this._tapGestureRecognizer.enabled = false;
+ delete this._tapGestureRecognizer;
+ }
+ }
+
_pinchGestureRecognizerStateDidChange(recognizer)
{
+ console.assert(this.visible);
if (recognizer.state !== GestureRecognizer.States.Ended && recognizer.state !== GestureRecognizer.States.Changed)
return;
@@ -77,7 +105,7 @@
_tapGestureRecognizerStateDidChange(recognizer)
{
- console.assert(this.showsStartButton);
+ console.assert(this.visible && this.showsStartButton);
if (recognizer.state === GestureRecognizer.States.Recognized && this.delegate && typeof this.delegate.iOSInlineMediaControlsRecognizedTapGesture === "function")
this.delegate.iOSInlineMediaControlsRecognizedTapGesture();
}
