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

Unexpected behavior on quick forward-back navigation in Fragments #35

Closed
midery opened this issue Jan 15, 2021 · 7 comments
Closed

Unexpected behavior on quick forward-back navigation in Fragments #35

midery opened this issue Jan 15, 2021 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@midery
Copy link

midery commented Jan 15, 2021

Hello! Thanks for the great library and simple approach for viewBinding initialization.
I've discovered really annoying bug: if we will use fragment view bindings, try to replace current fragment with new fragment, and then quickly press back - our current fragment will not show all initialization data, which we will provide in it in onCreateView call.

This happens because we're calling handler.post { viewBinding = null } in ClearOnDestroyLifecycleObserver.onDestroy. method. Thus, if we happen to open next screen, and exit from it really fast, the viewBinding, that is accessed in onCreateView method will be this undestroyed binding.

Method call list will be such:

  1. Fragment B: onCreateView
  2. Fragment B: BackPressed
  3. Fragment A: onDestroyView
  4. Fragment A: onCreateView <-- invalid viewBinding here
  5. Fragment A: onStart
  6. Fragment A: onResume
  7. Fragment A: Post OnDestroy (ViewBinding cleared)
@gmk57
Copy link

gmk57 commented Jan 17, 2021

I couldn't press Back quickly enough, but it is easily reproducible programmatically, by calling FragmentManager.popBackStack() immediately after commit(), or from second fragment's onCreate().

@RuslanDemyanov
Copy link

Also reproduced this error when switching fragments programmatically, is there any way to destroy binding without using a handler?

@kirich1409
Copy link
Collaborator

kirich1409 commented Jan 25, 2021

Also reproduced this error when switching fragments programmatically, is there any way to destroy binding without using a handler?

Yes, you can keep reference to the delegate instance and call clear()

@kirich1409
Copy link
Collaborator

Need to check the bug with AndroidX Fragment 1.3.0. @midery , please do this and write about results

@midery
Copy link
Author

midery commented Mar 4, 2021

Need to check the bug with AndroidX Fragment 1.3.0. @midery , please do this and write about results

Checked this issue in AndroidX Fragment 1.3.0. Surprisingly, it isn't reproducing anymore.

It happens because the Fragment.onDestroyView call is now "postponed" and cancellable in new FragmentManager implementation.

Fragment.destroyFragmentView now is bound to a FragmentTransition, which is used to show fragment's removal. If the transition is not yet completed - Fragment's view won't be destroyed.

Thus, if we will try to press back after the new fragment is added and current fragment is being replaced, it will happen before transition's completion, and the viewBindings would not be cleared.

So, the logs now are showing something like this:

  1. Fragment B: onCreateView
  2. Fragment B: BackPressed
  3. Fragment A: onDestroyView
  4. Fragment A: onCreateView <-- valid viewBinding here
  5. Fragment A: onStart
  6. Fragment A: onResume //on destroy is never called

TL;DR: Update the AndroidX Fragment artifact to version 1.3.0+ and this issue will be fixed.

@gmk57
Copy link

gmk57 commented Mar 4, 2021

viewBindings would not be cleared

Sorry, but this is exactly the issue. I see no difference between Fragment 1.2.5 & 1.3.0 in this respect.
Here is a sample project to reproduce. After forward-back navigation the text on screen is wrong, because FirstFragment's binding still points to old view during onViewCreated.

@kirich1409
Copy link
Collaborator

viewBindings would not be cleared

Sorry, but this is exactly the issue. I see no difference between Fragment 1.2.5 & 1.3.0 in this respect.
Here is a sample project to reproduce. After forward-back navigation the text on screen is wrong, because FirstFragment's binding still points to old view during onViewCreated.

Please open separate issue with your bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants