Fixed volume slider element event handling

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

 /*
  * Copyright (C) 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 79 matching lines @@
   if (isUserInteractionEvent(event))
     return true;
 
   // Some events are only captured during a slider drag.
   LayoutSliderItem slider = LayoutSliderItem(toLayoutSlider(layoutObject));
   if (!slider.isNull() && !slider.inDragMode())
     return false;
 
   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) {
   ClientRect* clientRect = element.getBoundingClientRect();
   int centerX =
       static_cast<int>((clientRect->left() + clientRect->right()) / 2);
   int centerY =
       static_cast<int>((clientRect->top() + clientRect->bottom()) / 2);
 
   return element.document().elementFromPoint(centerX, centerY);
@@ skipping 637 matching lines @@
   if (event->type() == EventTypeNames::mouseup) {
     Platform::current()->recordAction(
         UserMetricsAction("Media.Controls.ScrubbingEnd"));
     mediaControls().endScrubbing();
   }
 
   MediaControlInputElement::defaultEventHandler(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();
   if (event->type() == EventTypeNames::input) {
     // FIXME: This will need to take the timeline offset into consideration
     // once that concept is supported, see https://crbug.com/312699
     if (mediaElement().seekable()->contain(time))
       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();
 }
 
 bool MediaControlTimelineElement::willRespondToMouseClickEvents() {
   return isConnected() && document().isActive();
 }
 
 void MediaControlTimelineElement::setPosition(double currentTime) {
@@ skipping 27 matching lines @@
   slider->ensureUserAgentShadowRoot();
   slider->setType(InputTypeNames::range);
   slider->setAttribute(stepAttr, "any");
   slider->setAttribute(maxAttr, "1");
   slider->setShadowPseudoId(
       AtomicString("-webkit-media-controls-volume-slider"));
   return slider;
 }
 
 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"));
 
   if (event->type() == EventTypeNames::mouseup)
     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() {
   if (!isConnected() || !document().isActive())
     return false;
 
   return MediaControlInputElement::willRespondToMouseMoveEvents();
 }
 
 bool MediaControlVolumeSliderElement::willRespondToMouseClickEvents() {
@@ skipping 189 matching lines @@
 MediaControlCurrentTimeDisplayElement*
 MediaControlCurrentTimeDisplayElement::create(MediaControls& mediaControls) {
   MediaControlCurrentTimeDisplayElement* element =
       new MediaControlCurrentTimeDisplayElement(mediaControls);
   element->setShadowPseudoId(
       AtomicString("-webkit-media-controls-current-time-display"));
   return element;
 }
 
 }  // namespace blink