Skip to content

Commit 9526406

Browse files
jorge-cabfacebook-github-bot
authored andcommitted
Fix crash with nested FlatLists and fix edge case with nested views (#50855)
Summary: Pull Request resolved: #50855 This diff addresses a crash caused by view duplication in React Native Android. The issue occurred when a view was not already clipped and was laid out again, resulting in duplicated views. This problem was particularly noticeable when using nested FlatLists, which triggered a custom focus search with an incomplete and buggy duplicated FlatList container view. The fix involves preventing the duplication of views by checking if a view is clipped already before laying it out again. Additionally, this diff includes two other improvements: - Preventing clipping issues: When a view is nested within a non-ReactClippingViewGroup ancestor, focus searching would fail due to the needUpdateClippingRecursive logic only running on instances of ReactClippingViewGroup. By excluding these ancestors, we ensure that the next focusable view can be properly excluded from being clipped. - Minor fix: A minor fix was made to prevent potential issues in deeply nested cases. - Add a Kill switch with a feature flag and mobile config combo. Reviewed By: joevilches Differential Revision: D73471780 fbshipit-source-id: efbb968600f21b24ab1fa32222d555f346fb336e
1 parent 0f55ef7 commit 9526406

30 files changed

+385
-68
lines changed

packages/react-native/ReactAndroid/api/ReactAndroid.api

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2354,9 +2354,11 @@ public class com/facebook/react/fabric/FabricUIManager : com/facebook/react/brid
23542354
public fun dispatchCommand (IILcom/facebook/react/bridge/ReadableArray;)V
23552355
public fun dispatchCommand (IILjava/lang/String;Lcom/facebook/react/bridge/ReadableArray;)V
23562356
public fun dispatchCommand (ILjava/lang/String;Lcom/facebook/react/bridge/ReadableArray;)V
2357+
public fun findNextFocusableElement (III)Ljava/lang/Integer;
23572358
public fun getColor (I[Ljava/lang/String;)I
23582359
public fun getEventDispatcher ()Lcom/facebook/react/uimanager/events/EventDispatcher;
23592360
public fun getPerformanceCounters ()Ljava/util/Map;
2361+
public fun getRelativeAncestorList (II)[I
23602362
public fun getThemeData (I[F)Z
23612363
public fun initialize ()V
23622364
public fun invalidate ()V
@@ -3969,6 +3971,7 @@ public abstract interface class com/facebook/react/uimanager/ReactClippingViewGr
39693971
public abstract fun getRemoveClippedSubviews ()Z
39703972
public abstract fun setRemoveClippedSubviews (Z)V
39713973
public abstract fun updateClippingRect ()V
3974+
public abstract fun updateClippingRect (Ljava/util/Set;)V
39723975
}
39733976

39743977
public final class com/facebook/react/uimanager/ReactClippingViewGroupHelper {
@@ -5832,6 +5835,7 @@ public class com/facebook/react/views/scroll/ReactHorizontalScrollView : android
58325835
public fun executeKeyEvent (Landroid/view/KeyEvent;)Z
58335836
public fun flashScrollIndicators ()V
58345837
public fun fling (I)V
5838+
public fun focusSearch (Landroid/view/View;I)Landroid/view/View;
58355839
public fun getChildVisibleRect (Landroid/view/View;Landroid/graphics/Rect;Landroid/graphics/Point;)Z
58365840
public fun getClippingRect (Landroid/graphics/Rect;)V
58375841
public fun getFlingAnimator ()Landroid/animation/ValueAnimator;
@@ -5894,6 +5898,7 @@ public class com/facebook/react/views/scroll/ReactHorizontalScrollView : android
58945898
public fun setStateWrapper (Lcom/facebook/react/uimanager/StateWrapper;)V
58955899
public fun startFlingAnimator (II)V
58965900
public fun updateClippingRect ()V
5901+
public fun updateClippingRect (Ljava/util/Set;)V
58975902
}
58985903

58995904
public class com/facebook/react/views/scroll/ReactHorizontalScrollViewManager : com/facebook/react/uimanager/ViewGroupManager, com/facebook/react/views/scroll/ReactScrollViewCommandHelper$ScrollCommandHandler {
@@ -5959,6 +5964,7 @@ public class com/facebook/react/views/scroll/ReactScrollView : android/widget/Sc
59595964
public fun executeKeyEvent (Landroid/view/KeyEvent;)Z
59605965
public fun flashScrollIndicators ()V
59615966
public fun fling (I)V
5967+
public fun focusSearch (Landroid/view/View;I)Landroid/view/View;
59625968
public fun getChildVisibleRect (Landroid/view/View;Landroid/graphics/Rect;Landroid/graphics/Point;)Z
59635969
public fun getClippingRect (Landroid/graphics/Rect;)V
59645970
public fun getFlingAnimator ()Landroid/animation/ValueAnimator;
@@ -6022,6 +6028,7 @@ public class com/facebook/react/views/scroll/ReactScrollView : android/widget/Sc
60226028
public fun setStateWrapper (Lcom/facebook/react/uimanager/StateWrapper;)V
60236029
public fun startFlingAnimator (II)V
60246030
public fun updateClippingRect ()V
6031+
public fun updateClippingRect (Ljava/util/Set;)V
60256032
}
60266033

60276034
public final class com/facebook/react/views/scroll/ReactScrollViewCommandHelper {
@@ -6079,6 +6086,7 @@ public final class com/facebook/react/views/scroll/ReactScrollViewHelper {
60796086
public static final fun emitScrollEvent (Landroid/view/ViewGroup;FF)V
60806087
public static final fun emitScrollMomentumBeginEvent (Landroid/view/ViewGroup;II)V
60816088
public static final fun emitScrollMomentumEndEvent (Landroid/view/ViewGroup;)V
6089+
public static final fun findNextFocusableView (Landroid/view/ViewGroup;Landroid/view/View;IZ)Landroid/view/View;
60826090
public static final fun forceUpdateState (Landroid/view/ViewGroup;)V
60836091
public static final fun getDefaultScrollAnimationDuration (Landroid/content/Context;)I
60846092
public static final fun getNextFlingStartValue (Landroid/view/ViewGroup;III)I
@@ -6088,6 +6096,7 @@ public final class com/facebook/react/views/scroll/ReactScrollViewHelper {
60886096
public final fun registerFlingAnimator (Landroid/view/ViewGroup;)V
60896097
public static final fun removeLayoutChangeListener (Lcom/facebook/react/views/scroll/ReactScrollViewHelper$LayoutChangeListener;)V
60906098
public static final fun removeScrollListener (Lcom/facebook/react/views/scroll/ReactScrollViewHelper$ScrollListener;)V
6099+
public static final fun resolveAbsoluteDirection (IZI)I
60916100
public static final fun smoothScrollTo (Landroid/view/ViewGroup;II)V
60926101
public static final fun updateFabricScrollState (Landroid/view/ViewGroup;)V
60936102
public final fun updateFabricScrollState (Landroid/view/ViewGroup;II)V
@@ -6865,6 +6874,7 @@ public class com/facebook/react/views/view/ReactViewGroup : android/view/ViewGro
68656874
public fun setRemoveClippedSubviews (Z)V
68666875
public fun setTranslucentBackgroundDrawable (Landroid/graphics/drawable/Drawable;)V
68676876
public fun updateClippingRect ()V
6877+
public fun updateClippingRect (Ljava/util/Set;)V
68686878
public fun updateDrawingOrder ()V
68696879
}
68706880

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import androidx.annotation.AnyThread;
2828
import androidx.annotation.Nullable;
2929
import androidx.annotation.UiThread;
30+
import androidx.core.view.ViewCompat.FocusRealDirection;
3031
import com.facebook.common.logging.FLog;
3132
import com.facebook.infer.annotation.Assertions;
3233
import com.facebook.infer.annotation.Nullsafe;
@@ -262,6 +263,52 @@ public <T extends View> int addRootView(
262263
return rootTag;
263264
}
264265

266+
/**
267+
* Find the next focusable element's id and position relative to the parent from the shadow tree
268+
* based on the current focusable element and the direction.
269+
*
270+
* @return A NextFocusableNode object where the 'id' is the reactId/Tag of the next focusable
271+
* view, returns null if no view could be found
272+
*/
273+
public @Nullable Integer findNextFocusableElement(
274+
int parentTag, int focusedTag, @FocusRealDirection int direction) {
275+
if (mBinding == null) {
276+
return null;
277+
}
278+
279+
int generalizedDirection;
280+
281+
switch (direction) {
282+
case View.FOCUS_DOWN:
283+
generalizedDirection = 0;
284+
break;
285+
case View.FOCUS_UP:
286+
generalizedDirection = 1;
287+
break;
288+
case View.FOCUS_RIGHT:
289+
generalizedDirection = 2;
290+
break;
291+
case View.FOCUS_LEFT:
292+
generalizedDirection = 3;
293+
break;
294+
default:
295+
return null;
296+
}
297+
298+
int serializedNextFocusableNodeMetrics =
299+
mBinding.findNextFocusableElement(parentTag, focusedTag, generalizedDirection);
300+
301+
if (serializedNextFocusableNodeMetrics == -1) {
302+
return null;
303+
}
304+
305+
return serializedNextFocusableNodeMetrics;
306+
}
307+
308+
public @Nullable int[] getRelativeAncestorList(int rootTag, int childTag) {
309+
return mBinding != null ? mBinding.getRelativeAncestorList(rootTag, childTag) : null;
310+
}
311+
265312
@Override
266313
@AnyThread
267314
@ThreadConfined(ANY)

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManagerBinding.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ internal class FabricUIManagerBinding : HybridClassBase() {
5757

5858
external fun findNextFocusableElement(parentTag: Int, focusedTag: Int, direction: Int): Int
5959

60-
external fun findRelativeTopMostParent(rootTag: Int, childTag: Int): Int
60+
external fun getRelativeAncestorList(rootTag: Int, childTag: Int): IntArray
6161

6262
external fun stopSurface(surfaceId: Int)
6363

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlags.kt

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<48d7086046009130361c8878bded54d0>>
7+
* @generated SignedSource<<3043c6e2fd674eaf2c1d6c3a064083b2>>
88
*/
99

1010
/**
@@ -84,6 +84,12 @@ public object ReactNativeFeatureFlags {
8484
@JvmStatic
8585
public fun enableCppPropsIteratorSetter(): Boolean = accessor.enableCppPropsIteratorSetter()
8686

87+
/**
88+
* This enables the fabric implementation of focus search so that we can focus clipped elements
89+
*/
90+
@JvmStatic
91+
public fun enableCustomFocusSearchOnClippedElementsAndroid(): Boolean = accessor.enableCustomFocusSearchOnClippedElementsAndroid()
92+
8793
/**
8894
* Feature flag to configure eager attachment of the root view/initialisation of the JS code.
8995
*/

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxAccessor.kt

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<2657b39d8c9d8c0020139c267f3c7b2b>>
7+
* @generated SignedSource<<090ff0c403728370f2c88a2434c9ae12>>
88
*/
99

1010
/**
@@ -29,6 +29,7 @@ internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAcces
2929
private var enableAccumulatedUpdatesInRawPropsAndroidCache: Boolean? = null
3030
private var enableBridgelessArchitectureCache: Boolean? = null
3131
private var enableCppPropsIteratorSetterCache: Boolean? = null
32+
private var enableCustomFocusSearchOnClippedElementsAndroidCache: Boolean? = null
3233
private var enableEagerRootViewAttachmentCache: Boolean? = null
3334
private var enableFabricLogsCache: Boolean? = null
3435
private var enableFabricRendererCache: Boolean? = null
@@ -143,6 +144,15 @@ internal class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAcces
143144
return cached
144145
}
145146

147+
override fun enableCustomFocusSearchOnClippedElementsAndroid(): Boolean {
148+
var cached = enableCustomFocusSearchOnClippedElementsAndroidCache
149+
if (cached == null) {
150+
cached = ReactNativeFeatureFlagsCxxInterop.enableCustomFocusSearchOnClippedElementsAndroid()
151+
enableCustomFocusSearchOnClippedElementsAndroidCache = cached
152+
}
153+
return cached
154+
}
155+
146156
override fun enableEagerRootViewAttachment(): Boolean {
147157
var cached = enableEagerRootViewAttachmentCache
148158
if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsCxxInterop.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<67fd03886fec1f6874bf3ef07878bdc7>>
7+
* @generated SignedSource<<1d7b3dd1a1e40eb7802834c22eb01256>>
88
*/
99

1010
/**
@@ -46,6 +46,8 @@ public object ReactNativeFeatureFlagsCxxInterop {
4646

4747
@DoNotStrip @JvmStatic public external fun enableCppPropsIteratorSetter(): Boolean
4848

49+
@DoNotStrip @JvmStatic public external fun enableCustomFocusSearchOnClippedElementsAndroid(): Boolean
50+
4951
@DoNotStrip @JvmStatic public external fun enableEagerRootViewAttachment(): Boolean
5052

5153
@DoNotStrip @JvmStatic public external fun enableFabricLogs(): Boolean

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsDefaults.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<4aa66c9504fb46e2a6b052d708fdc4b0>>
7+
* @generated SignedSource<<bf8b0d4c128a040853d2b12f74597d46>>
88
*/
99

1010
/**
@@ -41,6 +41,8 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi
4141

4242
override fun enableCppPropsIteratorSetter(): Boolean = false
4343

44+
override fun enableCustomFocusSearchOnClippedElementsAndroid(): Boolean = true
45+
4446
override fun enableEagerRootViewAttachment(): Boolean = false
4547

4648
override fun enableFabricLogs(): Boolean = false

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsLocalAccessor.kt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<259f5f37683db64fd63ffb92e4fbae54>>
7+
* @generated SignedSource<<4c8f6a3a41aebb8c9e81c26cd7973f90>>
88
*/
99

1010
/**
@@ -33,6 +33,7 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
3333
private var enableAccumulatedUpdatesInRawPropsAndroidCache: Boolean? = null
3434
private var enableBridgelessArchitectureCache: Boolean? = null
3535
private var enableCppPropsIteratorSetterCache: Boolean? = null
36+
private var enableCustomFocusSearchOnClippedElementsAndroidCache: Boolean? = null
3637
private var enableEagerRootViewAttachmentCache: Boolean? = null
3738
private var enableFabricLogsCache: Boolean? = null
3839
private var enableFabricRendererCache: Boolean? = null
@@ -156,6 +157,16 @@ internal class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcc
156157
return cached
157158
}
158159

160+
override fun enableCustomFocusSearchOnClippedElementsAndroid(): Boolean {
161+
var cached = enableCustomFocusSearchOnClippedElementsAndroidCache
162+
if (cached == null) {
163+
cached = currentProvider.enableCustomFocusSearchOnClippedElementsAndroid()
164+
accessedFeatureFlags.add("enableCustomFocusSearchOnClippedElementsAndroid")
165+
enableCustomFocusSearchOnClippedElementsAndroidCache = cached
166+
}
167+
return cached
168+
}
169+
159170
override fun enableEagerRootViewAttachment(): Boolean {
160171
var cached = enableEagerRootViewAttachmentCache
161172
if (cached == null) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/featureflags/ReactNativeFeatureFlagsProvider.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<3082bb1b859031b09fd70de878722016>>
7+
* @generated SignedSource<<5a02fc6cd183f41724b3599fad1fd507>>
88
*/
99

1010
/**
@@ -41,6 +41,8 @@ public interface ReactNativeFeatureFlagsProvider {
4141

4242
@DoNotStrip public fun enableCppPropsIteratorSetter(): Boolean
4343

44+
@DoNotStrip public fun enableCustomFocusSearchOnClippedElementsAndroid(): Boolean
45+
4446
@DoNotStrip public fun enableEagerRootViewAttachment(): Boolean
4547

4648
@DoNotStrip public fun enableFabricLogs(): Boolean

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactClippingViewGroup.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ public interface ReactClippingViewGroup {
3232
*/
3333
public fun updateClippingRect()
3434

35+
public fun updateClippingRect(excludedView: Set<Int>?)
36+
3537
/**
3638
* Get rectangular bounds to which view is currently clipped to. Called only on views that has set
3739
* `removeCLippedSubviews` property value to `true`.

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import static com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_DISABLED;
1212
import static com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_END;
1313
import static com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_START;
14+
import static com.facebook.react.views.scroll.ReactScrollViewHelper.findNextFocusableView;
1415

1516
import android.animation.ObjectAnimator;
1617
import android.animation.ValueAnimator;
@@ -31,6 +32,7 @@
3132
import android.widget.OverScroller;
3233
import androidx.annotation.Nullable;
3334
import androidx.core.view.ViewCompat;
35+
import androidx.core.view.ViewCompat.FocusRealDirection;
3436
import com.facebook.common.logging.FLog;
3537
import com.facebook.infer.annotation.Assertions;
3638
import com.facebook.infer.annotation.Nullsafe;
@@ -39,6 +41,7 @@
3941
import com.facebook.react.bridge.ReactContext;
4042
import com.facebook.react.common.ReactConstants;
4143
import com.facebook.react.common.build.ReactBuildConfig;
44+
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
4245
import com.facebook.react.uimanager.BackgroundStyleApplicator;
4346
import com.facebook.react.uimanager.LengthPercentage;
4447
import com.facebook.react.uimanager.LengthPercentageType;
@@ -64,6 +67,7 @@
6467
import java.lang.reflect.Field;
6568
import java.util.ArrayList;
6669
import java.util.List;
70+
import java.util.Set;
6771

6872
/** Similar to {@link ReactScrollView} but only supports horizontal scrolling. */
6973
@Nullsafe(Nullsafe.Mode.LOCAL)
@@ -771,8 +775,26 @@ protected void onDetachedFromWindow() {
771775
}
772776
}
773777

778+
@Override
779+
public @Nullable View focusSearch(View focused, @FocusRealDirection int direction) {
780+
if (ReactNativeFeatureFlags.enableCustomFocusSearchOnClippedElementsAndroid()) {
781+
@Nullable View nextfocusableView = findNextFocusableView(this, focused, direction, true);
782+
783+
if (nextfocusableView != null) {
784+
return nextfocusableView;
785+
}
786+
}
787+
788+
return super.focusSearch(focused, direction);
789+
}
790+
774791
@Override
775792
public void updateClippingRect() {
793+
updateClippingRect(null);
794+
}
795+
796+
@Override
797+
public void updateClippingRect(@Nullable Set<Integer> excludedViewId) {
776798
if (!mRemoveClippedSubviews) {
777799
return;
778800
}
@@ -784,7 +806,7 @@ public void updateClippingRect() {
784806
ReactClippingViewGroupHelper.calculateClippingRect(this, mClippingRect);
785807
View contentView = getContentView();
786808
if (contentView instanceof ReactClippingViewGroup) {
787-
((ReactClippingViewGroup) contentView).updateClippingRect();
809+
((ReactClippingViewGroup) contentView).updateClippingRect(excludedViewId);
788810
}
789811
} finally {
790812
Systrace.endSection(Systrace.TRACE_TAG_REACT);

0 commit comments

Comments
 (0)