Transitions fail to return to original state after animateToStart() and beginDelayedTransition() [300157785]

Fixed

Bug

Status Update

No update yet.

Description

jb...@google.com

created issue #1

Sep 13, 2023 12:31AM

Component used: Transition Version used: 1.5.0-alpha02 Devices/Android versions reproduced on: API 34

If you do a gesture back in Fragments using transitions and cancel the gesture multiple times, after the first cancel, starting the gesture will result in the transition on the exiting view failing to run.

I believe the cause of this is that the beginDelayedTransition() called by with a 0 duration called by Fragment never triggers this onAnimatedEnd() callback in Visibility. That results in the visibility failing to be added removed from the overlay and so the subsequent transitions on the view do not run.

Here are logs synced with the calls from Fragment and Visibility. I would expect there to be an onAnimationEnd reversed=false log before the onTransitionEnd in Visibility.

 V  Animating to start (in Fragment)
 D  onAnimationEnd reversed=true (in Visibility)
 V  Calling beginDelayedTransition (in Fragment)
 D  onTransitionEnd (in Visibility)
 V  On transitionEnd of beginDelayedTransition (in Fragment)

The can be reproduced by patching in aosp/2748867 and doing the following in the navigation-integration app:

Launch App
Press the "Navigate Next" button 2 times to get to the "two" screen
Start a gesture back to the "one" screen and cancel it
Start another gesture back to the "one" screen and the "two" screen will immediately go blank

Comments

mo...@google.com <mo...@google.com> Sep 13, 2023 04:31PM

Accepted by mo...@google.com.

mo...@google.com <mo...@google.com> #2Sep 18, 2023 08:29PM

Assigned to jb...@google.com.

I don't know if this is something that can be changed, but I'll look into it.

va...@google.com <va...@google.com> #3Sep 21, 2023 06:57PM

Hmm... it is supposed to work like this, but you are seeing a bug.

va...@google.com <va...@google.com> #4Sep 21, 2023 07:08PM

It cou8ld be tricky to get this right because the onAnimationEnd() and onTransitionEnd() often modify the state. We must execute the Runnable only after the state has been changed to its final value.

I think it could be dangerous to make a change, but it is possible to run all the internal listeners first, then run all of the external ones with the Runnable executed between. Again, it wouldn't be safe because we can't guarantee that only the internal listeners change the system state.

jb...@google.com <jb...@google.com> #5Sep 26, 2023 08:12PM

Passing to Jeremy to get his thoughts

jb...@google.com <jb...@google.com> #6Sep 26, 2023 08:46PM

What we really need is a signal to say when all transitions are done and with it running early, we don't have that. For fragments this means that we could end up moving the fragment to RESUMED before all the transitions are actually completed and execute some code that is normally fine when the system is settled, but dangerous when it is not.

If moving onTransitionEnd() might be too dangerous are there other options? If you are in the animateToStart case should you even get an onTransitionEnd(). Maybe all external listeners get is an onTransitionCancel() since you are reversing a transition you started in the middle of it. Is that a safer signal to send back?

I think it is okay to separate the signals as long as they are explicit, but we for sure need a way to know that the Transition system is completely done doing any work and our Views are back in the original state that we passed them in.

ap...@google.com <ap...@google.com> #7Sep 26, 2023 08:56PM

Maybe you could notify at the end of your animateToStart lambda?

jb...@google.com <jb...@google.com> #8Oct 25, 2023 12:42AM

Reassigned to mo...@google.com.

Per offline discussion, we can move ahead with the solution suggested in comment#4 and run external listeners last.

This will change the current order:

start transition > animateToStart > onTransitionEnd() > Runnable > finish up transition work

start transition > animateToStart > Runnable > finish up transition work > onTransitionEnd()

This has the implications that if developers are doing something with a specific result in their onTransitionEnd() callback, they will also need to handle the cancellation case by overriding the new onTransitionEnd(Transition, boolean).

For example, if there is a transition from View_A to View_B and the onTransitionEnd() callback has logic that expects View_B to be the final state, it will now need to add some additional logic based on the boolean in the case that View_A is the final state.

AnimateToStartAlpha03.zip

108 KB

Download

mo...@google.com <mo...@google.com> Nov 8, 2023 10:21PM

Accepted by mo...@google.com.

mo...@google.com <mo...@google.com> #9Nov 9, 2023 12:14AM

Project: platform/frameworks/support
Branch: androidx-main

commit 889b310dee76183e0f89e76d014728f728190740
Author: George Mount <mount@google.com>
Date: Wed Dec 20 14:35:35 2023

Change order of listeners for seeking transitions.

Fixes: 307624554

When TransitionSeekController.animateToStart() is used,
externally added TransitionListeners will have onTransitionEnd()
called after the animateToStart()'s lambda. Internally-added
listeners will be called before the animateToStart()'s lambda.

Test: new test
Change-Id: Ic6a55916958f85d41c54d7739bde09cfc2352d7c

M transition/transition/src/androidTest/java/androidx/transition/SeekTransitionTest.kt
M transition/transition/src/main/java/androidx/transition/Transition.java

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

mo...@google.com <mo...@google.com> #10Nov 9, 2023 06:18PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.transition:transition:1.5.0-alpha06

ap...@google.com <ap...@google.com> #11Nov 10, 2023 11:27PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-main

commit e57dd5f9ac6cbb8cf83b221e2d5b3fbd3e88ce6b
Author: George Mount <mount@google.com>
Date: Thu Nov 09 14:33:53 2023

Fix animateToStart with Slide.

Fixes: 300157785

Slide was not repositioning the View to its proper
translation after animating it to the start position.
This fixes that so that it is moved.

Test: new test
Change-Id: I698f4dbcef46304f9aa545847d205f7b70c80d63

M transition/transition/src/androidTest/java/androidx/transition/SlideEdgeTest.java
M transition/transition/src/androidTest/java/androidx/transition/TranslationAnimationCreatorTest.java
M transition/transition/src/main/java/androidx/transition/TranslationAnimationCreator.java

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

pr...@google.com <pr...@google.com> #12Nov 29, 2023 09:39PM