Skip to content

fix: check is disposed fragment is in the FragmentManager#8166

Closed
vakrilov wants to merge 7 commits intomasterfrom
dispose-fragment-crash
Closed

fix: check is disposed fragment is in the FragmentManager#8166
vakrilov wants to merge 7 commits intomasterfrom
dispose-fragment-crash

Conversation

@vakrilov
Copy link
Contributor

@vakrilov vakrilov commented Dec 4, 2019

PR Checklist

What is the current behavior?

Fragment is always removed from the fragment manager in the Frame.disposeCurrentFragment() method. Different versions of androidX behave differently if you try to remove a fragment from a fragment manager.

What is the new behavior?

Added a check if the current fragment is actually contained in the fragment manager returned by the _getFragmentManager()

Fixes #8037

@cla-bot cla-bot bot added the cla: yes label Dec 4, 2019
@vakrilov vakrilov force-pushed the dispose-fragment-crash branch from 8959c8c to a75397a Compare December 4, 2019 12:52
@vakrilov vakrilov marked this pull request as ready for review December 5, 2019 07:37
@vmutafov vmutafov self-assigned this Dec 5, 2019
@vmutafov vmutafov self-requested a review December 5, 2019 08:36
@farfromrefug
Copy link
Collaborator

@vakrilov 4 things we can do and still be safe with different androidx versions :

  • add a comment about something not being right there(especially the this._getFragmentManager)
  • compare fragment.getFragmentManager() and this._getFragmentManager() they should be the same (thus the first point)
  • check the fragmentManager fragments like you do. Or maybe use findFragmentById?
  • change the order in onUnloaded as mentioned here

@vakrilov vakrilov force-pushed the dispose-fragment-crash branch 2 times, most recently from 3f812e1 to 37866cf Compare December 6, 2019 14:43
@vakrilov vakrilov force-pushed the dispose-fragment-crash branch from 37866cf to b20ec86 Compare December 6, 2019 14:44
@vakrilov
Copy link
Contributor Author

test

@vmutafov vmutafov removed their assignment Dec 16, 2019
@vakrilov vakrilov changed the base branch from master to release December 16, 2019 13:12
@vakrilov vakrilov changed the base branch from release to master December 16, 2019 13:14
@vakrilov
Copy link
Contributor Author

closing in favor of #8201

@vakrilov vakrilov closed this Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Android] showModal : Cannot remove Fragment attached to a different FragmentManager

4 participants