Diff
Modified: trunk/LayoutTests/ChangeLog (219551 => 219552)
--- trunk/LayoutTests/ChangeLog 2017-07-17 02:10:42 UTC (rev 219551)
+++ trunk/LayoutTests/ChangeLog 2017-07-17 05:29:20 UTC (rev 219552)
@@ -1,3 +1,26 @@
+2017-07-16 Antoine Quint <[email protected]>
+
+ Dismissing the captions panel using the mouse is too eager to remove the captions panel and media controls
+ https://bugs.webkit.org/show_bug.cgi?id=174571
+ <rdar://problem/33294968>
+
+ Reviewed by Eric Carlson.
+
+ Adding a method to show the tracks panel for a given shadow root and adding new tests to check the correct
+ behavior when dismissing the tracks panel when clicking over the media element or outside the media element
+ when it's paused.
+
+ * media/modern-media-controls/resources/media-controls-utils.js:
+ (pressOnElement):
+ (pressAtPoint):
+ (showTracksPanel):
+ (finishMediaControlsTest): Deleted.
+ * media/modern-media-controls/tracks-panel/tracks-panel-up-click-outside-media-does-not-dimiss-media-controls-when-media-is-paused-expected.txt: Added.
+ * media/modern-media-controls/tracks-panel/tracks-panel-up-click-outside-media-does-not-dimiss-media-controls-when-media-is-paused.html: Added.
+ * media/modern-media-controls/tracks-panel/tracks-panel-up-click-over-media-does-not-dimiss-media-controls-when-media-is-playing-expected.txt: Added.
+ * media/modern-media-controls/tracks-panel/tracks-panel-up-click-over-media-does-not-dimiss-media-controls-when-media-is-playing.html: Added.
+ * platform/mac/TestExpectations:
+
2017-07-16 Ali Juma <[email protected]>
DisallowUserAgentShadowContent moves out of non-UA shadow roots
Modified: trunk/LayoutTests/media/modern-media-controls/resources/media-controls-utils.js (219551 => 219552)
--- trunk/LayoutTests/media/modern-media-controls/resources/media-controls-utils.js 2017-07-17 02:10:42 UTC (rev 219551)
+++ trunk/LayoutTests/media/modern-media-controls/resources/media-controls-utils.js 2017-07-17 05:29:20 UTC (rev 219552)
@@ -60,19 +60,47 @@
// debug(`Trying to press on <${element.localName} class="${element.className}"> at ${centerX}x${centerY}.`);
+ pressAtPoint(centerX, centerY, continuation);
+
+ return true;
+}
+
+function pressAtPoint(x, y, continuation)
+{
+ if (typeof continuation !== "function")
+ continuation = new Function;
+
if ("createTouch" in document) {
testRunner.runUIScript(`
- uiController.singleTapAtPoint(${centerX}, ${centerY}, function() {
+ uiController.singleTapAtPoint(${x}, ${y}, function() {
uiController.uiScriptComplete("Done");
});`, continuation);
} else {
- eventSender.mouseMoveTo(centerX, centerY);
+ eventSender.mouseMoveTo(x, y);
eventSender.mouseDown();
eventSender.mouseUp();
continuation();
}
+}
- return true;
+function showTracksPanel(shadowRoot, continuation)
+{
+ if (typeof continuation !== "function")
+ continuation = new Function;
+
+ shouldBecomeDifferent("shadowRoot.querySelector('button.tracks')", "null", () => {
+ shouldBecomeDifferent("shadowRoot.querySelector('button.tracks').getBoundingClientRect().width", "0", () => {
+ debug("=> Tracks button is visible.")
+ debug("");
+ debug("Pressing on the tracks button.");
+ pressOnElement(shadowRoot.querySelector("button.tracks"));
+ shouldBecomeDifferent("shadowRoot.querySelector('.tracks-panel')", "null", () => {
+ debug("=> Tracks panel is visible.")
+ debug("");
+ continuation();
+ });
+ });
+ });
}
function finishMediaControlsTest()
Added: trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-outside-media-does-not-dimiss-media-controls-when-media-is-paused-expected.txt (0 => 219552)
--- trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-outside-media-does-not-dimiss-media-controls-when-media-is-paused-expected.txt (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-outside-media-does-not-dimiss-media-controls-when-media-is-paused-expected.txt 2017-07-17 05:29:20 UTC (rev 219552)
@@ -0,0 +1,23 @@
+Clicking outside the media when the tracks panel is up should dismiss the tracks panel but not the media controls.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Media started playing and was paused.
+
+PASS shadowRoot.querySelector('button.tracks') became different from null
+PASS shadowRoot.querySelector('button.tracks').getBoundingClientRect().width became different from 0
+=> Tracks button is visible.
+
+Pressing on the tracks button.
+PASS shadowRoot.querySelector('.tracks-panel') became different from null
+=> Tracks panel is visible.
+
+Clicking outside the media to dismiss the tracks panel.
+PASS shadowRoot.querySelector('.media-controls').classList.contains('faded') is false
+PASS shadowRoot.querySelector('.tracks-panel').classList.contains('fade-out') is true
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-outside-media-does-not-dimiss-media-controls-when-media-is-paused.html (0 => 219552)
--- trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-outside-media-does-not-dimiss-media-controls-when-media-is-paused.html (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-outside-media-does-not-dimiss-media-controls-when-media-is-paused.html 2017-07-17 05:29:20 UTC (rev 219552)
@@ -0,0 +1,34 @@
+<script src=""
+<script src="" type="text/_javascript_"></script>
+<body>
+<video src="" style="position: absolute; left: 10px; top: 10px; width: 640px; height: 360px;" controls autoplay></video>
+<script type="text/_javascript_">
+
+window.jsTestIsAsync = true;
+
+description("Clicking outside the media when the tracks panel is up should dismiss the tracks panel but not the media controls.");
+
+const media = document.querySelector("video");
+const shadowRoot = window.internals.shadowRoot(media);
+
+media.addEventListener("play", () => {
+ media.pause();
+
+ debug("Media started playing and was paused.");
+ debug("");
+ showTracksPanel(shadowRoot, () => {
+ debug("Clicking outside the media to dismiss the tracks panel.");
+ pressAtPoint(5, 5, () => {
+ requestAnimationFrame(() => {
+ shouldBeFalse("shadowRoot.querySelector('.media-controls').classList.contains('faded')");
+ shouldBeTrue("shadowRoot.querySelector('.tracks-panel').classList.contains('fade-out')");
+ debug("");
+ finishJSTest();
+ });
+ });
+ });
+});
+
+</script>
+<script src=""
+</body>
Added: trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-over-media-does-not-dimiss-media-controls-when-media-is-playing-expected.txt (0 => 219552)
--- trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-over-media-does-not-dimiss-media-controls-when-media-is-playing-expected.txt (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-over-media-does-not-dimiss-media-controls-when-media-is-playing-expected.txt 2017-07-17 05:29:20 UTC (rev 219552)
@@ -0,0 +1,23 @@
+Clicking over the media when the tracks panel is up and the media is playing should dismiss the tracks panel but not the media controls.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Media started playing.
+
+PASS shadowRoot.querySelector('button.tracks') became different from null
+PASS shadowRoot.querySelector('button.tracks').getBoundingClientRect().width became different from 0
+=> Tracks button is visible.
+
+Pressing on the tracks button.
+PASS shadowRoot.querySelector('.tracks-panel') became different from null
+=> Tracks panel is visible.
+
+Clicking over the media to dismiss the tracks panel.
+PASS shadowRoot.querySelector('.media-controls').classList.contains('faded') is false
+PASS shadowRoot.querySelector('.tracks-panel').classList.contains('fade-out') is true
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-over-media-does-not-dimiss-media-controls-when-media-is-playing.html (0 => 219552)
--- trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-over-media-does-not-dimiss-media-controls-when-media-is-playing.html (rev 0)
+++ trunk/LayoutTests/media/modern-media-controls/tracks-panel/tracks-panel-up-click-over-media-does-not-dimiss-media-controls-when-media-is-playing.html 2017-07-17 05:29:20 UTC (rev 219552)
@@ -0,0 +1,32 @@
+<script src=""
+<script src="" type="text/_javascript_"></script>
+<body>
+<video src="" style="position: absolute; left: 10px; top: 10px; width: 640px; height: 360px;" controls autoplay></video>
+<script type="text/_javascript_">
+
+window.jsTestIsAsync = true;
+
+description("Clicking over the media when the tracks panel is up and the media is playing should dismiss the tracks panel but not the media controls.");
+
+const media = document.querySelector("video");
+const shadowRoot = window.internals.shadowRoot(media);
+
+media.addEventListener("play", () => {
+ debug("Media started playing.");
+ debug("");
+ showTracksPanel(shadowRoot, () => {
+ debug("Clicking over the media to dismiss the tracks panel.");
+ pressAtPoint(11, 11, () => {
+ requestAnimationFrame(() => {
+ shouldBeFalse("shadowRoot.querySelector('.media-controls').classList.contains('faded')");
+ shouldBeTrue("shadowRoot.querySelector('.tracks-panel').classList.contains('fade-out')");
+ debug("");
+ finishJSTest();
+ });
+ });
+ });
+});
+
+</script>
+<script src=""
+</body>
Modified: trunk/LayoutTests/platform/mac/TestExpectations (219551 => 219552)
--- trunk/LayoutTests/platform/mac/TestExpectations 2017-07-17 02:10:42 UTC (rev 219551)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2017-07-17 05:29:20 UTC (rev 219552)
@@ -1578,6 +1578,8 @@
media/modern-media-controls/status-label [ Pass ]
media/modern-media-controls/time-control [ Pass ]
media/modern-media-controls/time-label [ Pass ]
+media/modern-media-controls/tracks-panel/tracks-panel-up-click-outside-media-does-not-dimiss-media-controls-when-media-is-paused.html [ Pass ]
+media/modern-media-controls/tracks-panel/tracks-panel-up-click-over-media-does-not-dimiss-media-controls-when-media-is-playing.html [ Pass ]
media/modern-media-controls/tracks-button [ Pass ]
media/modern-media-controls/volume-up-support [ Pass ]
Modified: trunk/Source/WebCore/ChangeLog (219551 => 219552)
--- trunk/Source/WebCore/ChangeLog 2017-07-17 02:10:42 UTC (rev 219551)
+++ trunk/Source/WebCore/ChangeLog 2017-07-17 05:29:20 UTC (rev 219552)
@@ -1,3 +1,40 @@
+2017-07-16 Antoine Quint <[email protected]>
+
+ Dismissing the captions panel using the mouse is too eager to remove the captions panel and media controls
+ https://bugs.webkit.org/show_bug.cgi?id=174571
+ <rdar://problem/33294968>
+
+ Reviewed by Eric Carlson.
+
+ We did several things wrong when dismissing the tracks panel:
+
+ - we did not check whether we were hosted in a shadow root when figuring if a click was on the tracks panel
+ - we did not check whether we clicked over the media when dismissing the tracks panel
+ - we did not check whether auto-hide was on before fading the media controls out when we clicked outside
+ the media controls bounds
+
+ We now correctly account for all of those cases and implement the following behavior when clickng as the tracks
+ panel is presented:
+
+ - dismiss the panel if the click is outside of the panel
+ - dismiss the panel and the media controls if the click is outside the video and the media controls have
+ auto-hide on (ie. media is playing)
+ - dismiss the panel and the media controls after the track selection animation is finished if a track is selected
+
+ Tests: media/modern-media-controls/tracks-panel/tracks-panel-up-click-outside-media-does-not-dimiss-media-controls-when-media-is-paused.html
+ media/modern-media-controls/tracks-panel/tracks-panel-up-click-over-media-does-not-dimiss-media-controls-when-media-is-playing.html
+
+ * Modules/modern-media-controls/controls/media-controls.js:
+ (MediaControls.prototype.hideTracksPanel): Only hide the media controls if we clicked outside of the media
+ controls bounds and if we have auto-hide on when idle (ie. the media is playing).
+ (MediaControls.prototype.isPointInControls): Add an option to specify whether the container should be
+ considered when checking if a point is contained within the media controls bounds.
+ * Modules/modern-media-controls/controls/tracks-panel.js:
+ (TracksPanel.prototype._handleMousedown):
+ (TracksPanel.prototype._isPointInTracksPanel): Correctly check whether the element that we started pressing
+ on is contained within the tracks panel, accounting for the case where we are presented within a shadow root
+ (ie. always when runing inside a Web page).
+
2017-07-16 Ali Juma <[email protected]>
DisallowUserAgentShadowContent moves out of non-UA shadow roots
Modified: trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js (219551 => 219552)
--- trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js 2017-07-17 02:10:42 UTC (rev 219551)
+++ trunk/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js 2017-07-17 05:29:20 UTC (rev 219552)
@@ -187,12 +187,12 @@
let shouldFadeControlsBar = true;
if (window.event instanceof MouseEvent)
- shouldFadeControlsBar = !this.isPointInControls(new DOMPoint(event.clientX, event.clientY));
+ shouldFadeControlsBar = !this.isPointInControls(new DOMPoint(event.clientX, event.clientY), true);
this.tracksButton.on = false;
this.tracksButton.element.focus();
this.autoHideController.hasSecondaryUIAttached = false;
- this.faded = shouldFadeControlsBar;
+ this.faded = this.autoHideController.fadesWhileIdle && shouldFadeControlsBar;
this.tracksPanel.hide();
}
@@ -201,7 +201,7 @@
this.element.classList.add("fade-in");
}
- isPointInControls(point)
+ isPointInControls(point, includeContainer)
{
let ancestor = this.element.parentNode;
while (ancestor && !(ancestor instanceof ShadowRoot))
@@ -212,6 +212,10 @@
return false;
const tappedElement = shadowRoot.elementFromPoint(point.x, point.y);
+
+ if (includeContainer && this.element === tappedElement)
+ return true;
+
return this.children.some(child => child.element.contains(tappedElement));
}
Modified: trunk/Source/WebCore/Modules/modern-media-controls/controls/tracks-panel.js (219551 => 219552)
--- trunk/Source/WebCore/Modules/modern-media-controls/controls/tracks-panel.js 2017-07-17 02:10:42 UTC (rev 219551)
+++ trunk/Source/WebCore/Modules/modern-media-controls/controls/tracks-panel.js 2017-07-17 05:29:20 UTC (rev 219552)
@@ -190,7 +190,7 @@
_handleMousedown(event)
{
- if (this.element.contains(event.target))
+ if (this._isPointInTracksPanel(new DOMPoint(event.clientX, event.clientY)))
return;
this._dismiss();
@@ -199,6 +199,18 @@
event.stopPropagation();
}
+ _isPointInTracksPanel(point)
+ {
+ let ancestor = this.element.parentNode;
+ while (ancestor && !(ancestor instanceof ShadowRoot))
+ ancestor = ancestor.parentNode;
+
+ if (!ancestor)
+ ancestor = document;
+
+ return this.element.contains(ancestor.elementFromPoint(point.x, point.y));
+ }
+
_handleKeydown(event)
{
switch (event.key) {
