Repository navigation
Keep the Text accessibility delegate when the text has no links - #59521
meta-codesync[bot] merged 1 commit into
Conversation
Every Text update reset the accessibility delegate, creating a new ExploreByTouchHelper even when TalkBack is off and the text has no links. Keep the existing delegate when the text has no links before or after the update, and make an "auto" view important as setting the delegate does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Visual regressionNo visual differences at ios: no capture |
Device measurement — with and without this changeI'm carrying this change in an app on React Native 0.86.3 and measured it on two phones. Without it — each With it — the median is 8.9 µs on the S23 (−46%) and 1.6 µs on the RedMagic (−30%).
Release builds of the same app with and without this PR's three files, react-native 0.86.3 compiled from source, each run flinging a list that mounts rows of Every run
The runs alternated without, with, with, without on each phone. Two runs on the same side differ by at most 0.26 µs in median. QuerySELECT slice.dur FROM slice
JOIN thread_track ON slice.track_id = thread_track.id
WHERE thread_track.utid = <the app's main thread>
AND slice.name = 'PreparedLayoutTextViewManager.updateExtraData' |
|
@javache could you take a look at this one? There's a device measurement in the comment above: |
|
@javache has imported this pull request. If you are a Meta employee, you can view this in D130917594. |
|
This pull request was successfully merged by @hryhoriiK97 in 502f112 When will my fix make it into a release? | How to file a pick request? |
## Summary: On Android, every Text update goes through `updateExtraData` in the Text view manager (`PreparedLayoutTextViewManager`, and `ReactTextViewManager` for the older text view). There, the text's clickable spans are stored in a view tag and the accessibility delegate is reset, so that TalkBack can reach the links. **Current behavior.** The comment says this is for a text view that "contains any clickable spans", but `PreparedLayoutTextViewManager` checks `is Spanned`, which is true for every Text, and `ReactTextViewManager` has no check at all. So every update of every Text creates a new `ReactTextViewAccessibilityDelegate`, even when TalkBack is off and the text has no links. Creating one is an `ExploreByTouchHelper`: it looks up the `AccessibilityManager`, makes the view focusable and then restores it, and allocates a `Handler` and a map. When the text has no links, nothing changes for the delegate, since it reads the links only when it's created. **With this change.** Both managers call a new `ReactTextViewAccessibilityDelegate.updateAccessibilityLinks`. When the view already has a `ReactTextViewAccessibilityDelegate` and the text has no links, before or after the update, it keeps that delegate. In every other case (the first update, links added, changed or removed, or another delegate on the view) it resets the delegate as before. A plain Text still gets the delegate on its first update; `PreparedLayoutTextView` needs it to expose its text to TalkBack. Setting a delegate through `ViewCompat` also makes a view whose `importantForAccessibility` is `auto` important. The kept path does the same, so a Text set back to `auto` (by the prop, or by view recycling) still ends up important, as before. The one difference: the accessibility actions a delegate caches from the `accessibilityActions` prop now stay between text updates, as they already do for every other view, instead of being dropped with the old delegate. ## Changelog: [ANDROID] [CHANGED] - Text updates keep the accessibility delegate when the text has no links ## Test Plan: - The pinned ktfmt (`--dry-run --set-exit-if-changed`) passes on all changed files. - Added `ReactTextViewAccessibilityDelegateTest` (4 Robolectric tests: the first update installs the text delegate; updates without links keep the same delegate; adding and removing links resets it; a view set back to `auto` is important after an update). With the old code only "updates without links keep the delegate" fails. `PreparedLayoutTextViewAccessibilityTextTest` and `ReactTextViewPropertyTest` still pass. - **Device:** Samsung Galaxy A16 (SM-A165F, Android 16), RNTester built with Gradle in Release (`assembleRelease`, arm64-v8a) and compiled with `cmd package compile -m speed`. The scenarios are a local Playground screen, not part of this PR: 200 numbers that change every 100 ms, and 60 lines of mixed-script text (emoji, CJK, Arabic, Hebrew, Polish) that change every 100 ms. - **A/B setup:** a single APK with a local switch (an `adb shell setprop`, not part of this PR) that runs either the old reset on every update or this change. Launches alternate A B B A A B B A, 4 per arm and scenario; 15 s Perfetto traces with the app's trace sections. The changes are medians. | | before | this PR | change | |---|--:|--:|--:| | `updateExtraData` per call, numbers | 36.6–39.1 µs | 28.4–30.0 µs | −21% | | Mount per commit, numbers | 12.65–13.20 ms | 10.74–11.35 ms | −13% | | Main thread CPU in 15 s, numbers | 3,614–3,654 ms | 3,484–3,625 ms | −3% | | `updateExtraData` per call, mixed scripts | 48.3–55.4 µs | 38.5–41.1 µs | −28% | | Mount per commit, mixed scripts | 5.49–6.06 ms | 4.79–5.07 ms | −18% | | Main thread CPU in 15 s, mixed scripts | 2,858–2,950 ms | 2,804–2,849 ms | −4% | These times include the cost of the trace sections themselves, about half of `updateExtraData`'s samples in a sampled profile, so without tracing the relative gain is larger. - **Accessibility:** `uiautomator dump` of `TextExample`, `AccessibilityExample` and `AccessibilityAndroidExample`, 4 scroll positions each, with the old and the new code: the same 166 nodes, with the same text, content description and focusable, clickable and important-for-accessibility flags (positions differ only by the scroll). I couldn't run TalkBack on the device I used. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Differential Revision: D130917594 Pull Request resolved: react#59521

Summary:
On Android, every Text update goes through
updateExtraDatain the Text view manager (PreparedLayoutTextViewManager, andReactTextViewManagerfor the older text view). There, the text's clickable spans are stored in a view tag and the accessibility delegate is reset, so that TalkBack can reach the links.Current behavior. The comment says this is for a text view that "contains any clickable spans", but
PreparedLayoutTextViewManagerchecksis Spanned, which is true for every Text, andReactTextViewManagerhas no check at all. So every update of every Text creates a newReactTextViewAccessibilityDelegate, even when TalkBack is off and the text has no links. Creating one is anExploreByTouchHelper: it looks up theAccessibilityManager, makes the view focusable and then restores it, and allocates aHandlerand a map. When the text has no links, nothing changes for the delegate, since it reads the links only when it's created.With this change. Both managers call a new
ReactTextViewAccessibilityDelegate.updateAccessibilityLinks. When the view already has aReactTextViewAccessibilityDelegateand the text has no links, before or after the update, it keeps that delegate. In every other case (the first update, links added, changed or removed, or another delegate on the view) it resets the delegate as before. A plain Text still gets the delegate on its first update;PreparedLayoutTextViewneeds it to expose its text to TalkBack.Setting a delegate through
ViewCompatalso makes a view whoseimportantForAccessibilityisautoimportant. The kept path does the same, so a Text set back toauto(by the prop, or by view recycling) still ends up important, as before. The one difference: the accessibility actions a delegate caches from theaccessibilityActionsprop now stay between text updates, as they already do for every other view, instead of being dropped with the old delegate.Changelog:
[ANDROID] [CHANGED] - Text updates keep the accessibility delegate when the text has no links
Test Plan:
--dry-run --set-exit-if-changed) passes on all changed files.ReactTextViewAccessibilityDelegateTest(4 Robolectric tests: the first update installs the text delegate; updates without links keep the same delegate; adding and removing links resets it; a view set back toautois important after an update). With the old code only "updates without links keep the delegate" fails.PreparedLayoutTextViewAccessibilityTextTestandReactTextViewPropertyTeststill pass.assembleRelease, arm64-v8a) and compiled withcmd package compile -m speed. The scenarios are a local Playground screen, not part of this PR: 200 numbers that change every 100 ms, and 60 lines of mixed-script text (emoji, CJK, Arabic, Hebrew, Polish) that change every 100 ms.adb shell setprop, not part of this PR) that runs either the old reset on every update or this change. Launches alternate A B B A A B B A, 4 per arm and scenario; 15 s Perfetto traces with the app's trace sections. The changes are medians.updateExtraDataper call, numbersupdateExtraDataper call, mixed scriptsThese times include the cost of the trace sections themselves, about half of
updateExtraData's samples in a sampled profile, so without tracing the relative gain is larger.uiautomator dumpofTextExample,AccessibilityExampleandAccessibilityAndroidExample, 4 scroll positions each, with the old and the new code: the same 166 nodes, with the same text, content description and focusable, clickable and important-for-accessibility flags (positions differ only by the scroll). I couldn't run TalkBack on the device I used.🤖 Generated with Claude Code