Skip to content
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

-[RCTAccessibilityManager announceForAccessibility:] calls into AppKit from background thread #1852

Merged
merged 1 commit into from
Jun 16, 2023

Conversation

nakambo
Copy link
Collaborator

@nakambo nakambo commented Jun 15, 2023

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

Specifically, NSAccessibilityPostNotificationWithUserInfo. Other methods such as getCurrentHighContrastState provide cached state (cached on main thread via observeValueForKeyPath et al). Some other methods such as setAccessibilityFocus dispatch to the main thread.

Not doing so could lead to AppKit further firing selectors on UI logic in app code and lead to downstream issues.

Test Plan

I've verified that original badness I observed in Office that led to discovering this no longer reproduces. Announcement posts in Accessibility Inspector.

…t from background thread

Specifically, NSAccessibilityPostNotificationWithUserInfo. Other methods such as `getCurrentHighContrastState` provide cached state (cached on main thread via `observeValueForKeyPath` et al). Some other methods such as `setAccessibilityFocus` dispatch to the main thread.

Not doing so could lead to AppKit further firing selectors on UI logic in app code and lead to downstream issues.
@nakambo nakambo requested a review from Saadnajmi June 15, 2023 23:09
@nakambo nakambo requested a review from a team as a code owner June 15, 2023 23:09
@github-actions
Copy link

Fails
🚫

📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • macOS
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against 1574d6e

@nakambo nakambo merged commit 04bf38c into main Jun 16, 2023
16 of 19 checks passed
@nakambo nakambo deleted the nakambo/fix-a11y-thread branch June 16, 2023 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants