search

ncapdevi / FragNav

An Android library for managing multiple stacks of fragments
1.5k stars 220 forks source link

fix 'java.lang.IllegalStateException', reason: 'Can't change tag of f… #118

Closed itzharDev closed 5 years ago

itzharDev commented 6 years ago

…ragment'

mateherber commented 6 years ago

This is a consequence of not calling executePendingTransactions. Reproduce:

so B->C->B then we may not find the already added root fragment for B because the FragmentTransaction.add.commit might still not be executed.

I think we'll have to come up with something to handle all these possibilities without forcing the executePendingTransactions.

I think the fix above only seemingly solves the problem and adds an untracked root fragment (and possibly leaks it). Considering the example above it would end up with something like B->C->B'(when B also gets added to fragment manager and is being tracked in the mFragmentStacks B' is not tracked by the library).

Rather i suggest calling FragNavController.executePendingTransactions after each pop/add/replace/switchtab.

ncapdevi commented 6 years ago

@itzharDev Hey, I don't want you to think that I'm ignoring you. Thanks for the PR. As @mateherber said above, this is an issue which we're aware of and looking to provide an overall fix/improvement to. You also had some unfortunate timing as the entire library was being ported over to Kotlin, so this PR became much more difficult. When we begin to fix the overall issue here, I will make sure to alert you and keep you involved. Thanks!

itzharDev commented 6 years ago

Tnx a lot. Mean while I fork your library and customize for my needs

On Wed, 7 Feb 2018, 21:31 Nic Capdevila, notifications@github.com wrote:

@itzharDev https://github.com/itzhardev Hey, I don't want you to think that I'm ignoring you. Thanks for the PR. As @mateherber https://github.com/mateherber said above, this is an issue which we're aware of and looking to provide an overall fix/improvement to. You also had some unfortunate timing as the entire library was being ported over to Kotlin, so this PR became much more difficult. When we begin to fix the overall issue here, I will make sure to alert you and keep you involved. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ncapdevi/FragNav/pull/118#issuecomment-363883705, or mute the thread https://github.com/notifications/unsubscribe-auth/AL53hvneoDG9LIhLPdD9jDfdv1nFFHokks5tSfoXgaJpZM4RgEK8 .

mateherber commented 6 years ago

Hi @itzharDev As an update for this issue. I've created a PR which supposedly fix your use case: https://github.com/ncapdevi/FragNav/pull/137

Can you consistently reproduce your issue? Can you test it with the PR branch to see if it solved? It would be a great help.

rhonyabdullah commented 6 years ago

Hello @mateherber Im currently using v 2.4.0 and getting lot of crash on fabric, i need a hotfixes rather than pulling 3.0.0 Rc tag, so do you think this is make sense to execute FragNavController.executePendingTransactions after each pop/add/replace/switchtab like you say above while i'm still using v.2.4.0 ? And after v3.0 released i'll remove my executedPendingTransaction

mateherber commented 6 years ago

@rhonyabdullah yes I'd call executePendingTransactions after each method of above. Actually until v2.2 or 2.3 it had been the case.

Hopefully v3 will be released soon fixing issues.

rhonyabdullah commented 6 years ago

One more @mateherber Should i have to executePendingTransaction after pushFragment ?

mateherber commented 6 years ago

@rhonyabdullah Yeah I'd say have executePendingTransactions after pushFragment popFragment, clearStack, replaceFragment, switchTab, clearFragmentManager.

mateherber commented 6 years ago

This can be closed as it should be fixed in 3.0.0

ncapdevi commented 5 years ago

Thanks for the help!