When using Fragment 1.3.0 together with Leanback, GuidedStepSupportFragment will run the wrong transition animation and can crash. [179934757]

Fixed

Bug

Status Update

No update yet.

Description

lc...@gmail.com

created issue #1

Feb 11, 2021 03:38AM

Component used:

Fragment 1.3.0
Leanback 1.1.0-beta01 Devices/Android versions reproduced on: GoogleTV

When using the above components together, there are two problems:

GuidedStepSupportFragment transition (both UI_STYLE_ENTRANCE and UI_STYLE_REPLACE style) doesn't run properly
The App crashed the second time calling GuidedSupportFragment.add.

2021-02-11 12:15:58.811 29784-29784/com.example.guided_step_transition_bug E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.example.guided_step_transition_bug, PID: 29784
    java.lang.IllegalArgumentException: Unique transitionNames are required for all sharedElements
        at androidx.fragment.app.FragmentTransaction.addSharedElement(FragmentTransaction.java:599)
        at androidx.leanback.app.GuidedStepSupportFragment.addNonNullSharedElementTransition(GuidedStepSupportFragment.java:557)
        at androidx.leanback.app.GuidedStepSupportFragment.onAddSharedElementTransition(GuidedStepSupportFragment.java:534)
        at androidx.leanback.app.GuidedStepSupportFragment.add(GuidedStepSupportFragment.java:511)
        at androidx.leanback.app.GuidedStepSupportFragment.add(GuidedStepSupportFragment.java:476)
        at com.example.guided_step_transition_bug.ExampleGuidedStepFragment.onGuidedActionClicked(MainActivity.kt:35)
        at androidx.leanback.app.GuidedStepSupportFragment$2.onGuidedActionClicked(GuidedStepSupportFragment.java:1107)
        at androidx.leanback.widget.GuidedActionAdapter.performOnActionClick(GuidedActionAdapter.java:451)
        at androidx.leanback.widget.GuidedActionAdapter$1.onClick(GuidedActionAdapter.java:130)
        at android.view.View.performClick(View.java:7140)
        at android.view.View.performClickInternal(View.java:7117)
        at android.view.View.onKeyUp(View.java:14165)
        at android.view.KeyEvent.dispatch(KeyEvent.java:2824)
        at android.view.View.dispatchKeyEvent(View.java:13374)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1917)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at androidx.leanback.widget.BaseGridView.dispatchKeyEvent(BaseGridView.java:1081)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at androidx.leanback.widget.GuidedActionsRelativeLayout.dispatchKeyEvent(GuidedActionsRelativeLayout.java:75)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at android.view.ViewGroup.dispatchKeyEvent(ViewGroup.java:1922)
        at com.android.internal.policy.DecorView.superDispatchKeyEvent(DecorView.java:453)
        at com.android.internal.policy.PhoneWindow.superDispatchKeyEvent(PhoneWindow.java:1839)
        at android.app.Activity.dispatchKeyEvent(Activity.java:3968)
        at androidx.core.app.ComponentActivity.superDispatchKeyEvent(ComponentActivity.java:122)
        at androidx.core.view.KeyEventDispatcher.dispatchKeyEvent(KeyEventDispatcher.java:84)
        at androidx.core.app.ComponentActivity.dispatchKeyEvent(ComponentActivity.java:140)
        at androidx.appcompat.app.AppCompatActivity.dispatchKeyEvent(AppCompatActivity.java:535)
        at androidx.appcompat.view.WindowCallbackWrapper.dispatchKeyEvent(WindowCallbackWrapper.java:59)
        at androidx.appcompat.app.AppCompatDelegateImpl$AppCompatWindowCallback.dispatchKeyEvent(AppCompatDelegateImpl.java:2533)
        at com.android.internal.policy.DecorView.dispatchKeyEvent(DecorView.java:367)
        at android.view.ViewRootImpl$ViewPostImeInputStage.processKeyEvent(ViewRootImpl.java:5413)
        at android.view.ViewRootImpl$ViewPostImeInputStage.onProcess(ViewRootImpl.java:5281)
        at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788)
        at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4841)
        at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4807)
        at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:4947)
        at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4815)
        at android.view.ViewRootImpl$AsyncInputStage.apply(ViewRootImpl.java:5004)
2021-02-11 12:15:58.813 29784-29784/com.example.guided_step_transition_bug E/AndroidRuntime:     at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788)
        at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4841)
        at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4807)
        at android.view.ViewRootImpl$InputStage.apply(ViewRootImpl.java:4815)
        at android.view.ViewRootImpl$InputStage.deliver(ViewRootImpl.java:4788)
        at android.view.ViewRootImpl$InputStage.onDeliverToNext(ViewRootImpl.java:4841)
        at android.view.ViewRootImpl$InputStage.forward(ViewRootImpl.java:4807)
        at android.view.ViewRootImpl$AsyncInputStage.forward(ViewRootImpl.java:4980)
        at android.view.ViewRootImpl$ImeInputStage.onFinishedInputEvent(ViewRootImpl.java:5141)
        at android.view.inputmethod.InputMethodManager$PendingEvent.run(InputMethodManager.java:3064)
        at android.view.inputmethod.InputMethodManager.invokeFinishedInputEventCallback(InputMethodManager.java:2607)
        at android.view.inputmethod.InputMethodManager.finishedInputEvent(InputMethodManager.java:2598)
        at android.view.inputmethod.InputMethodManager$ImeInputEventSender.onInputEventFinished(InputMethodManager.java:3041)
        at android.view.InputEventSender.dispatchInputEventFinished(InputEventSender.java:143)
        at android.os.MessageQueue.nativePollOnce(Native Method)
        at android.os.MessageQueue.next(MessageQueue.java:336)
        at android.os.Looper.loop(Looper.java:174)
        at android.app.ActivityThread.main(ActivityThread.java:7356)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)

When using Fragment 1.2.5, there is no problem.

Sample project attached.

fragment_1_3_transition_bug.tar.gz

4.2 MB

Download

Comments

il...@google.com <il...@google.com> Feb 12, 2021 07:44PM

Assigned to da...@google.com.

da...@google.com <da...@google.com> #2Feb 17, 2021 07:19PM

Reassigned to il...@google.com.

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

il...@google.com <il...@google.com> Feb 17, 2021 07:27PM

Reassigned to jb...@google.com.

da...@google.com <da...@google.com> #3Feb 17, 2021 07:47PM

I have a crash workaround that just set all the transitionName again upon click the button.

It still has issue that it does not animate the right panel of the action list.

MainActivity.kt

4.9 KB

Download

fragment_1_3_0_with_workaround.webm

474 KB

Download

fragment_1_2_5.webm

545 KB

Download

da...@google.com <da...@google.com> #4Feb 17, 2021 08:42PM

For debugging: add this line to frameworks/support/samples/SupportLeanbackDemos/build.gradle to force it build from fragment library source: implementation(project(":fragment:fragment"))

Then install the sample app on TV emulator: ./gradlew support-leanback-demos:installDebug

There are two menus in the demo app home screen: "Guided Step" and "Guided Step (Support Version)", first is the framework fragment that doesn't crash, second is the support library version fragment that will crash with latest fragment source code.

jb...@google.com <jb...@google.com> #5Mar 4, 2021 04:55AM

Based on the videos, it looks like fragment 1.3.0 is doing the shared element transition for the next button and fragment 1.2.5 is not. In the 1.3.0, you see the text "Next" does not move, indicating that it is being shared between the two transitioning fragments. From the description, it seems like you don't want all of the transitions in the onAddSharedElementTransition method here. For this particular case, it seems like the only shared element you actual want is action_fragment_background, (here), but it is likely you may want some others (i.e. guidedactions_list_background, or guidedactions_list_background2, etc.) in other cases.

For the other issue of the shared element being null. It looks like we could be accidentally clearing the transition names of views that were marked as shared element transitions, but actually aren't. This isn't an issue if you use ViewCompat.setTransitionName() before adding your shared element transition, but if you set it in the view xml as leanback does, it never gets set again. Only adding shared elements for views that are actually doing a sharedElementTransition or calling setTransitionName before adding shared elements will always work regardless of what fragments does.

jb...@google.com <jb...@google.com> Mar 4, 2021 06:08PM

Reassigned to da...@google.com.

da...@google.com <da...@google.com> #6Mar 4, 2021 06:32PM

Reassigned to jb...@google.com.

It does want all of the shared elemen for transition.

The fragment replace fragment transition is defined here

https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:leanback/leanback/src/main/java/androidx/leanback/app/GuidedStepSupportFragment.java;l=904?q=GuidedStepSupportFragment

The reason that GuidedStepFragment want *all* of that to be shared element transition is because ChangeBounds transition needs to change bounds of every view in the hiearchy, from the RecyclerView, up to the root.

da...@google.com <da...@google.com> #7Mar 4, 2021 06:38PM

Reassigned to da...@google.com.

I will do some more investigation.

jb...@google.com <jb...@google.com> #8Mar 5, 2021 12:39AM

Reassigned to jb...@google.com.

We are going to address the issue with clearing the transition names that is causing the crash.

The transitions themselves are working as intended. The old system would only look at named views, not the children at all when targeting the shared element transition. We now look at the entire view hierarchy so children are transitioned properly.

ap...@google.com <ap...@google.com> #9Mar 8, 2021 05:04PM

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

commit ae6aff9326ce2b4f25731635230f600be18a3ba6
Author: Jeremy Woods <jbwoods@google.com>
Date: Thu Mar 04 15:28:27 2021

Ensure Fragments do not clear view transitionNames

When transitioning views are nested within ViewGroups that have
transitionNames but are not transitioning groups, the views are captured
as sharedElement views multiple times within the DefaultSpecialEffects
controller. As a result of this, as we collect and clear the transition
names, we capture a null name and end up restoring the null name to the
view once the shared element transition has finished.

We should only capture each shared element view once and therefore we
only restore it once, with the proper transition name.

RelNote: "Views within in a shared element hierarchy will no longer have
their transition name cleared when doing a shared element transition."
Test: testNestedSharedElementView
Bug: 179934757

Change-Id: I4d4a6d7770d5c6d54e4c647559d5cabae71f0051

A fragment/fragment/src/androidTest/java/androidx/fragment/app/FragmentSharedElementTransitionTest.kt
A fragment/fragment/src/androidTest/res/layout/nested_transition_groups.xml
M fragment/fragment/src/main/java/androidx/fragment/app/DefaultSpecialEffectsController.java

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

jb...@google.com <jb...@google.com> #10Mar 10, 2021 08:52PM

Marked as fixed.

This has been fixed internally and will be available in the next version of Fragments.

da...@google.com <da...@google.com> #11Mar 10, 2021 09:32PM

Thanks Jeremy!

Issue 179934757

Description

Issue summary

Comments

il...@google.com <il...@google.com> Feb 12, 2021 07:44PM

da...@google.com <da...@google.com> #2Feb 17, 2021 07:19PM

il...@google.com <il...@google.com> Feb 17, 2021 07:27PM

da...@google.com <da...@google.com> #3Feb 17, 2021 07:47PM

da...@google.com <da...@google.com> #4Feb 17, 2021 08:42PM

jb...@google.com <jb...@google.com> #5Mar 4, 2021 04:55AM

jb...@google.com <jb...@google.com> Mar 4, 2021 06:08PM

da...@google.com <da...@google.com> #6Mar 4, 2021 06:32PM

da...@google.com <da...@google.com> #7Mar 4, 2021 06:38PM

jb...@google.com <jb...@google.com> #8Mar 5, 2021 12:39AM

ap...@google.com <ap...@google.com> #9Mar 8, 2021 05:04PM

jb...@google.com <jb...@google.com> #10Mar 10, 2021 08:52PM

da...@google.com <da...@google.com> #11Mar 10, 2021 09:32PM

Add comment

Issue metadata