Fix arrivals onBackPressed() on Android 13+#1563
Conversation
|
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 (1)
📝 WalkthroughWalkthrough
ChangesBack Press Handling Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
aaronbrethorst
left a comment
There was a problem hiding this comment.
Rob, this is a really nicely scoped fix. The root-cause writeup in the PR description is genuinely illuminating — pinning the silent bypass on WindowOnBackInvokedDispatcher → OnBackPressedDispatcher → ComponentActivity.super.onBackPressed() skipping subclass overrides is exactly the kind of explanation that makes a one-file diff easy to land with confidence. The Before/After videos are also a great touch.
I cross-checked the migration against the official Android docs (Provide custom back navigation; Add support for the predictive back gesture) and the implementation matches the documented pattern step-for-step.
Critical Issues (0 found)
None.
Important Issues (0 found)
None.
Suggestions (1 found)
- The fallback inside
handleOnBackPressed()uses the documentedsetEnabled(false); getOnBackPressedDispatcher().onBackPressed();pattern, which permanently disables the callback after first fallback [HomeActivity.java:428-429]. In practice this is fine forHomeActivitybecause the only time the fallback runs is when no panel is showing and the activity is about to finish, so the disabled state never matters. Worth being aware of as a subtle behavior of this idiom, but no change needed here.
Follow-up (out of scope, tracked separately)
The same Android 13+ bug almost certainly affects two other activities still using the deprecated onBackPressed() override:
TripPlanActivity.java:261-267— has panel-collapse logic structurally identical to the bug fixed here.InfrastructureIssueActivity.java:722-726— its post-back spinner reset is being silently skipped on API 33+.
I've opened #1566 to track these migrations so they don't get lost.
thanks for knocking this out!
…sedCallback On Android 13+ (API 33+), back presses route through OnBackPressedDispatcher, whose fallback calls ComponentActivity.super.onBackPressed() directly, silently bypassing subclass overrides of onBackPressed(). Migrate the two remaining affected activities to register an OnBackPressedCallback in onCreate(), mirroring the HomeActivity fix from #1563: - TripPlanActivity: panel-collapse logic now runs on back press again. Also route the toolbar home button through the dispatcher so software and hardware back stay consistent. - InfrastructureIssueActivity: the services spinner reset now runs after each back press again. The callback temporarily disables itself to let the fragment back stack pop (or the activity finish), then re-enables for subsequent presses.
Problem
Pressing back while viewing a stop's arrivals exited the app instead of dismissing the panel and returning to the map.
Root cause
HomeActivity overrode
onBackPressed()to handle panel dismissal, but this override was silently bypassed on Android 13+ (API 33+). Modern Android routes back presses throughWindowOnBackInvokedDispatcher→AndroidXOnBackPressedDispatcher, whose fallback callsComponentActivity.super.onBackPressed()directly — skipping subclass overrides entirely. The app has usedandroidx.activity:activity-ktx:1.9.3for about 2 years, which made this the behavior on all currently-supported devices.Fix
Migrate the back press logic from
onBackPressed()into anOnBackPressedCallbackregistered withgetOnBackPressedDispatcher()inonCreate(). This is the API that both the AndroidX dispatcher and the OS-levelOnBackInvokedDispatcheractually invoke, and it works correctly on all API levels (not just 13+).Behavior
Before:
before.mp4
After:
after.mp4
Apply the
AndroidStyle.xmlstyle template to your code in Android Studio.Run the unit tests with
gradlew connectedObaGoogleDebugAndroidTestto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them for the initial submission of the pull request. When addressing comments on a pull request, please push a new commit per comment when possible (reviewers will squash and merge using GitHub merge tool)
Summary by CodeRabbit