New Fragment StateManager does not call on onCreateView when a Fragment that had it transition postponed is popped [166658128]

Fixed

Bug

Status Update

No update yet.

Description

tj...@gmail.com

created issue #1

Aug 28, 2020 03:29PM

Component used: Fragment Version used: 1.3.0-alpha08 Devices/Android versions reproduced on: Emulator / Pixel 3

If this is a bug in the library, we would appreciate if you could attach:

Sample project to trigger the issue.
A screenrecord or screenshots showing the issue (if UI related).

I have a sequence of 3 Fragments that use shared element transitions, and use postponeEnterTransition() on all of them.

Everything is great when I go from Fragments 1 -> 2 -> 3. However, when I pop from 3 -> 2, the onCreateView and onViewCreated methods are skipped. onStart and onResume are called however. What's worse, after popping, I see the old view instance for Fragment 2 that should have been destroyed when I move to Fragment 3. OnDestroyView is also never called for Fragment 2 at all.

Repository with reproducing code: https://github.com/tunjid/Android-Extensions/tree/feature/fragment-1.3.0

Fragment 1 -> DoggoListFragment Fragment 2 -> DoggoPagerFragment Fragment 3 -> AdoptDoggoFragment

Comments

il...@google.com <il...@google.com> Sep 16, 2020 06:31PM

Assigned to jb...@google.com.

jb...@google.com <jb...@google.com> #2Sep 16, 2020 11:39PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

tj...@gmail.com <tj...@gmail.com> #3Sep 17, 2020 12:50AM

The issue still persists using the snapshot with the build id provided
unfortunately.

>

jb...@google.com <jb...@google.com> #4Sep 17, 2020 06:38AM

Status: In Progress (Accepted) (reopened)

You're right, I was testing the wrong thing, this is currently still broken with that build. Thank you for checking that.

ap...@google.com <ap...@google.com> #5Sep 18, 2020 07:07PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit e9d5770dd9324d14cce6b469277f8c55fa9821f4
Author: Jeremy Woods <jbwoods@google.com>
Date: Thu Sep 17 00:12:29 2020

Ensure transitions with only shared elements end

In DefaultSpecialEffects controller if there is a fragment that has only
a sharedElementTransition it currently does not have a way to end.

DefaultSpecialEffects should check if the transitionInfo has a
sharedElement when it is looking to start transitions, and when checking
if a transition involves a shared element.

Relnote: "Fragments that only have a shared element transition now
properly complete their special effects meaning they actually move to
their final state"
Test: noSharedElementToSharedElementTest
Bug: 166658128

Change-Id: Iaebc76914b9c24968a2943c18911b183f9dd8a4f

M fragment/fragment/src/androidTest/java/androidx/fragment/app/FragmentTransitionTest.kt
M fragment/fragment/src/main/java/androidx/fragment/app/DefaultSpecialEffectsController.java

https://android-review.googlesource.com/1428822

jb...@google.com <jb...@google.com> #6Sep 22, 2020 04:38PM

This should now be fixed.

You can verify this by following the snapshot instructions with build id 6847503 and switching your fragment dependency to 1.3.0-SNAPSHOT.

tj...@gmail.com <tj...@gmail.com> #7Sep 22, 2020 10:49PM

I'm sorry if I sound like a broken record, but the issue still seems to
persist when I test it with the sample repository provided:

onDestroyView is never called for Fragment 2 (DoggoPagerFragment), and when
I pop Fragment 3 (AdoptDoggoFragment) I'm met with the stale view from
Fragment 2

jb...@google.com <jb...@google.com> #8Sep 22, 2020 11:41PM

You're seeing it still broken with build id 6847503? (i.e. maven { url 'https://androidx.dev/snapshots/builds/6847503/artifacts/repository' } added to your repository?)

tj...@gmail.com <tj...@gmail.com> #9Sep 23, 2020 12:02AM

Oh my apologies! It persisted because I updated the fragment dependency to
androidx.fragment:fragment:1.3.0-SNAPSHOT, but forgot to update the ktx
extensions to androidx.fragment:fragment-ktx:1.3.0-SNAPSHOT.

Updating both fixes it, sorry about the false alarm and thanks for the fix!

>

jb...@google.com <jb...@google.com> #10Sep 23, 2020 01:07AM