-
Notifications
You must be signed in to change notification settings - Fork 56
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 on double rotation with modal #1288
Conversation
86e1b7f
to
3d7c59f
Compare
👋 @mkevins, gave this solution a go and seems to work OK. Thing is, I am not sure what's the full set of assumptions Obviously, what we want to avoid here is the case where the user rotates the device and data is lost. Trivial testing (tried undo redo history too) shows that data is preserved, but it'd be better if we get to the bottom of this. Can you drill in when you get the chance? |
@hypest 👋 I somehow missed this ping. Cameron's comment here sums up the lifecycle methods nicely. In short, the |
@mkevins do we still need this PR? |
Fixes wordpress-mobile/WordPress-Android#10227
WIP - Do Not Merge Yet
Related PRs
WordPress-Android: wordpress-mobile/WordPress-Android#10383
Description
Since
GutenbergContainerFragment
is retained, it lives outside of the activity lifecycle. This results in a change in the way the fragment lifecycle methods are called with respect to the activity lifecylcle. Specifically,onDestroy
is not called when an activity is re-created.The
GutenbergContainerFragment
instantiates and keeps a reference to aWPAndroidGlueCode
, which, in turn, instantiates and keeps a reference to aReactInstanceManager
, all of which live outside the activity lifecycle. When the fragment is destroyed, theReactInstanceManager
'sonHostDestroy
method is called with the current activity, to perform cleanup. However, this does not occur when the fragment is retained, as it is only called through from the fragment'sonDestroy
method.This PR overrides the fragment's
onDetach
method, calling through to a newonDetach
method on the WPAndroidGlueCode. That method performs the cleanup with the activity being destroyed (e.g. during an orientation change).This approach works, but I think the current state could be cleaned up, thus, this PR is only a draft, with the intent to gather more context. It may be that we can simplify this to changing the fragment to directly call
WPAndroidGlueCode
'sonDestroy
method, rather than creating the method of the same name there.To test:
Follow steps here: wordpress-mobile/WordPress-Android#10227 (comment) and here: wordpress-mobile/WordPress-Android#10227 (comment)
Update release notes:
RELEASE-NOTES.txt
.