Fix TalkBack seeking on the full player progress bar - #5547
Conversation
Accessibility seek actions change SeekBar progress without touch tracking callbacks, so the seek was never sent to the player and the next playback position update snapped the bar back (#2264).
|
Claude finished @joashrajin's task in 2m 22s —— View job Code Review: TalkBack seeking on the full player progress bar
Nice fix — the root-cause analysis in the description is spot on, and the approach (mark 1.
|
Generated by 🚫 Danger |
There was a problem hiding this comment.
Pull request overview
Fixes an accessibility bug where TalkBack (and other non-touch interactions like keyboard arrows) could move the full player seek bar but the UI would snap back and the seek wouldn’t actually be committed, by adding a debounced non-touch seek commit path inside PlayerSeekBar.
Changes:
- Track touch vs non-touch seek interactions and debounce non-touch progress changes into a single committed seek.
- Cancel pending non-touch commits when touch-drag starts and when the view detaches.
- Add a changelog entry for the TalkBack seek fix.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| modules/features/player/src/main/java/au/com/shiftyjelly/pocketcasts/player/view/PlayerSeekBar.kt | Adds non-touch seek handling (TalkBack/keyboard) with debounced commit and cancellation hooks. |
| CHANGELOG.md | Notes the user-facing TalkBack seek fix in 8.17. |
| override fun onStartTrackingTouch(seekBar: SeekBar) { | ||
| touchSeeking = true | ||
| removeCallbacks(commitNonTouchSeek) | ||
| changeListener?.onSeekPositionChangeStart() | ||
| seeking = true | ||
| } |
| override fun onDetachedFromWindow() { | ||
| removeCallbacks(commitNonTouchSeek) | ||
| super.onDetachedFromWindow() | ||
| } |
Address review feedback on the non-touch seek path: - Fire onSeekPositionChangeStart once per seek session so a touch drag interrupting an in-flight TalkBack/keyboard seek can't emit two starts for one stop, and start is dispatched before the changing callback. - Reset seeking/touchSeeking in onDetachedFromWindow so a reused view instance can't get stuck ignoring position updates.
|
Thanks for the review — addressed both correctness points in da7fccf: 1. 2. Double Left the state machine in the |
Description
TalkBack users can't seek with the full player progress bar: each accessibility swipe moves the bar, but the position immediately snaps back (reported in #2264 as "progression leaps back to the zero timepoint").
Root cause: TalkBack seek actions (
ACTION_SET_PROGRESS) and keyboard arrow keys change theSeekBarprogress viaonProgressChanged(fromUser = true)without theonStartTrackingTouch/onStopTrackingTouchcallbacks a finger drag produces.PlayerSeekBaronly committed a seek fromonStopTrackingTouch, so for accessibility seeks:seekingwas never set, so the next playback position update (setCurrentTime) immediately reverted the bar, andThis PR handles non-touch progress changes in
PlayerSeekBar:fromUserchange with no active touch tracking marksseeking = true(stopping the snap-back) and commits the seek through the existingonSeekPositionChangeStoppath after a short debounce, so consecutive TalkBack swipes accumulate into a single seek.This benefits both consumers of
PlayerSeekBar: the full screen player (via the Composenowplaying/PlayerSeekBarwrapper) and the full screen video player.References #2264 — this resolves the progress bar item. The missing-labels item from that issue was already fixed in #2742, and the bottom-navigation item needs separate on-device confirmation, so this PR intentionally does not auto-close the issue.
Testing Instructions
Verified on-device (Samsung, Android SDK 34) by dispatching
AccessibilityAction.ACTION_SET_PROGRESS— the exact action TalkBack sends when adjusting a slider — at the live full-player seek bar:[995, 995, 995, 996, 996, 997]) and playback continued from there (media session position advanced 693687ms → 1009800ms → 1042829ms) instead of snapping back. Before this change the samples revert toward the original position on the next playback tick.To reproduce manually with TalkBack:
Screenshots or Screencast
Checklist
./gradlew spotlessApplyto automatically apply formatting/linting)View; the player module has no Robolectric setup, so a JVM unit test isn't practical without adding that infrastructure. Behaviour was instead verified on-device via the accessibility action pathway (see Testing Instructions).modules/services/localization/src/main/res/values/strings.xml— n/a, no new stringstrackPlaybackSeeknow also fires for accessibility seeks)I have tested any UI changes...