Keep a recycled view's accessibility delegate and reset its cached actions

Clearing the whole delegate in prepareToRecycleView also removed the
ReactScrollViewAccessibilityDelegate that ReactScrollView,
ReactHorizontalScrollView and ReactNestedScrollView install in their
constructors, which recycleView never installs again.

The delegate now stays, and ReactAccessibilityDelegate.prepareToRecycle()
clears the actions it cached from the previous props. accessibilityLabelledBy
was only read in the call that set it, so it is a local there.

Author: YevheniiKotyrlo

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

@@ -19,6 +19,7 @@
 import androidx.annotation.ColorInt;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.core.view.AccessibilityDelegateCompat;
 import androidx.core.view.ViewCompat;
 import com.facebook.common.logging.FLog;
 import com.facebook.react.R;
@@ -167,8 +168,11 @@ public BaseViewManager(@Nullable ReactApplicationContext reactContext) {
     view.setFocusableInTouchMode(false);
 
     // Accessibility state the props apply to the view itself, as a new view has it. The delegate
-    // is cleared on the view: ViewCompat would install an empty one and mark the view important.
-    view.setAccessibilityDelegate(null);
+    // stays, since a view's constructor may have installed it, and forgets what it cached.
+    AccessibilityDelegateCompat accessibilityDelegate = ViewCompat.getAccessibilityDelegate(view);
+    if (accessibilityDelegate instanceof ReactAccessibilityDelegate) {
+      ((ReactAccessibilityDelegate) accessibilityDelegate).internal_prepareToRecycle();
+    }
     view.setContentDescription(null);
     view.setSelected(false);
     view.setImportantForAccessibility(View.IMPORTANT_FOR_ACCESSIBILITY_AUTO);

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

@@ -57,7 +57,6 @@ public open class ReactAccessibilityDelegate( // The View this delegate is attac
         }
       }
   private val accessibilityActionsMap = HashMap<Int, String?>()
-  private var accessibilityLabelledBy: View? = null
 
   init {
     // We need to reset these two properties, as ExploreByTouchHelper sets focusable to "true" and
@@ -68,6 +67,12 @@ public open class ReactAccessibilityDelegate( // The View this delegate is attac
     hostView.importantForAccessibility = originalImportantForAccessibility
   }
 
+  /** Forgets the actions cached from the props of a view that is about to be recycled. */
+  @JvmName("internal_prepareToRecycle")
+  internal fun prepareToRecycle() {
+    accessibilityActionsMap.clear()
+  }
+
   override fun onInitializeAccessibilityNodeInfo(host: View, info: AccessibilityNodeInfoCompat) {
     super.onInitializeAccessibilityNodeInfo(host, info)
 
@@ -90,9 +95,9 @@ public open class ReactAccessibilityDelegate( // The View this delegate is attac
 
     val accessibilityLabelledBy = host.getTag(R.id.labelled_by)
     if (accessibilityLabelledBy != null) {
-      this.accessibilityLabelledBy = findView(host.rootView, accessibilityLabelledBy as String)
-      if (this.accessibilityLabelledBy != null) {
-        info.setLabeledBy(this.accessibilityLabelledBy)
+      val labellingView = findView(host.rootView, accessibilityLabelledBy as String)
+      if (labellingView != null) {
+        info.setLabeledBy(labellingView)
       }
     }
 

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.kt

@@ -9,6 +9,7 @@
 
 package com.facebook.react.uimanager
 
+import android.view.View
 import android.view.View.OnFocusChangeListener
 import android.view.accessibility.AccessibilityNodeInfo
 import androidx.core.view.ViewCompat
@@ -21,6 +22,10 @@ import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsDefaults
 import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
 import com.facebook.react.touch.JSResponderHandler
+import com.facebook.react.views.scroll.ReactHorizontalScrollViewManager
+import com.facebook.react.views.scroll.ReactNestedScrollViewManager
+import com.facebook.react.views.scroll.ReactScrollViewAccessibilityDelegate
+import com.facebook.react.views.scroll.ReactScrollViewManager
 import com.facebook.react.views.view.ReactViewGroup
 import com.facebook.react.views.view.ReactViewManager
 import com.facebook.testutils.shadows.ShadowArguments
@@ -238,11 +243,27 @@ class BaseViewManagerTest {
   }
 
   @Test
-  fun testRecycledViewWithoutAccessibilityPropsHasNoAccessibilityDelegate() {
-    val recycled = recycleView(JavaOnlyMap.of(ViewProps.ROLE, "button"))
+  fun testRecycledScrollViewsKeepTheAccessibilityDelegateTheirConstructorsInstall() {
+    ReactNativeFeatureFlags.dangerouslyReset()
+    ReactNativeFeatureFlags.override(
+        object : ReactNativeFeatureFlagsDefaults() {
+          override fun enableViewRecycling(): Boolean = true
+
+          override fun enableViewRecyclingForScrollView(): Boolean = true
+        },
+    )
+
+    val recycledScrollViews =
+        listOf(
+            recycleView(ReactScrollViewManager(), JavaOnlyMap()),
+            recycleView(ReactHorizontalScrollViewManager(), JavaOnlyMap()),
+            recycleView(ReactNestedScrollViewManager(), JavaOnlyMap()),
+        )
 
-    Assertions.assertThat(ViewCompat.hasAccessibilityDelegate(recycled))
-        .isEqualTo(ViewCompat.hasAccessibilityDelegate(view))
+    for (recycled in recycledScrollViews) {
+      Assertions.assertThat(ViewCompat.getAccessibilityDelegate(recycled))
+          .isInstanceOf(ReactScrollViewAccessibilityDelegate::class.java)
+    }
   }
 
   @Test
@@ -303,17 +324,25 @@ class BaseViewManagerTest {
   private fun recycleView(
       previousProps: JavaOnlyMap,
       beforeDrop: (ReactViewGroup) -> Unit = {},
-  ): ReactViewGroup {
-    val previous = createView(previousProps)
+  ): ReactViewGroup = recycleView(viewManager, previousProps, beforeDrop)
+
+  private fun <T : View> recycleView(
+      manager: ViewManager<T, *>,
+      previousProps: JavaOnlyMap,
+      beforeDrop: (T) -> Unit = {},
+  ): T {
+    val previous = createView(manager, previousProps)
     beforeDrop(previous)
-    viewManager.onDropViewInstance(previous)
-    val recycled = createView(JavaOnlyMap())
+    manager.onDropViewInstance(previous)
+    val recycled = createView(manager, JavaOnlyMap())
     Assertions.assertThat(recycled).isSameAs(previous)
     return recycled
   }
 
-  private fun createView(props: JavaOnlyMap): ReactViewGroup =
-      viewManager.createView(
+  private fun createView(props: JavaOnlyMap): ReactViewGroup = createView(viewManager, props)
+
+  private fun <T : View> createView(manager: ViewManager<T, *>, props: JavaOnlyMap): T =
+      manager.createView(
           1,
           themedReactContext,
           ReactStylesDiffMap(props),