[Modern Media Controls] Clicking on the tracks button when the tracks panel is up...
authorgraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Mar 2017 19:44:09 +0000 (19:44 +0000)
committergraouts@webkit.org <graouts@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Mar 2017 19:44:09 +0000 (19:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=168517
<rdar://problem/30577636>

Reviewed by Dean Jackson.

Source/WebCore:

We completely turn off default event handling in MediaDocument.cpp since we're implementing the
behavior we expect to pause and resume the video in the modern-media-controls module already. This
gets rid of this odd case where the content would not see the "click" event while the C++ side would
handle it and pause the video.

* Modules/modern-media-controls/media/media-controller.js:
(MediaController):
(MediaController.prototype.handleEvent):
(MediaController.prototype._containerWasClicked): Deleted.
* html/MediaDocument.cpp:
(WebCore::MediaDocument::defaultEventHandler):

LayoutTests:

* media/video-click-dblckick-standalone.html: We disable modern-media-controls here since we know that this test won't pass with them on.

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

LayoutTests/ChangeLog
LayoutTests/media/video-click-dblckick-standalone.html
Source/WebCore/ChangeLog
Source/WebCore/Modules/modern-media-controls/media/media-controller.js
Source/WebCore/html/MediaDocument.cpp

index 5325519..e3cff50 100644 (file)
@@ -1,3 +1,13 @@
+2017-03-27  Antoine Quint  <graouts@apple.com>
+
+        [Modern Media Controls] Clicking on the tracks button when the tracks panel is up in a media document pauses the video
+        https://bugs.webkit.org/show_bug.cgi?id=168517
+        <rdar://problem/30577636>
+
+        Reviewed by Dean Jackson.
+
+        * media/video-click-dblckick-standalone.html: We disable modern-media-controls here since we know that this test won't pass with them on.
+
 2017-03-27  Ryan Haddad  <ryanhaddad@apple.com>
 
         Mark media/modern-media-controls/time-label/time-label-white-space-nowrap.html as flaky.
index 7fc83d8..22b0a9e 100644 (file)
@@ -1,3 +1,4 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableModernMediaControls=false ] -->
 <html>
     <head>
         <script src=media-file.js></script>
index 24428b1..d417eab 100644 (file)
@@ -1,3 +1,23 @@
+2017-03-27  Antoine Quint  <graouts@apple.com>
+
+        [Modern Media Controls] Clicking on the tracks button when the tracks panel is up in a media document pauses the video
+        https://bugs.webkit.org/show_bug.cgi?id=168517
+        <rdar://problem/30577636>
+
+        Reviewed by Dean Jackson.
+
+        We completely turn off default event handling in MediaDocument.cpp since we're implementing the
+        behavior we expect to pause and resume the video in the modern-media-controls module already. This
+        gets rid of this odd case where the content would not see the "click" event while the C++ side would
+        handle it and pause the video.
+
+        * Modules/modern-media-controls/media/media-controller.js:
+        (MediaController):
+        (MediaController.prototype.handleEvent):
+        (MediaController.prototype._containerWasClicked): Deleted.
+        * html/MediaDocument.cpp:
+        (WebCore::MediaDocument::defaultEventHandler):
+
 2017-03-27  Youenn Fablet  <youenn@apple.com>
 
         Tighten RTCDatachannel creation and parameter getters
index ed8c26b..af6074d 100644 (file)
@@ -36,7 +36,6 @@ class MediaController
 
         this.container = shadowRoot.appendChild(document.createElement("div"));
         this.container.className = "media-controls-container";
-        this.container.addEventListener("click", this, true);
 
         if (host) {
             host.controlsDependOnPageScaleFactor = this.layoutTraits & LayoutTraits.iOS;
@@ -133,9 +132,7 @@ class MediaController
             this._updateControlsIfNeeded();
             // We must immediately perform layouts so that we don't lag behind the media layout size.
             scheduler.flushScheduledLayoutCallbacks();
-        } else if (event.type === "click" && event.currentTarget === this.container)
-            this._containerWasClicked(event);
-        else if (event.currentTarget === this.media) {
+        } else if (event.currentTarget === this.media) {
             this._updateControlsIfNeeded();
             if (event.type === "webkitpresentationmodechanged")
                 this._returnMediaLayerToInlineIfNeeded();
@@ -144,13 +141,6 @@ class MediaController
 
     // Private
 
-    _containerWasClicked(event)
-    {
-        // We need to call preventDefault() here since, in the case of Media Documents,
-        // playback may be toggled when clicking on the video.
-        event.preventDefault();
-    }
-
     _updateControlsIfNeeded()
     {
         const layoutTraits = this.layoutTraits;
index 4de40a1..4624168 100644 (file)
@@ -181,7 +181,12 @@ static inline HTMLVideoElement* ancestorVideoElement(Node* node)
 
 void MediaDocument::defaultEventHandler(Event& event)
 {
-    // Match the default Quicktime plugin behavior to allow 
+    // Modern media controls have their own event handling to determine when to
+    // pause or resume playback.
+    if (RuntimeEnabledFeatures::sharedFeatures().modernMediaControlsEnabled())
+        return;
+    
+    // Match the default Quicktime plugin behavior to allow
     // clicking and double-clicking to pause and play the media.
     Node* targetNode = event.target()->toNode();
     if (!targetNode)