Fixed
Status Update
Comments
il...@google.com <il...@google.com> #2
Project: platform/frameworks/support
Branch: androidx-main
commit 57ca221882695bd6a52549f4d9ea3b812e6fe87c
Author: Simon Schiller <simonschiller@users.noreply.github.com>
Date: Mon Mar 22 16:09:30 2021
[GH] [FragmentStrictMode] Detect <fragment> tag usage
## Proposed Changes
- Detect `<fragment>` tag usage inside XML layouts
## Testing
Test: See `FragmentStrictModeTest#detectFragmentTagUsage`
## Issues Fixed
Fixes: 153738235
This is an imported pull request fromhttps://github.com/androidx/androidx/pull/141 .
Resolves #141
Github-Pr-Head-Sha: 4ea052596e4341b9f11bcf335e2bc38045a91f19
GitOrigin-RevId: 62e7487aa4874eef6bb556490e193717cf937251
Change-Id: Iae48578e85e4e4897f806d7ade2e2a660adf9479
M fragment/fragment/api/public_plus_experimental_current.txt
M fragment/fragment/api/restricted_current.txt
M fragment/fragment/src/androidTest/java/androidx/fragment/app/strictmode/FragmentStrictModeTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/FragmentLayoutInflaterFactory.java
M fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentStrictMode.java
A fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentTagUsageViolation.java
https://android-review.googlesource.com/1649748
Branch: androidx-main
commit 57ca221882695bd6a52549f4d9ea3b812e6fe87c
Author: Simon Schiller <simonschiller@users.noreply.github.com>
Date: Mon Mar 22 16:09:30 2021
[GH] [FragmentStrictMode] Detect <fragment> tag usage
## Proposed Changes
- Detect `<fragment>` tag usage inside XML layouts
## Testing
Test: See `FragmentStrictModeTest#detectFragmentTagUsage`
## Issues Fixed
Fixes: 153738235
This is an imported pull request from
Resolves #141
Github-Pr-Head-Sha: 4ea052596e4341b9f11bcf335e2bc38045a91f19
GitOrigin-RevId: 62e7487aa4874eef6bb556490e193717cf937251
Change-Id: Iae48578e85e4e4897f806d7ade2e2a660adf9479
M fragment/fragment/api/public_plus_experimental_current.txt
M fragment/fragment/api/restricted_current.txt
M fragment/fragment/src/androidTest/java/androidx/fragment/app/strictmode/FragmentStrictModeTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/FragmentLayoutInflaterFactory.java
M fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentStrictMode.java
A fragment/fragment/src/main/java/androidx/fragment/app/strictmode/FragmentTagUsageViolation.java
t....@gmail.com <t....@gmail.com> #3
I can guarantee that onCreateView() is indeed being called twice, and there is an onDestroyView() in between. So there may be an issue with your tests.
Were you not able to reproduce the problem with a BottomNavigationView and a call to startPostponedTransition()?
Were you not able to reproduce the problem with a BottomNavigationView and a call to startPostponedTransition()?
il...@google.com <il...@google.com> #4
No, I was not able to reproduce it.
t....@gmail.com <t....@gmail.com> #5
OK, I've created a sample project, reproducing the issue:
https://github.com/timusus/android-fragment-navigation-bug
I've noticed that this issue only occurs for the Fragment set as 'startDestination' in the navigation graph xml.
So to explain what the key players are:
1) Have a BottomNavigationView setup with NavController
2) Have a Fragment marked as the 'startDestination' in the navigation graph
3) Have that same Fragment perform postponeEnterTransition()
Also, to clarify. onCreateView() is not being called twice on the same Fragment instance, so I apologise - your tests may be fine. It appears there are two instances of the fragment in question.
I've noticed that this issue only occurs for the Fragment set as 'startDestination' in the navigation graph xml.
So to explain what the key players are:
1) Have a BottomNavigationView setup with NavController
2) Have a Fragment marked as the 'startDestination' in the navigation graph
3) Have that same Fragment perform postponeEnterTransition()
Also, to clarify. onCreateView() is not being called twice on the same Fragment instance, so I apologise - your tests may be fine. It appears there are two instances of the fragment in question.
il...@google.com <il...@google.com> #6
Thanks, your sample app was very helpful in reproducing what you saw. I was able to reproduce it by going to the Home fragment, then tapping on the Dashboard button to go back to the Dashboard fragment.
If you look at the NavigationUI code (https://android.googlesource.com/platform/frameworks/support/+/androidx-master-dev/navigation/navigation-ui/src/main/java/androidx/navigation/ui/NavigationUI.java#57 ), it does a few things when you click a MenuItem:
1) It adds the correct cross fade animations
2) It pops up to the start destination of your graph
3) It uses setLaunchSingleTop(true)
And then it navigates to the selected destination (i.e., the screen associated with the MenuItem you clicked) with the goal being that the user is at the exact correct location in the app.
Step 2) ensures that we don't keep an every growing stack. In your case, this pops the any instance of the Home or Notification fragment from the back stack. Step 3) ensures that there's only one instance of your start destination Dashboard fragment is on the back stack.
Why you're experiencing the behavior is due to the intersection of two things:
a) Fragments don't have a single top like functionality (there's no onNewIntent() like for an Activity), so FragmentNavigator, when faced with a single top operation like this one, pops the old instance and creates a new instance of the Fragment, passing through the new arguments, animations, etc.
b) As you've noticed, postponed Fragments are a special case that requires some extra work by FragmentManager. What happens is that the new Dashboard instance has its view created and becomes postponed. Since FragmentManager can't know when that will stop being the case, it basically reverses the replace() operation it was just about to do, causing the old Fragment to be eligible for having its view created. Therefore the second onCreateView() you see is the old instance having its view temporarily created to "fill the gap" until the new instance is available. After the new instance calls startPostponedEnterTransition(), the replace() is commited again and the old instance has its view removed.
From the Navigation perspective, it is indeed doing the right thing.
From the Fragment perspective, this case of pop+replace with a postponed Fragment is perhaps 'working as designed', but we're aware that the way that FragmentManager manages postponed Fragments is something that needs some improvement (to put it mildly). We have some ideas in this area, so I'll leave this issue open as a case we should specifically test to ensure it is fixed with any internal restructuring we do.
If you look at the NavigationUI code (
1) It adds the correct cross fade animations
2) It pops up to the start destination of your graph
3) It uses setLaunchSingleTop(true)
And then it navigates to the selected destination (i.e., the screen associated with the MenuItem you clicked) with the goal being that the user is at the exact correct location in the app.
Step 2) ensures that we don't keep an every growing stack. In your case, this pops the any instance of the Home or Notification fragment from the back stack. Step 3) ensures that there's only one instance of your start destination Dashboard fragment is on the back stack.
Why you're experiencing the behavior is due to the intersection of two things:
a) Fragments don't have a single top like functionality (there's no onNewIntent() like for an Activity), so FragmentNavigator, when faced with a single top operation like this one, pops the old instance and creates a new instance of the Fragment, passing through the new arguments, animations, etc.
b) As you've noticed, postponed Fragments are a special case that requires some extra work by FragmentManager. What happens is that the new Dashboard instance has its view created and becomes postponed. Since FragmentManager can't know when that will stop being the case, it basically reverses the replace() operation it was just about to do, causing the old Fragment to be eligible for having its view created. Therefore the second onCreateView() you see is the old instance having its view temporarily created to "fill the gap" until the new instance is available. After the new instance calls startPostponedEnterTransition(), the replace() is commited again and the old instance has its view removed.
From the Navigation perspective, it is indeed doing the right thing.
From the Fragment perspective, this case of pop+replace with a postponed Fragment is perhaps 'working as designed', but we're aware that the way that FragmentManager manages postponed Fragments is something that needs some improvement (to put it mildly). We have some ideas in this area, so I'll leave this issue open as a case we should specifically test to ensure it is fixed with any internal restructuring we do.
t....@gmail.com <t....@gmail.com> #7
Thanks for the detailed response Ian, I appreciate it.
I wonder if this problem would go away if I used the google sample code for bottom navigation, where it maintains multiple backstacks (maybe that eliminates a 'pop') . I might give that a try.
Keep up the great work.
I wonder if this problem would go away if I used the google sample code for bottom navigation, where it maintains multiple backstacks (maybe that eliminates a 'pop') . I might give that a try.
Keep up the great work.
si...@gmail.com <si...@gmail.com> #8
I'm facing the same problem.
When navigating to a fragment with postponeEnterTransition(), the start destination of the graph blinks before completing navigation to desired fragment.
Are you going to fix this?
When navigating to a fragment with postponeEnterTransition(), the start destination of the graph blinks before completing navigation to desired fragment.
Are you going to fix this?
il...@google.com <il...@google.com> #9
This has been fixed internally and will be avilable in Fragment 1.3.0-alpha08.
Note: this fix relies on using the
t....@gmail.com <t....@gmail.com> #10
Excellent. Thanks so much!
sm...@gmail.com <sm...@gmail.com> #11
I'm experiencing a similiar issue with version 1.3.1. I wrote some instrumented tests using androidx.fragment:fragment-testing:1.2.5 which are supposed to simulate Fragment being minimized (home button) and brought back again. I perform that by calling FragmentScenario.moveToState() to go RESUMED -> CREATED -> RESUMED. With 1.2.5 everything was fine, but after upgrade to 1.3.1 these tests started failing due to onCreateView() being called twice. I've described my problem with greater detail on SO https://stackoverflow.com/questions/66629817/testing-androidx-fragment-lifecycle-by-stopping-and-resuming-with-fragmentscenar .
This issue is already fixed in 1.3.1, so this probably is a problem in my configuration. Or maybe it's a regression ? It'd be helpful if someone experienced in this topic would take a look. I could provide sample project reproducing the issue on github if needed.
This issue is already fixed in 1.3.1, so this probably is a problem in my configuration. Or maybe it's a regression ? It'd be helpful if someone experienced in this topic would take a look. I could provide sample project reproducing the issue on github if needed.
il...@google.com <il...@google.com> #12
Re #11 - going from RESUMED
-> CREATED
will destroy your view (the CREATED
state is the state immediately after onCreate()
- i.e., before onCreateView()
), moving back to up RESUMED
will recreate the View. It sounds like Fragment 1.3.1 is working as intended.
Description
Version used:
androidx.navigation:navigation-fragment-ktx:2.2.0-rc01
androidx.navigation:navigation-ui-ktx:2.2.0-rc01
Devices/Android versions reproduced on:
Pixel 3
Android 10
When switching tabs (using BottomNavigation / setupWithNavController()), when the destination Fragment contains a call to postponeEnterTransition() in onCreate() or onCreateView(), the onCreateView() lifecycle method gets called twice. This behavior is visible to the user, via a sort of 'flicker' (depending on the layout), as the fragment's view is recreated.
I suspect this may be due to some code in
`FragmentManager.addAddedFragments(@NonNull ArraySet<Fragment> added)`
It appears the destination Fragment (which invoked postponeEnterTransition()) has moveToState() called on it twice (vie addAddedFragments()). My guess, is that the destination fragment hasn't completed its first moveToState() before the second moveToState() is called, so the secondMoveToState() is attempting to move from a stale state.
I'm not able to provide a sample project for you at this stage.