Fix TalkBack seeking on the full player progress bar - #5547
joashrajin wants to merge 5 commits into
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 could move the full-player progress bar, but the position immediately snapped back and the seek was never committed (reported in #2264 as “progression leaps back to the zero timepoint”).
Root cause
TalkBack
ACTION_SET_PROGRESSactions and keyboard arrows update a nativeSeekBarthroughonProgressChanged(fromUser = true)without the touch-onlyonStartTrackingTouch/onStopTrackingTouchcallbacks. The player seek bar therefore did not enter its seeking state or send the requested position to playback.A deeper review also found that a simple delayed commit was not sufficient: an older async completion could clear a newer interaction, a delayed request could target the next episode, video reported completion before playback actually moved, detach could silently discard an accepted accessibility action, and seek analytics could race the playback-state mutation.
What changed
seekIfPlayingToTimeMsoverload’s synchronous guard, callback ABI, and sync-caller behavior.This benefits both consumers of
PlayerSeekBar: the Compose full-screen player and the full-screen video player.References #2264 — this resolves the progress bar item. The missing-labels item 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
Automated validation:
The new tests cover:
ACTION_SET_PROGRESSchanges coalescing to the latest targetThe original accessibility path was also verified on-device (Samsung, Android SDK 34) by dispatching
AccessibilityAction.ACTION_SET_PROGRESSat the live full-player seek bar: a seek from 695s to 995s held at the target and playback continued from there instead of snapping back.Optional manual smoke test:
Screenshots or Screencast
n/a — no visual changes.
Checklist
./gradlew spotlessApplyto automatically apply formatting/linting)modules/services/localization/src/main/res/values/strings.xml— n/a, no new stringsI have tested any UI changes...