-
Notifications
You must be signed in to change notification settings - Fork 24.9k
[iOS][Accessibility][Critical?] #35774: accessibility state race condition #54131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[iOS][Accessibility][Critical?] #35774: accessibility state race condition #54131
Conversation
Refactor accessibilityIncrement and accessibilityDecrement to fix a race condition on iOS.
Hi @maxencehenneron! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
This is actually worse than I thought: We have the same issue with custom actions (my last commit fixes it). Meaning running a custom action that updates the accessibilityState (for example "selected", "expanded") will announce the previous state so any vision-impaired user would think the action was not performed and run it again. That time it would announce "selected" when it's actually not selected anymore. I want to stress that accessibility is a legal requirement in Europe and the US and required by many legal frameworks. |
…bilityPerformEscape
This PR can now be reviewed |
Great, finally 👍 I wonder why this was never fixed before, because we have this issue at IBM iX for many of our clients apps and as a11y is a legal requirement in the EU we getting more and more trouble with this issue 😅 Even if this uses experimental code, it seems to be the easiest fix. |
Small note on my PR: You have to implement |
Refactor accessibility actions to fix a race condition on iOS. Fixes #35774
Summary:
I know you will most likely not like my fix, but there currently is a race condition that creates issues for our most vulnerable users.
Any accessibility action performed on an element on iOS are out of sync, from custom actions, taps, magic taps, increment, decrement or escape.
This is because the VoiceOver on iOS will immediately read the text once an action is called as it expects the function to update accessibilityValue / accessibilityState. The problem is we're emitting an asynchronous event to update this value so there is currently no easy way to fix this.
What I found is working is using this experimental_flushSync function to update the state synchronously. This is obviously bad for performance but i'd argue that for the current context (vision impairment) having 60fps when running an action (that doesn't happen often) is less important than having wrong vocal directions.
Changelog:
[iOS] [Fixed] - Updated accessibilityIncrement, accessibilityDecrement, accessibilityActivate, accessibilityPerformEscape, accessibilityPerformMagicTap to synchronously update the state when the action is run
[iOS] [Fixed] - Add missing accessibility props to TouchableOpacity
Test Plan:
Before:
ScreenRecording_10-10-2025.16-23-24_1.mov
After:
https://github.com/user-attachments/assets/5e31e2de-63e9-4234-bb4c-d46dfe1cabcf