Skip to content

Commit

Permalink
Fix PlayerView touch handling
Browse files Browse the repository at this point in the history
Overriding onTouchEvent was causing multiple issues, and
appears to be unnecessary. Removing the override fixes:

1. StyledPlayerView accessibility issue where "hide player
   controls" actually toggled play/pause.
2. Delivery of events to a registered OnClickListener when
   useController is false.
3. Delivery of events to a registered OnLongClickListener
   in all configurations.
4. Incorrectly treating a sequence of touch events that
   exit the bounds of the view before ACTION_UP as a click,
   both for delivery to OnClickListener and for toggling
   the controls.

Note: After this change, control visibility will not be
toggled if the application developer explicitly sets the
view to be non-clickable. I think that's probably working
as intended though. It seems correct that a non-clickable
view would not respond to clicks.

Issue: #8627
Issue: #9605
Issue: #9861
PiperOrigin-RevId: 433016626
  • Loading branch information
ojw28 authored and icbaker committed Mar 9, 2022
1 parent b2a5298 commit 2028215
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 53 deletions.
11 changes: 11 additions & 0 deletions RELEASENOTES.md
Expand Up @@ -35,6 +35,17 @@
views to be used with other `Player` implementations, and removes the
dependency from the UI module to the ExoPlayer module. This is a
breaking change.
* UI:
* Fix delivery of events to `OnClickListener`s set on `StyledPlayerView`
and `PlayerView`, in the case that `useController=false`
([#9605](https://github.com/google/ExoPlayer/issues/9605)). Also fix
delivery of events to `OnLongClickListener` for all view configurations.
* Fix incorrectly treating a sequence of touch events that exit the bounds
of `StyledPlayerView` and `PlayerView` before `ACTION_UP` as a click
([#9861](https://github.com/google/ExoPlayer/issues/9861)).
* Fix `StyledPlayerView` accessibility issue where it was not possible to
tapping would toggle playback rather than hiding the controls
([#8627](https://github.com/google/ExoPlayer/issues/8627)).
* RTSP:
* Add RTP reader for HEVC
([#36](https://github.com/androidx/media/pull/36)).
Expand Down
Expand Up @@ -397,6 +397,7 @@ public PlayerView(Context context, @Nullable AttributeSet attrs, int defStyleAtt

LayoutInflater.from(context).inflate(playerLayoutId, this);
setDescendantFocusability(FOCUS_AFTER_DESCENDANTS);
setClickable(true);

// Content frame.
contentFrame = findViewById(R.id.exo_content_frame);
Expand Down Expand Up @@ -1072,31 +1073,10 @@ public SubtitleView getSubtitleView() {
return subtitleView;
}

@Override
public boolean onTouchEvent(MotionEvent event) {
if (!useController() || player == null) {
return false;
}
switch (event.getAction()) {
case MotionEvent.ACTION_DOWN:
isTouching = true;
return true;
case MotionEvent.ACTION_UP:
if (isTouching) {
isTouching = false;
performClick();
return true;
}
return false;
default:
return false;
}
}

@Override
public boolean performClick() {
super.performClick();
return toggleControllerVisibility();
toggleControllerVisibility();
return super.performClick();
}

@Override
Expand Down Expand Up @@ -1192,16 +1172,15 @@ private boolean useArtwork() {
return false;
}

private boolean toggleControllerVisibility() {
private void toggleControllerVisibility() {
if (!useController() || player == null) {
return false;
return;
}
if (!controller.isVisible()) {
maybeShowController(true);
} else if (controllerHideOnTouch) {
controller.hide();
}
return true;
}

/** Shows the playback controls, but only if forced or shown indefinitely. */
Expand Down
Expand Up @@ -311,6 +311,7 @@ public StyledPlayerView(Context context, @Nullable AttributeSet attrs, int defSt

LayoutInflater.from(context).inflate(playerLayoutId, this);
setDescendantFocusability(FOCUS_AFTER_DESCENDANTS);
setClickable(true);

// Content frame.
contentFrame = findViewById(R.id.exo_content_frame);
Expand Down Expand Up @@ -1018,30 +1019,10 @@ public SubtitleView getSubtitleView() {
return subtitleView;
}

@Override
public boolean onTouchEvent(MotionEvent event) {
if (!useController() || player == null) {
return false;
}
switch (event.getAction()) {
case MotionEvent.ACTION_DOWN:
isTouching = true;
return true;
case MotionEvent.ACTION_UP:
if (isTouching) {
isTouching = false;
return performClick();
}
return false;
default:
return false;
}
}

@Override
public boolean performClick() {
super.performClick();
return toggleControllerVisibility();
toggleControllerVisibility();
return super.performClick();
}

@Override
Expand Down Expand Up @@ -1137,18 +1118,15 @@ private boolean useArtwork() {
return false;
}

private boolean toggleControllerVisibility() {
private void toggleControllerVisibility() {
if (!useController() || player == null) {
return false;
return;
}
if (!controller.isFullyVisible()) {
maybeShowController(true);
return true;
} else if (controllerHideOnTouch) {
controller.hide();
return true;
}
return false;
}

/** Shows the playback controls, but only if forced or shown indefinitely. */
Expand Down

0 comments on commit 2028215

Please sign in to comment.