Skip to content

GitLab

T
tangled
Commit f05ffc1a authored by Tommy Steimel's avatar Tommy Steimel Committed by Commit Bot
Browse files
Options

Ignore certain pointer events when the user interacts via touch 

This CL adds a bool to track whether the user is interacting via touch
or mouse. When the user is interacting via touch, we ignore certain
pointer events that were causing many issues on Android (or any touch-
enabled device).

In particular, this fixes an issue where users were unable to use the
overlay play button in certain cases. The underlying chain of events
that caused the button to not receive a tap was:

1) While controls are shown, user taps on overlay play button to pause
2) MediaControlsImpl sees a pointerout event from the user's tap, and
   stops the hide timer
3) OnTimeUpdate fired, seeing a stopped timer and hiding the controls
4) MediaControlsImpl sees a gesturetap from the user's tap, and decides
   to show the controls
5) The controls reappear, but the user's tap has already been handled
   and the overlay play button sees nothing

Bug: 822763
Change-Id: Ia085c7f1cd082c0f44b60f22333146cd6de5f841
Reviewed-on: https://chromium-review.googlesource.com/982900Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Reviewed-by: default avatarBecca Hughes <beccahughes@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547695}
parent 0e554b21
Hide whitespace changes
Inline Side-by-side
Showing with 98 additions and 64 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ f05ffc1a
...@@ -1207,6 +1207,7 @@ crbug.com/613672 [ Mac ] virtual/mouseevent_fractional/fast/events/pointerevents ...@@ -1207,6 +1207,7 @@ crbug.com/613672 [ Mac ] virtual/mouseevent_fractional/fast/events/pointerevents
crbug.com/613672 [ Mac ] virtual/mouseevent_fractional/fast/events/synthetic-events/tap-on-scaled-screen.html [ Skip ] crbug.com/613672 [ Mac ] virtual/mouseevent_fractional/fast/events/synthetic-events/tap-on-scaled-screen.html [ Skip ]
crbug.com/613672 [ Mac ] virtual/scroll_customization/fast/scroll-behavior/scroll-customization/scroll-customization-property.html [ Skip ] crbug.com/613672 [ Mac ] virtual/scroll_customization/fast/scroll-behavior/scroll-customization/scroll-customization-property.html [ Skip ]
crbug.com/613672 [ Mac ] virtual/modern-media-controls/media/controls/modern/tap-to-hide-controls.html [ Skip ] crbug.com/613672 [ Mac ] virtual/modern-media-controls/media/controls/modern/tap-to-hide-controls.html [ Skip ]
crbug.com/613672 [ Mac ] virtual/modern-media-controls/media/controls/modern/singletouch-on-play-button.html [ Skip ]
# We should send PointerLeave events for stylus devices. # We should send PointerLeave events for stylus devices.
crbug.com/583413 external/wpt/pointerevents/pointerevent_pointerleave_pen-manual.html [ Failure ] crbug.com/583413 external/wpt/pointerevents/pointerevent_pointerleave_pen-manual.html [ Failure ]
......
third_party/WebKit/LayoutTests/media/controls/modern/singletouch-on-play-button.html 0 → 100644
View file @ f05ffc1a
<!DOCTYPE html>
<html>
<title>Test that the player pauses if single-touched on the play button.</title>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<script src="../../media-controls.js"></script>
<video controls width=400 src="../../content/60_sec_video.webm"></video>
<script>
async_test(t => {
const video = document.querySelector('video');
video.addEventListener('playing', t.step_func(() => {
// Single tap in the middle of the button.
const coordinates =
elementCoordinates(mediaControlsOverlayPlayButtonInternal(video));
singleTouchAtCoordinates(coordinates[0], coordinates[1]);
}), { once: true });
video.addEventListener('pause', t.step_func_done(), { once: true });
video.play();
});
</script>
</html>
third_party/WebKit/LayoutTests/media/media-controls.js
View file @ f05ffc1a
...@@ -404,6 +404,8 @@ function singleTapOnControl(control, callback) { ...@@ -404,6 +404,8 @@ function singleTapOnControl(control, callback) {
singleTapAtCoordinates(coordinates[0], coordinates[1], callback); singleTapAtCoordinates(coordinates[0], coordinates[1], callback);
} }
// This function does not work on Mac due to crbug.com/613672. When using this
// function, add an entry into TestExpectations to skip on Mac.
function singleTouchAtCoordinates(xPos, yPos, callback) { function singleTouchAtCoordinates(xPos, yPos, callback) {
chrome.gpuBenchmarking.pointerActionSequence([ chrome.gpuBenchmarking.pointerActionSequence([
{ {
......
third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp
View file @ f05ffc1a
...@@ -362,6 +362,11 @@ MediaControlsImpl::MediaControlsImpl(HTMLMediaElement& media_element) ...@@ -362,6 +362,11 @@ MediaControlsImpl::MediaControlsImpl(HTMLMediaElement& media_element)
this, this,
&MediaControlsImpl::ElementSizeChangedTimerFired), &MediaControlsImpl::ElementSizeChangedTimerFired),
keep_showing_until_timer_fires_(false) { keep_showing_until_timer_fires_(false) {
// On touch devices, start with the assumption that the user will interact via
// touch events.
Settings* settings = media_element.GetDocument().GetSettings();
is_touch_interaction_ = settings ? settings->GetMaxTouchPoints() > 0 : false;
resize_observer_->observe(&media_element); resize_observer_->observe(&media_element);
} }
...@@ -835,7 +840,6 @@ void MediaControlsImpl::MakeOpaqueFromPointerEvent() { ...@@ -835,7 +840,6 @@ void MediaControlsImpl::MakeOpaqueFromPointerEvent() {
return; return;
MakeOpaque(); MakeOpaque();
pointer_event_did_show_controls_ = true;
} }
void MediaControlsImpl::MakeTransparent() { void MediaControlsImpl::MakeTransparent() {
...@@ -1191,12 +1195,9 @@ void MediaControlsImpl::MaybeToggleControlsFromTap() { ...@@ -1191,12 +1195,9 @@ void MediaControlsImpl::MaybeToggleControlsFromTap() {
if (MediaElement().paused()) if (MediaElement().paused())
return; return;
// If the controls are visible we should try to hide them unless they should // If the controls are visible then hide them. If the controls are not visible
// be kept around for another reason. If the controls are not visible then // then show them and start the timer to automatically hide them.
// show them and start the timer to automatically hide them. If a pointer if (IsVisible()) {
// event showed the controls in this batch of events then we should not hiden
// the controls.
if (IsVisible() && !pointer_event_did_show_controls_) {
MakeTransparent(); MakeTransparent();
} else { } else {
MakeOpaque(); MakeOpaque();
...@@ -1204,8 +1205,6 @@ void MediaControlsImpl::MaybeToggleControlsFromTap() { ...@@ -1204,8 +1205,6 @@ void MediaControlsImpl::MaybeToggleControlsFromTap() {
keep_showing_until_timer_fires_ = true; keep_showing_until_timer_fires_ = true;
StartHideMediaControlsTimer(); StartHideMediaControlsTimer();
} }
pointer_event_did_show_controls_ = false;
} }
} }
...@@ -1228,61 +1227,17 @@ void MediaControlsImpl::DefaultEventHandler(Event* event) { ...@@ -1228,61 +1227,17 @@ void MediaControlsImpl::DefaultEventHandler(Event* event) {
// Touch events are treated differently to avoid fake mouse events to trigger // Touch events are treated differently to avoid fake mouse events to trigger
// random behavior. The expect behaviour for touch is that a tap will show the // random behavior. The expect behaviour for touch is that a tap will show the
// controls and they will hide when the timer to hide fires. // controls and they will hide when the timer to hide fires.
if (is_touch_event) { if (is_touch_event)
if (event->type() != EventTypeNames::gesturetap) HandleTouchEvent(event);
return;
if (!ContainsRelatedTarget(event)) { if (event->type() == EventTypeNames::mouseover && !is_touch_event)
if (!MediaElement().paused()) { is_touch_interaction_ = false;
if (!IsVisible()) {
MakeOpaque();
// When the panel switches from invisible to visible, we need to mark
// the event handled to avoid buttons below the tap to be activated.
event->SetDefaultHandled();
}
if (ShouldHideMediaControls(kIgnoreWaitForTimer)) {
keep_showing_until_timer_fires_ = true;
StartHideMediaControlsTimer();
}
}
}
return; if ((event->type() == EventTypeNames::pointerover ||
} event->type() == EventTypeNames::pointermove ||
event->type() == EventTypeNames::pointerout) &&
if (event->type() == EventTypeNames::pointerover) { !is_touch_interaction_)
if (!ContainsRelatedTarget(event)) { HandlePointerEvent(event);
is_mouse_over_controls_ = true;
if (!MediaElement().paused()) {
MakeOpaqueFromPointerEvent();
StartHideMediaControlsIfNecessary();
}
}
return;
}
if (event->type() == EventTypeNames::pointerout) {
if (!ContainsRelatedTarget(event)) {
is_mouse_over_controls_ = false;
StopHideMediaControlsTimer();
}
return;
}
// The pointer event has finished so we should clear the bit.
if (event->type() == EventTypeNames::mouseout) {
pointer_event_did_show_controls_ = false;
return;
}
if (event->type() == EventTypeNames::pointermove) {
// When we get a mouse move, show the media controls, and start a timer
// that will hide the media controls after a 3 seconds without a mouse move.
MakeOpaqueFromPointerEvent();
if (ShouldHideMediaControls(kIgnoreVideoHover))
StartHideMediaControlsTimer();
return;
}
// If the user is interacting with the controls via the keyboard, don't hide // If the user is interacting with the controls via the keyboard, don't hide
// the controls. This will fire when the user tabs between controls (focusin) // the controls. This will fire when the user tabs between controls (focusin)
...@@ -1316,6 +1271,51 @@ void MediaControlsImpl::DefaultEventHandler(Event* event) { ...@@ -1316,6 +1271,51 @@ void MediaControlsImpl::DefaultEventHandler(Event* event) {
} }
} }
void MediaControlsImpl::HandlePointerEvent(Event* event) {
if (event->type() == EventTypeNames::pointerover) {
if (!ContainsRelatedTarget(event)) {
is_mouse_over_controls_ = true;
if (!MediaElement().paused()) {
MakeOpaqueFromPointerEvent();
StartHideMediaControlsIfNecessary();
}
}
} else if (event->type() == EventTypeNames::pointerout) {
if (!ContainsRelatedTarget(event)) {
is_mouse_over_controls_ = false;
StopHideMediaControlsTimer();
}
} else if (event->type() == EventTypeNames::pointermove) {
// When we get a mouse move, show the media controls, and start a timer
// that will hide the media controls after a 3 seconds without a mouse move.
is_mouse_over_controls_ = true;
MakeOpaqueFromPointerEvent();
if (ShouldHideMediaControls(kIgnoreVideoHover))
StartHideMediaControlsTimer();
}
}
void MediaControlsImpl::HandleTouchEvent(Event* event) {
if (IsModern()) {
is_mouse_over_controls_ = false;
is_touch_interaction_ = true;
}
if (event->type() == EventTypeNames::gesturetap &&
!ContainsRelatedTarget(event) && !MediaElement().paused()) {
if (!IsVisible()) {
MakeOpaque();
// When the panel switches from invisible to visible, we need to mark
// the event handled to avoid buttons below the tap to be activated.
event->SetDefaultHandled();
}
if (ShouldHideMediaControls(kIgnoreWaitForTimer)) {
keep_showing_until_timer_fires_ = true;
StartHideMediaControlsTimer();
}
}
}
void MediaControlsImpl::HideMediaControlsTimerFired(TimerBase*) { void MediaControlsImpl::HideMediaControlsTimerFired(TimerBase*) {
unsigned behavior_flags = unsigned behavior_flags =
hide_timer_behavior_flags_ | kIgnoreFocus | kIgnoreVideoHover; hide_timer_behavior_flags_ | kIgnoreFocus | kIgnoreVideoHover;
......
third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.h
View file @ f05ffc1a
...@@ -274,6 +274,9 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement, ...@@ -274,6 +274,9 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
void DefaultEventHandler(Event*) override; void DefaultEventHandler(Event*) override;
bool ContainsRelatedTarget(Event*); bool ContainsRelatedTarget(Event*);
void HandlePointerEvent(Event*);
void HandleTouchEvent(Event*);
// Internal cast related methods. // Internal cast related methods.
void RemotePlaybackStateChanged(); void RemotePlaybackStateChanged();
void RefreshCastButtonVisibility(); void RefreshCastButtonVisibility();
...@@ -350,11 +353,15 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement, ...@@ -350,11 +353,15 @@ class MODULES_EXPORT MediaControlsImpl final : public HTMLDivElement,
bool keep_showing_until_timer_fires_ : 1; bool keep_showing_until_timer_fires_ : 1;
bool pointer_event_did_show_controls_ = false;
Member<MediaDownloadInProductHelpManager> download_iph_manager_; Member<MediaDownloadInProductHelpManager> download_iph_manager_;
bool is_acting_as_audio_controls_ = false; bool is_acting_as_audio_controls_ = false;
// Our best guess on whether the user is interacting with the controls via
// touch (as opposed to mouse). This is used to determine how to handle
// certain pointer events. In particular, when the user is interacting via
// touch events, we want to ignore pointerover/pointerout/pointermove events.
bool is_touch_interaction_ = false;
}; };
DEFINE_ELEMENT_TYPE_CASTS(MediaControlsImpl, IsMediaControls()); DEFINE_ELEMENT_TYPE_CASTS(MediaControlsImpl, IsMediaControls());
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment