Skip to content

Commit 6f73542

Browse files
YevheniiKotyrlometa-codesync[bot]
authored andcommitted
Reset accessibility state when recycling Android views (#59047)
Summary: With `enableViewRecycling` on, a recycled view keeps the accessibility state its previous props applied. `BaseViewManager.prepareToRecycleView` clears the accessibility tags, but not the content description, `importantForAccessibility`, the selected state, the live region, `screenReaderFocusable`, the `role` and collection tags, or the actions its accessibility delegate cached. The next view of that type is built from it and applies only the props it declares, so TalkBack announces a label and a role the view never had, the delegate sends TalkBack's click to an action only the previous props declared, and a view last used inside an `aria-hidden` subtree hides its new content from accessibility services. With `experimental_accessibilityOrder` on as well, a recycled `ReactViewGroup` keeps its previous order list, and a recycled child keeps the parent and focusability tags its former parent's order put on it. This resets that state in `prepareToRecycleView` to what a new view has, clears the order list in `ReactViewGroup.initView`, and restores `TextView`'s default importance in `ReactTextView.recycleView`. The delegate stays, since `ReactScrollView`, `ReactHorizontalScrollView` and `ReactNestedScrollView` install theirs in their constructors: a new `ReactAccessibilityDelegate.prepareToRecycle()` clears the actions it cached, and `accessibilityLabelledBy`, read only in the call that sets it, is a local there. It sits beside the `enabled` reset #58799 added. ## Changelog: [ANDROID] [FIXED] - Recycled views no longer keep the accessibility state of their previous props Pull Request resolved: #59047 Test Plan: New cases in `BaseViewManagerTest` and `ReactTextViewPropertyTest` recycle a view through `onDropViewInstance` and `createView` and compare it with a new one. Before the change six fail: content description `"Dismiss sheet"`, `role` tag `LIST`, a click dispatched to the previous props' `activate` action instead of performed, importance `4` where a new text view has `1`, the order list `["first"]`, and the former parent in `accessibility_order_parent`. Another case recycles each scroll view and checks that it keeps the `ReactScrollViewAccessibilityDelegate` its constructor installed. With the change, `./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest -PenableWarningsAsErrors=true` passes (727 tests) and the release sources compile with the same flag; the RNTester debug build and the rest of `build_android` were not run. RNTester with view recycling on, on an Android 16 emulator with TalkBack, [before and after](#59047 (comment)): plain rows mounted after labelled rows were removed are read as a selected "Dismiss sheet" button without the change, and as their text with it. On a Galaxy S23 (Android 14) app with view recycling on, plain rows mounted after rows carrying `aria-label`, `role`, `aria-selected`, `aria-disabled` and `aria-hidden` were removed reported the removed rows' label as their content description before the change. Built from source with it, the same rows report no label, role, selected or disabled state, and their text is in the accessibility tree. Both device runs used the first revision of this change, which cleared the delegate on recycle instead of resetting it. Reviewed By: Abbondanzo Differential Revision: D125427268 Pulled By: javache fbshipit-source-id: 1dad635ef6f1e5cef351d355865edc567f7bce58
1 parent 32131bf commit 6f73542

6 files changed

Lines changed: 246 additions & 4 deletions

File tree

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import androidx.annotation.ColorInt;
2020
import androidx.annotation.NonNull;
2121
import androidx.annotation.Nullable;
22+
import androidx.core.view.AccessibilityDelegateCompat;
2223
import androidx.core.view.ViewCompat;
2324
import com.facebook.common.logging.FLog;
2425
import com.facebook.react.R;
@@ -93,6 +94,11 @@ public BaseViewManager(@Nullable ReactApplicationContext reactContext) {
9394
view.setTag(R.id.accessibility_actions, null);
9495
view.setTag(R.id.accessibility_value, null);
9596
view.setTag(R.id.accessibility_state_expanded, null);
97+
view.setTag(R.id.accessibility_collection, null);
98+
view.setTag(R.id.accessibility_collection_item, null);
99+
view.setTag(R.id.role, null);
100+
view.setTag(R.id.accessibility_order_parent, null);
101+
view.setTag(R.id.original_focusability, null);
96102
view.setTag(R.id.accessibility_state_disabled, null);
97103
view.setEnabled(true);
98104
view.setTag(R.id.view_clipped, null);
@@ -161,6 +167,20 @@ public BaseViewManager(@Nullable ReactApplicationContext reactContext) {
161167
view.setFocusable(false);
162168
view.setFocusableInTouchMode(false);
163169

170+
// Accessibility state the props apply to the view itself, as a new view has it. The delegate
171+
// stays, since a view's constructor may have installed it, and forgets what it cached.
172+
AccessibilityDelegateCompat accessibilityDelegate = ViewCompat.getAccessibilityDelegate(view);
173+
if (accessibilityDelegate instanceof ReactAccessibilityDelegate) {
174+
((ReactAccessibilityDelegate) accessibilityDelegate).internal_prepareToRecycle();
175+
}
176+
view.setContentDescription(null);
177+
view.setSelected(false);
178+
view.setImportantForAccessibility(View.IMPORTANT_FOR_ACCESSIBILITY_AUTO);
179+
ViewCompat.setAccessibilityLiveRegion(view, ViewCompat.ACCESSIBILITY_LIVE_REGION_NONE);
180+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
181+
view.setScreenReaderFocusable(false);
182+
}
183+
164184
// Predictably, alpha defaults to 1:
165185
// https://android.googlesource.com/platform/frameworks/base/+/a175a5b/core/java/android/view/View.java#2186
166186
// This accounts for resetting mBackfaceOpacity and mBackfaceVisibility

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ public open class ReactAccessibilityDelegate( // The View this delegate is attac
5858
}
5959
}
6060
private val accessibilityActionsMap = HashMap<Int, String?>()
61-
private var accessibilityLabelledBy: View? = null
6261

6362
init {
6463
// We need to reset these two properties, as ExploreByTouchHelper sets focusable to "true" and
@@ -69,6 +68,12 @@ public open class ReactAccessibilityDelegate( // The View this delegate is attac
6968
hostView.importantForAccessibility = originalImportantForAccessibility
7069
}
7170

71+
/** Forgets the actions cached from the props of a view that is about to be recycled. */
72+
@JvmName("internal_prepareToRecycle")
73+
internal fun prepareToRecycle() {
74+
accessibilityActionsMap.clear()
75+
}
76+
7277
override fun onInitializeAccessibilityNodeInfo(host: View, info: AccessibilityNodeInfoCompat) {
7378
super.onInitializeAccessibilityNodeInfo(host, info)
7479

@@ -91,9 +96,9 @@ public open class ReactAccessibilityDelegate( // The View this delegate is attac
9196

9297
val accessibilityLabelledBy = host.getTag(R.id.labelled_by)
9398
if (accessibilityLabelledBy != null) {
94-
this.accessibilityLabelledBy = findView(host.rootView, accessibilityLabelledBy as String)
95-
if (this.accessibilityLabelledBy != null) {
96-
info.setLabeledBy(this.accessibilityLabelledBy)
99+
val labellingView = findView(host.rootView, accessibilityLabelledBy as String)
100+
if (labellingView != null) {
101+
info.setLabeledBy(labellingView)
97102
}
98103
}
99104

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,8 @@ private void initView() {
195195
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
196196
setFocusable(View.FOCUSABLE_AUTO);
197197
}
198+
// TextView's constructor makes the view important for accessibility
199+
setImportantForAccessibility(View.IMPORTANT_FOR_ACCESSIBILITY_YES);
198200

199201
setHyphenationFrequency(Layout.HYPHENATION_FREQUENCY_NONE);
200202
updateView(); // call after changing ellipsizeLocation in particular

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ public open class ReactViewGroup public constructor(context: Context?) :
189189
allChildrenCount = 0
190190
clippingRect = null
191191
hitSlopRect = null
192+
axOrderList = null
192193
// pointerEvents setter reads _overflow, so _overflow must be assigned first.
193194
_overflow = Overflow.VISIBLE
194195
pointerEvents = PointerEvents.AUTO

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

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,29 @@
99

1010
package com.facebook.react.uimanager
1111

12+
import android.view.View
1213
import android.view.View.OnFocusChangeListener
14+
import android.view.accessibility.AccessibilityNodeInfo
15+
import androidx.core.view.ViewCompat
1316
import com.facebook.react.R
1417
import com.facebook.react.bridge.BridgeReactContext
1518
import com.facebook.react.bridge.DynamicFromObject
1619
import com.facebook.react.bridge.JavaOnlyArray
1720
import com.facebook.react.bridge.JavaOnlyMap
21+
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
22+
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsDefaults
1823
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
24+
import com.facebook.react.touch.JSResponderHandler
25+
import com.facebook.react.views.scroll.ReactHorizontalScrollViewManager
26+
import com.facebook.react.views.scroll.ReactNestedScrollViewManager
27+
import com.facebook.react.views.scroll.ReactScrollViewAccessibilityDelegate
28+
import com.facebook.react.views.scroll.ReactScrollViewManager
1929
import com.facebook.react.views.view.ReactViewGroup
2030
import com.facebook.react.views.view.ReactViewManager
2131
import com.facebook.testutils.shadows.ShadowArguments
2232
import java.util.Locale
2333
import org.assertj.core.api.Assertions
34+
import org.junit.After
2435
import org.junit.Before
2536
import org.junit.Test
2637
import org.junit.runner.RunWith
@@ -41,12 +52,24 @@ class BaseViewManagerTest {
4152
@Before
4253
fun setUp() {
4354
ReactNativeFeatureFlagsForTests.setUp()
55+
ReactNativeFeatureFlags.override(
56+
object : ReactNativeFeatureFlagsDefaults() {
57+
override fun enableViewRecycling(): Boolean = true
58+
},
59+
)
60+
DisplayMetricsHolder.initDisplayMetricsIfNotInitialized(RuntimeEnvironment.getApplication())
4461
viewManager = ReactViewManager()
4562
val context = BridgeReactContext(RuntimeEnvironment.getApplication())
4663
themedReactContext = ThemedReactContext(context, context, null, -1)
4764
view = ReactViewGroup(themedReactContext)
4865
}
4966

67+
@After
68+
fun tearDown() {
69+
DisplayMetricsHolder.setScreenDisplayMetrics(null)
70+
ReactNativeFeatureFlags.dangerouslyReset()
71+
}
72+
5073
@Test
5174
fun testAccessibilityRoleNone() {
5275
viewManager.setAccessibilityRole(view, "none")
@@ -162,4 +185,168 @@ class BaseViewManagerTest {
162185
viewManager.setAccessibilityLabelledBy(view, DynamicFromObject(JavaOnlyArray()))
163186
Assertions.assertThat(view.getTag(R.id.labelled_by)).isNull()
164187
}
188+
189+
@Test
190+
fun testRecycledViewHasTheAccessibilityStateOfANewView() {
191+
val recycled =
192+
recycleView(
193+
JavaOnlyMap.of(
194+
ViewProps.ACCESSIBILITY_LABEL,
195+
"Dismiss sheet",
196+
ViewProps.ACCESSIBILITY_STATE,
197+
JavaOnlyMap.of("selected", true),
198+
ViewProps.ACCESSIBILITY_LIVE_REGION,
199+
"assertive",
200+
ViewProps.IMPORTANT_FOR_ACCESSIBILITY,
201+
"no-hide-descendants",
202+
ViewProps.SCREEN_READER_FOCUSABLE,
203+
true,
204+
)
205+
)
206+
207+
Assertions.assertThat(recycled.contentDescription).isEqualTo(view.contentDescription)
208+
Assertions.assertThat(recycled.isSelected).isEqualTo(view.isSelected)
209+
Assertions.assertThat(recycled.accessibilityLiveRegion).isEqualTo(view.accessibilityLiveRegion)
210+
Assertions.assertThat(recycled.importantForAccessibility)
211+
.isEqualTo(view.importantForAccessibility)
212+
Assertions.assertThat(recycled.isScreenReaderFocusable).isEqualTo(view.isScreenReaderFocusable)
213+
}
214+
215+
@Test
216+
fun testRecycledViewDropsTheRoleAndCollectionOfItsPreviousProps() {
217+
val recycled =
218+
recycleView(
219+
JavaOnlyMap.of(
220+
ViewProps.ROLE,
221+
"list",
222+
ViewProps.ACCESSIBILITY_COLLECTION,
223+
JavaOnlyMap.of("rowCount", 1, "columnCount", 1, "hierarchical", false),
224+
ViewProps.ACCESSIBILITY_COLLECTION_ITEM,
225+
JavaOnlyMap.of(
226+
"rowIndex",
227+
0,
228+
"rowSpan",
229+
1,
230+
"columnIndex",
231+
0,
232+
"columnSpan",
233+
1,
234+
"heading",
235+
false,
236+
),
237+
)
238+
)
239+
240+
Assertions.assertThat(recycled.getTag(R.id.role)).isNull()
241+
Assertions.assertThat(recycled.getTag(R.id.accessibility_collection)).isNull()
242+
Assertions.assertThat(recycled.getTag(R.id.accessibility_collection_item)).isNull()
243+
}
244+
245+
@Test
246+
fun testRecycledScrollViewsKeepTheAccessibilityDelegateTheirConstructorsInstall() {
247+
ReactNativeFeatureFlags.dangerouslyReset()
248+
ReactNativeFeatureFlags.override(
249+
object : ReactNativeFeatureFlagsDefaults() {
250+
override fun enableViewRecycling(): Boolean = true
251+
252+
override fun enableViewRecyclingForScrollView(): Boolean = true
253+
},
254+
)
255+
256+
val recycledScrollViews =
257+
listOf(
258+
recycleView(ReactScrollViewManager(), JavaOnlyMap()),
259+
recycleView(ReactHorizontalScrollViewManager(), JavaOnlyMap()),
260+
recycleView(ReactNestedScrollViewManager(), JavaOnlyMap()),
261+
)
262+
263+
for (recycled in recycledScrollViews) {
264+
Assertions.assertThat(ViewCompat.getAccessibilityDelegate(recycled))
265+
.isInstanceOf(ReactScrollViewAccessibilityDelegate::class.java)
266+
}
267+
}
268+
269+
@Test
270+
fun testRecycledViewPerformsItsOwnAccessibilityClick() {
271+
val recycled =
272+
recycleView(
273+
JavaOnlyMap.of(
274+
ViewProps.ACCESSIBILITY_ACTIONS,
275+
JavaOnlyArray.of(JavaOnlyMap.of("name", "activate")),
276+
)
277+
) { previous ->
278+
previous.createAccessibilityNodeInfo()
279+
}
280+
var clicks = 0
281+
recycled.setOnClickListener { clicks++ }
282+
283+
recycled.performAccessibilityAction(AccessibilityNodeInfo.ACTION_CLICK, null)
284+
285+
Assertions.assertThat(clicks).isEqualTo(1)
286+
}
287+
288+
@Test
289+
fun testRecycledViewDropsTheAccessibilityOrderOfItsPreviousProps() {
290+
ReactNativeFeatureFlags.dangerouslyReset()
291+
ReactNativeFeatureFlags.override(
292+
object : ReactNativeFeatureFlagsDefaults() {
293+
override fun enableViewRecycling(): Boolean = true
294+
295+
override fun enableAccessibilityOrder(): Boolean = true
296+
},
297+
)
298+
299+
val recycled =
300+
recycleView(JavaOnlyMap.of(ViewProps.ACCESSIBILITY_ORDER, JavaOnlyArray.of("first")))
301+
302+
Assertions.assertThat(recycled.axOrderList).isNull()
303+
}
304+
305+
@Test
306+
fun testRecycledViewDropsTheAccessibilityOrderItsParentApplied() {
307+
val parent = ReactViewGroup(themedReactContext)
308+
309+
val recycled =
310+
recycleView(JavaOnlyMap()) { previous ->
311+
ReactAxOrderHelper.buildAxOrderList(
312+
previous,
313+
parent,
314+
mutableListOf<String>(),
315+
arrayOfNulls(0),
316+
)
317+
ReactAxOrderHelper.disableFocusForSubtree(previous, mutableListOf<String>())
318+
}
319+
320+
Assertions.assertThat(recycled.getTag(R.id.accessibility_order_parent)).isNull()
321+
Assertions.assertThat(recycled.getTag(R.id.original_focusability)).isNull()
322+
}
323+
324+
private fun recycleView(
325+
previousProps: JavaOnlyMap,
326+
beforeDrop: (ReactViewGroup) -> Unit = {},
327+
): ReactViewGroup = recycleView(viewManager, previousProps, beforeDrop)
328+
329+
private fun <T : View> recycleView(
330+
manager: ViewManager<T, *>,
331+
previousProps: JavaOnlyMap,
332+
beforeDrop: (T) -> Unit = {},
333+
): T {
334+
val previous = createView(manager, previousProps)
335+
beforeDrop(previous)
336+
manager.onDropViewInstance(previous)
337+
val recycled = createView(manager, JavaOnlyMap())
338+
Assertions.assertThat(recycled).isSameAs(previous)
339+
return recycled
340+
}
341+
342+
private fun createView(props: JavaOnlyMap): ReactViewGroup = createView(viewManager, props)
343+
344+
private fun <T : View> createView(manager: ViewManager<T, *>, props: JavaOnlyMap): T =
345+
manager.createView(
346+
1,
347+
themedReactContext,
348+
ReactStylesDiffMap(props),
349+
null,
350+
JSResponderHandler(),
351+
)
165352
}

‎packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/ReactTextViewPropertyTest.kt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import com.facebook.react.bridge.BridgeReactContext
1616
import com.facebook.react.bridge.CatalystInstance
1717
import com.facebook.react.bridge.JavaOnlyMap
1818
import com.facebook.react.bridge.ReactTestHelper.createMockCatalystInstance
19+
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags
20+
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsDefaults
1921
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
22+
import com.facebook.react.touch.JSResponderHandler
2023
import com.facebook.react.uimanager.BackgroundStyleApplicator
2124
import com.facebook.react.uimanager.DisplayMetricsHolder
2225
import com.facebook.react.uimanager.LengthPercentage
@@ -44,6 +47,11 @@ class ReactTextViewPropertyTest {
4447
@Before
4548
fun setup() {
4649
ReactNativeFeatureFlagsForTests.setUp()
50+
ReactNativeFeatureFlags.override(
51+
object : ReactNativeFeatureFlagsDefaults() {
52+
override fun enableViewRecycling(): Boolean = true
53+
},
54+
)
4755
context = BridgeReactContext(RuntimeEnvironment.getApplication())
4856
catalystInstanceMock = createMockCatalystInstance()
4957
context.initializeWithInstance(catalystInstanceMock)
@@ -55,6 +63,7 @@ class ReactTextViewPropertyTest {
5563
@After
5664
fun teardown() {
5765
DisplayMetricsHolder.setScreenDisplayMetrics(null)
66+
ReactNativeFeatureFlags.dangerouslyReset()
5867
}
5968

6069
private fun buildStyles(vararg keysAndValues: Any?): ReactStylesDiffMap {
@@ -78,4 +87,22 @@ class ReactTextViewPropertyTest {
7887
assertThat(BackgroundStyleApplicator.getBorderRadius(view, BorderRadiusProp.BORDER_RADIUS))
7988
.isNull()
8089
}
90+
91+
@Test
92+
fun testRecycledViewIsAsImportantForAccessibilityAsANewView() {
93+
val previous =
94+
manager.createView(
95+
1,
96+
themedContext,
97+
buildStyles("importantForAccessibility", "no-hide-descendants"),
98+
null,
99+
JSResponderHandler(),
100+
)
101+
manager.onDropViewInstance(previous)
102+
val recycled = manager.createView(2, themedContext, buildStyles(), null, JSResponderHandler())
103+
104+
assertThat(recycled).isSameAs(previous)
105+
assertThat(recycled.importantForAccessibility)
106+
.isEqualTo(manager.createViewInstance(themedContext).importantForAccessibility)
107+
}
81108
}

0 commit comments

Comments
 (0)