Skip to content

Fix: state loss in SearchActivity and MediaDetailPagerFragment during screen rotation#6605

Open
Kota-Jagadeesh wants to merge 10 commits intocommons-app:mainfrom
Kota-Jagadeesh:fix/search-rotation-state-loss
Open

Fix: state loss in SearchActivity and MediaDetailPagerFragment during screen rotation#6605
Kota-Jagadeesh wants to merge 10 commits intocommons-app:mainfrom
Kota-Jagadeesh:fix/search-rotation-state-loss

Conversation

@Kota-Jagadeesh
Copy link
Copy Markdown
Collaborator

@Kota-Jagadeesh Kota-Jagadeesh commented Jan 5, 2026

Description (required)

What changes did you make and why?

  1. Implemented State Restoration in SearchActivity: added tha onSaveInstanceState to track the current media position and updated onCreate to restore the media detail view if a position was previously saved. This replaces the previous temporary fix in onResume which forced a back-press on rotation.
  2. Synchronized MediaDetailPagerFragment: Updated the fragment to save and restore its internal ViewPager current item index.
  3. Improved UX & Navigation: By using binding.root.post for restoration, I ensured the UI is fully laid out before re-opening details, preventing crashes and ensuring the user stays on the same image they were viewing before rotating the device.
  4. Cleaned up Fragment Stack: Removed the onBackPressed() call from onResume() which was an anti-pattern causing the fragment back-stack to increase incorrectly.

Tests performed (required)

Tested ProdDebug on Redmi Note 13 Pro with API level 35.

  • Verified that rotating from Portrait to Landscape while viewing an image detail does not return the user to the search results list.
  • Verified that the Back button correctly returns the user to the search results after a rotation occurs.
  • Verified that isFeaturedImage and editable flags are preserved after rotation, keeping menu items consistent.

Part of issue #6508

@github-actions
Copy link
Copy Markdown

✅ Generated APK variants!

@Kota-Jagadeesh
Copy link
Copy Markdown
Collaborator Author

based on #6347 (comment), #6624 (comment)

@RitikaPahwa4444 i've gone througgh the history in #6347 and the implementation in SearchActivity.kt, and yep i now understandd why the "simple" null-check approach is a regression risk.

since the SearchActivity currently relies on an onBackPressed() in onResume to handle rotation, rather than properly saving and restoring the position state, any attempt to nullify the binding in the onDestroy triggers race conditions with the Fragment backstack and the UI visibility logic. i now understand that "Binding will always be null" refers to these asynchronous edge cases where the UI attempts to update after the Activity has already begun its destruction or "forced back" flow.

and i'm interested in rohit's suggestion to start small with a systematic refactor. i'd love to work on this and would it be a good idea to start by moving the Search state or the Media Detail navigation logic into a ViewModel so we can eventually remove these manual lifecycle overrides safely?

@RitikaPahwa4444
Copy link
Copy Markdown
Collaborator

@Kota-Jagadeesh there are several PRs covering memory leaks and seems unrelated to the linked issue. I mentioned about it only because you added that null check. Please avoid that comment as we discussed earlier in your closed PR. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants