React-native-navigation: StringIndexOutOfBoundsException on shared element transition

[SudoPlz]

Issue Description

As soon as we try to push to a new screen with shared element transitions we get a crash with the following stacktrace:

java.lang.StringIndexOutOfBoundsException: length=9; index=9
     FATAL EXCEPTION: main
Process: com.example.app.android, PID: 28135
java.lang.StringIndexOutOfBoundsException: length=9; index=9
    at java.lang.String.charAt(Native Method)
    at android.text.SpannableStringInternal.charAt(SpannableStringInternal.java:164)
    at com.shazam.android.widget.text.reflow.ReflowTextAnimatorHelper.getRuns(ReflowTextAnimatorHelper.java:193)
    at com.shazam.android.widget.text.reflow.ReflowTextAnimatorHelper.createAnimator(ReflowTextAnimatorHelper.java:122)
    at com.shazam.android.widget.text.reflow.ReflowTextAnimatorHelper$Builder.buildAnimator(ReflowTextAnimatorHelper.java:651)
    at com.reactnativenavigation.views.element.animators.TextChangeAnimator.create(TextChangeAnimator.kt:38)
    at com.reactnativenavigation.views.element.SharedElementTransition.createAnimators(SharedElementTransition.kt:25)
    at com.reactnativenavigation.views.element.TransitionAnimatorCreator.createSharedElementAnimator(TransitionAnimatorCreator.kt:75)
    at com.reactnativenavigation.views.element.TransitionAnimatorCreator.createSharedElementTransitionAnimators(TransitionAnimatorCreator.kt:68)
    at com.reactnativenavigation.views.element.TransitionAnimatorCreator.createAnimator(TransitionAnimatorCreator.kt:32)
    at com.reactnativenavigation.views.element.TransitionAnimatorCreator.create(TransitionAnimatorCreator.kt:25)
    at com.reactnativenavigation.views.element.TransitionAnimatorCreator$create$1.invokeSuspend(Unknown Source:16)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.UndispatchedCoroutine.afterResume(Builders.common.kt:214)
    at kotlinx.coroutines.AbstractCoroutine.resumeWith(AbstractCoroutine.kt:113)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:46)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:56)
    at kotlinx.coroutines.EventLoop.processUnconfinedEvent(EventLoop.common.kt:69)
    at kotlinx.coroutines.DispatchedTaskKt.resumeUnconfined(DispatchedTask.kt:184)
    at kotlinx.coroutines.DispatchedTaskKt.dispatch(DispatchedTask.kt:108)
    at kotlinx.coroutines.CancellableContinuationImpl.dispatchResume(CancellableContinuationImpl.kt:308)
    at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl(CancellableContinuationImpl.kt:318)
    at kotlinx.coroutines.CancellableContinuationImpl.resumeWith(CancellableContinuationImpl.kt:250)
    at com.reactnativenavigation.views.element.finder.OptimisticViewFinder$find$$inlined$suspendCancellableCoroutine$lambda$2.run(OptimisticViewFinder.kt:18)
    at com.reactnativenavigation.viewcontrollers.viewcontroller.-$$Lambda$P7xe27l85RASi5iJcSC7JIxgcA8.on(Unknown Source:2)
    at com.reactnativenavigation.utils.CollectionUtils.forEach(CollectionUtils.java:123)
    at com.reactnativenavigation.utils.CollectionUtils.forEach(CollectionUtils.java:117)
    at com.reactnativenavigation.viewcontrollers.viewcontroller.ViewController.lambda$onViewWillAppear$3$ViewController(ViewController.java:244)
    at com.reactnativenavigation.viewcontrollers.viewcontroller.-$$Lambda$ViewController$PhC2b_qLFURv0VgdFfWaEP_k-pE.run(Unknown Source:2)
    at android.os.Handler.handleCallback(Handler.java:873)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at androidx.test.espresso.base.Interrogator.a(Interrogator.java:31)
    at androidx.test.espresso.base.UiControllerImpl.a(UiControllerImpl.java:131)
    at androidx.test.espresso.base.UiControllerImpl.a(UiControllerImpl.java:125)
    at androidx.test.espresso.base.UiControllerImpl.a(UiControllerImpl.java:101)
    at androidx.test.espresso.action.Tap$1.sendTap(Tap.java:5)
    at androidx.test.espresso.action.GeneralClickAction.perform(GeneralClickAction.java:9)
    at androidx.test.espresso.ViewInteraction$SingleExecutionViewAction.perform(ViewInteraction.java:6)
    at androidx.test.espresso.ViewInteraction.a(Unknown Source:140)
    at androidx.test.espresso.ViewInteraction$1.call(Unknown Source:4)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at android.os.Handler.handleCallback(Handler.java:873)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loop(Looper.java:214)
    at android.app.ActivityThread.main(ActivityThread.java:7032)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:494)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:965)

---

Environment

• React Native Navigation version: "react-native-navigation": "^7.0.0",
• React Native version: "react-native": "^0.63.3",
• Platform(s) (iOS, Android, or both?): Android
• Device info (Simulator/Device? OS version? Debug/Release?): Galaxy S10 SM-G973F, API Level 28 Release (Doesn't happen on all android devices)

[SudoPlz]

@guyca by taking a first look, does that stack-trace ring any bells for you?

[guyca]

Hey @SudoPlz ! Unfortunately this is the first time I encounter a crash with the text animator 😢

Any information you can give on the texts being animated?

[SudoPlz]

@guyca that crash was caught on Firebase testlab right before we released. So we do have a vid, but since we animate multiple elements I can only guess which one was the faulty one.

After digging into that issue, it looks like reflow-animator get's the max char length from the source and target text strings here https://github.com/shazam/reflow-animator/blob/d5ca60b17dcc8015bd7385767c4e0a52b9225b94/library/src/main/java/com/shazam/android/widget/text/reflow/ReflowTextAnimatorHelper.java#L165

and then if any of those is bigger than the other, while iterating a crash is caused on this line https://github.com/shazam/reflow-animator/blob/d5ca60b17dcc8015bd7385767c4e0a52b9225b94/library/src/main/java/com/shazam/android/widget/text/reflow/ReflowTextAnimatorHelper.java#L190 because charAt is asked to return an index number that is invalid.

I worked around that issue by doing the following:

diff --git a/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/views/element/SharedElementTransition.kt b/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/views/element/SharedElementTransition.kt
index 1a7f610..193e0a3 100644
--- a/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/views/element/SharedElementTransition.kt
+++ b/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/views/element/SharedElementTransition.kt
@@ -22,11 +22,18 @@ class SharedElementTransition(appearing: ViewController<*>, private val options:
     override fun createAnimators(): AnimatorSet {
         val animators = animators()
                 .filter { it.shouldAnimateProperty() }
-                .map { it.create(options).apply {
-                    duration = options.getDuration()
-                    startDelay = options.getStartDelay()
-                    interpolator = options.getInterpolator()
-                } }
+                .map {
+                    try {
+                        return@map it.create(options).apply {
+                            duration = options.getDuration()
+                            startDelay = options.getStartDelay()
+                            interpolator = options.getInterpolator()
+                        }
+                    } catch (exc: RuntimeException ) {
+                        return@map null;
+                    }
+                }
+                .filterNotNull()
         val set = AnimatorSet()
         set.playTogether(animators)
         return set

because I couldn't easily reproduce that. Would you be kind enough to further test that use-case on your end if that's a crash you wish to avoid in the future?

[guyca]

@SudoPlz Any chance the texts being animated are not equal? Could they be of different lengths?

[SudoPlz]

@guyca duh, I must be stupid. yes they may not be absolutely equal indeed. That must be it.

[guyca]

@SudoPlz I'll add a quick fix to this now.
https://github.com/wix/react-native-navigation/blob/master/lib/android/app/src/main/java/com/reactnativenavigation/views/element/animators/TextChangeAnimator.kt#L17

Mistakingly - we animate the property of the length of the texts is different 🤦

[SudoPlz]

the power of 1 character xD awesome, I'm happy that we figured that out. And I totally missed that shouldAnimateProperty method 🤦 it makes rejecting that animator so much cleaner!

[guyca]

@SudoPlz Send me a video on Discord of how you're using shared element. I'm curious 🙈

[SudoPlz]

@guyca This is my first attempt with shared elements, here's what I tried to do:
https://share.squarespace.com/llu2RzZY

Btw it seems to me that the logic in that shouldAnimateProperty method should be

TextViewUtils.getTextSize(fromChild) == TextViewUtils.getTextSize(toChild) && !fromXy.equals(toXy.x, toXy.y)
instead of
TextViewUtils.getTextSize(fromChild) != TextViewUtils.getTextSize(toChild) || !fromXy.equals(toXy.x, toXy.y)

a.k.a animated of the texts have the same length and are not already in the same x/y position, but my understanding could be wrong.

[guyca]

Right, that's the fix 👍

[SudoPlz]

Awesome, I'm keeping that open in case you want to add a fix for that anyway. If not feel free to close that.
I'll fix it on our end too.

Thanks for taking a look I appreciate it man! 💯 ❤️

[guyca]

I wonder if the additional characters are the ellipsis which is added when the text gets truncated..

[SudoPlz]

I'm not following, what ellipsis are you referring to?

[SudoPlz]

Also while we're at it, I don't understand why the icon is turning to a [?] during the animation, this is another weird side effect.

[guyca]

~Rofl testSize is fontSize! the conditional is correct! 😂 We're just missing a check for the text length~

Edit: Never work after sunset.