fix(tv): stop a For You detail return leaving the screen without focus - #183
Merged
RXWatcher merged 3 commits intoAug 6, 2026
Merged
Conversation
Returning from a detail screen could leave For You with no focus owner at all,
so the D-pad stopped doing anything until the user navigated away.
When the relocation loop cannot reach the launch card it reports Exhausted. The
entire recovery for that was a single unchecked call:
if (result == ForYouReturnFocusResult.Exhausted) {
requestFocusSafely { forYouFocusRequester.requestFocus() }
}
requestFocusSafely returns Handled / Rejected / Disposed, and the result was
discarded. If that last request did not take - the pill not attached on the
frame it was asked - nothing retried, nothing else was tried, and nothing was
logged, because requestFocusSafely deliberately converts the "FocusRequester is
not initialized" throw into a value rather than letting it surface. The failure
was silent as well as unhandled, which is why it never appeared in a log.
The no-target branch a few lines above already retries across frames; this path
did not. It now walks the filter pills, which are composed for the life of the
screen, so one of them can take focus even while the feed is still settling, and
reports when none of them could.
Reported as intermittent (roughly one in three) on Shield after
Watchlist -> item -> Play -> Back. Consistent with a slower device spending more
frames before the row attaches, and with playback return being the most
disruptive recomposition on that screen.
Not reproduced directly: four cycles on a Google TV Streamer stayed clean, and
this was found by reading the path rather than by catching it live.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe TV recommendations screen now performs bounded, frame-spaced focus recovery across the For You, Watchlist, and Favorites controls. The new focus bridge handles rejected, unavailable, and throwing focus requests. Tests cover success, retries, and exhaustion. ChangesRecommendations focus recovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TvRecommendationsScreen
participant TvRecommendationsFocusBridge
participant FocusRequester
TvRecommendationsScreen->>TvRecommendationsFocusBridge: request fallback focus
TvRecommendationsFocusBridge->>TvRecommendationsFocusBridge: wait for the next frame
TvRecommendationsFocusBridge->>FocusRequester: safely request focus for each candidate
FocusRequester-->>TvRecommendationsFocusBridge: accept or reject request
TvRecommendationsFocusBridge-->>TvRecommendationsScreen: return recovery result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
@coderabbitai review |
|
Contributor
Author
|
@coderabbitai full review |
|
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Returning from a detail screen could leave For You with no focus owner at all, so the D-pad stopped responding until you navigated away.
The dead end
When the relocation loop cannot reach the launch card it reports
Exhausted. The entire recovery for that case was one unchecked call:requestFocusSafelyreturnsHandled/Rejected/Disposed, and the result was discarded. If that request did not take — the pill not attached on the frame we happened to ask — nothing retried, no other candidate was tried, and nothing was logged.The silence is the reason this was hard to find.
requestFocusSafelydeliberately converts theFocusRequester is not initializedthrow into a value rather than letting it surface, so a failed last-resort claim produced no warning in logcat at all.The no-target branch a few lines above already does the right thing — it retries
forYouFocusRequesteracross frames and only gives up afterTvFrameRelocationMaxAttempts. This path just never got the same treatment.The fix
Retry across frames, and fall through the filter pills (
For You/Watchlist/Favorites). Those are composed for the life of the screen, so one can take focus even while the feed is still settling. A candidate that throws is treated as not yet attached rather than fatal, since the node can attach on a later frame. If nothing takes focus, say so.Why it matches the report
Reported as intermittent, roughly one in three, on Shield after
Watchlist → item → Play → Back:Verification
ForYouFallbackFocusTestcovers first-candidate claim, fall-through, later-frame attachment, a throwing candidate, total failure, and no candidates:androidTvApp:testDebugUnitTestgreen (forced rerun):androidTvApp:compileDebugKotlinAndroidcleanNot reproduced directly. Four cycles on a Google TV Streamer stayed clean — this was found by reading the path, not by catching it live. Worth a Shield test before merging.
Summary by CodeRabbit