Fixed volume slider element event handling

third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp

Index: third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp
diff --git a/third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp b/third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp
index dbf7802053de4e74a2d62a16a1ecee66c8cfe946..e2dee6c342009fc1f3114311cf02572db44441b7 100644
--- a/third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp
+++ b/third_party/WebKit/Source/core/html/shadow/MediaControlElements.cpp
@@ -97,7 +97,11 @@ bool isUserInteractionEventForSlider(Event* event, LayoutObject* layoutObject) {
 
   const AtomicString& type = event->type();
   return type == EventTypeNames::mouseover ||
-         type == EventTypeNames::mouseout || type == EventTypeNames::mousemove;
+         type == EventTypeNames::mouseout ||
+         type == EventTypeNames::mousemove ||
+         type == EventTypeNames::pointerover ||
+         type == EventTypeNames::pointerout ||
+         type == EventTypeNames::pointermove;
 }
 
 Element* elementFromCenter(Element& element) {
@@ -755,7 +759,10 @@ void MediaControlTimelineElement::defaultEventHandler(Event* event) {
 
   if (event->type() == EventTypeNames::mouseover ||
       event->type() == EventTypeNames::mouseout ||
-      event->type() == EventTypeNames::mousemove)
+      event->type() == EventTypeNames::mousemove ||
+      event->type() == EventTypeNames::pointerover ||
+      event->type() == EventTypeNames::pointerout ||
+      event->type() == EventTypeNames::pointermove)

[liberato (no reviews please), 2017/01/11 16:07:26]

can we (eventually) just ignore mouse* here in favor of pointers?  
if so, might be worth a TODO.

i think that mouse vs pointer does have some special-case logic for when to
restart the timer to hide the control bar.  otherwise, i'm not sure that we'd
want to care about the difference.

[mustaq, 2017/01/11 17:34:35]

I totally agree, will add a TODO. It would involve a change across all UI
elements. I didn't attempt it here because I am not familiar with the code. E.g.
I don't know how isUserInteractionEventForSlider() affects events through nested
elements.

[mustaq, 2017/01/11 21:24:58]

Done.

     return;
 
   double time = value().toDouble();
@@ -766,6 +773,11 @@ void MediaControlTimelineElement::defaultEventHandler(Event* event) {
       mediaElement().setCurrentTime(time);
   }
 
+  // TODO(mustaq): On a single click, this block is executed 5 or 6 times
+  // depending on whether the slider position has been changed or not, once for
+  // each of these events: pointerdown, mousedown, (input?), pointerup, mouseup,
+  // DOMActive, click. Perhaps we only care about the "input" event (as in
+  // MediaControlVolumeSliderElement::defaultEventHandler)?
   LayoutSliderItem slider = LayoutSliderItem(toLayoutSlider(layoutObject()));
   if (!slider.isNull() && slider.inDragMode())
     mediaControls().updateCurrentTimeDisplay();
@@ -813,21 +825,11 @@ MediaControlVolumeSliderElement* MediaControlVolumeSliderElement::create(
 }
 
 void MediaControlVolumeSliderElement::defaultEventHandler(Event* event) {
-  if (event->isMouseEvent() &&
-      toMouseEvent(event)->button() !=
-          static_cast<short>(WebPointerProperties::Button::Left))
-    return;
-
   if (!isConnected() || !document().isActive())
     return;
 
   MediaControlInputElement::defaultEventHandler(event);
 
-  if (event->type() == EventTypeNames::mouseover ||
-      event->type() == EventTypeNames::mouseout ||
-      event->type() == EventTypeNames::mousemove)
-    return;
-
   if (event->type() == EventTypeNames::mousedown)
     Platform::current()->recordAction(
         UserMetricsAction("Media.Controls.VolumeChangeBegin"));
@@ -836,9 +838,11 @@ void MediaControlVolumeSliderElement::defaultEventHandler(Event* event) {
     Platform::current()->recordAction(
         UserMetricsAction("Media.Controls.VolumeChangeEnd"));
 
-  double volume = value().toDouble();
-  mediaElement().setVolume(volume);
-  mediaElement().setMuted(false);
+  if (event->type() == EventTypeNames::input) {
+    double volume = value().toDouble();
+    mediaElement().setVolume(volume);
+    mediaElement().setMuted(false);
+  }
 }
 
 bool MediaControlVolumeSliderElement::willRespondToMouseMoveEvents() {