Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash in onSaveInstanceState in Reader #12018

Merged

Conversation

malinajirka
Copy link
Contributor

@malinajirka malinajirka commented May 25, 2020

Fixes #11992

As explained here this issue was introduced with the changes to the BottomNavigation.

However, the root cause of this issue was introduced by Google in AppCompat 1.1.0. onSaveInstanceState is invoked even for fragments on back stack which views were never displayed/created. More info here.

Downside is that we'll lose scroll position on double rotation, but I don't think it's a big deal. Especially since this will happen only when the user isn't on the Reader tab.

To test:

  1. Open the App
  2. Click on Reader
  3. Click on "My Site"
  4. Rotate your device twice -> make sure the app doesn't crash

  1. Enable "Don't keep activities" under Device Settings -> Systems -> Advanced -> Developer options -> Apps section
  2. Open WordPress Android app and toggle from Reader tab to My site tab
  3. Click Blog Posts on My site tab and return from it
  4. Click Blog Posts again or Site Pages on My site tab -> make sure the app doesn't crash

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

This crash was introduced in 14.9-rc-1, so this PR is targeting a frozen branch (cc @oguzkocer).

@peril-wordpress-mobile
Copy link

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

You can test the changes on this Pull Request by downloading the APK here.

Copy link
Contributor

@ashiagr ashiagr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, 👍.

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