From c381c13a1a6d991123ae6fc4cf65ff7abbd5a776 Mon Sep 17 00:00:00 2001 From: Bartlomiej Bloniarz Date: Fri, 23 Oct 2026 02:36:21 -0700 Subject: [PATCH] Reland: [Android] Fix View accessibility payloads that Props 2.0 sends wrong or drops Summary: Reland of https://github.com/react/react-native/pull/59079. It was reverted because code outside this repository that compiles against these headers still read `role` as a plain `Role` and stopped compiling. With Props 2.0 on Android (`enableExclusivePropsUpdateAndroid` and the related flags), `BaseViewManager` receives the View payload from `getDiffProps`. Some accessibility props come out wrong: - Changing one member of `accessibilityState`, such as `busy`: only that member is sent, and Java treats the map as the whole state, so a selected, disabled view becomes unselected and enabled. - A removed `accessibilityRole` or `role`: not cleared. - An explicit `role="none"`: not sent on mount, so the view falls back to `accessibilityRole`. - A removed `accessibilityValue`: sent as `{}`, so the old value stays in the content description. This PR sends `accessibilityState` in full, clears a removed role or value, and makes `role` optional so that unset and `"none"` differ. Nothing changes on Android with the flags off. iOS shares the props constructor, so a removed role is now also cleared in its C++ props. I didn't test iOS. ## Changelog: [ANDROID] [FIXED] - Fix accessibilityState, a removed accessibilityRole, role or accessibilityValue, and role="none" with Props 2.0 Differential Revision: D126922843 --- .../View/RCTViewComponentView.mm | 9 +- .../components/view/AccessibilityProps.cpp | 15 ++-- .../components/view/AccessibilityProps.h | 2 +- .../components/view/HostPlatformViewProps.cpp | 87 ++++++++----------- .../view/tests/ViewPropsDiffTest.cpp | 49 +++++++++++ .../api-snapshots/ReactAndroidDebugCxx.api | 2 +- .../ReactAndroidFrameworksCxx.api | 2 +- .../api-snapshots/ReactAndroidNewarchCxx.api | 2 +- .../api-snapshots/ReactAndroidReleaseCxx.api | 2 +- .../api-snapshots/ReactAppleDebugCxx.api | 2 +- .../api-snapshots/ReactAppleFrameworksCxx.api | 2 +- .../api-snapshots/ReactAppleNewarchCxx.api | 2 +- .../api-snapshots/ReactAppleReleaseCxx.api | 2 +- .../api-snapshots/ReactCommonDebugCxx.api | 2 +- .../ReactCommonFrameworksCxx.api | 2 +- .../api-snapshots/ReactCommonNewarchCxx.api | 2 +- .../api-snapshots/ReactCommonReleaseCxx.api | 2 +- 17 files changed, 111 insertions(+), 75 deletions(-) diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm b/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm index bdd0bb5af232..8a074cd2b604 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm @@ -67,7 +67,7 @@ static BOOL RCTViewIsInteractiveAccessibilityElement(UIView *view, const ViewPro // `role` wins over the legacy `accessibilityRole` when both are set, matching // how the traits themselves are resolved. - if (props.role != Role::None) { + if (props.role.value_or(Role::None) != Role::None) { static const std::unordered_set interactiveRoles{ Role::Button, Role::Checkbox, @@ -82,7 +82,7 @@ static BOOL RCTViewIsInteractiveAccessibilityElement(UIView *view, const ViewPro Role::Switch, Role::Tab, Role::Treeitem}; - return interactiveRoles.contains(props.role); + return interactiveRoles.contains(*props.role); } static const std::unordered_set interactiveAccessibilityRoles{ @@ -1690,8 +1690,9 @@ - (NSString *)accessibilityValue } NSMutableArray *valueComponents = [NSMutableArray new]; - NSString *roleString = (props.role != Role::None) ? [NSString stringWithUTF8String:toString(props.role).c_str()] - : [NSString stringWithUTF8String:props.accessibilityRole.c_str()]; + NSString *roleString = (props.role.value_or(Role::None) != Role::None) + ? [NSString stringWithUTF8String:toString(*props.role).c_str()] + : [NSString stringWithUTF8String:props.accessibilityRole.c_str()]; // In iOS, checkbox and radio buttons aren't recognized as traits. However, // because our apps use checkbox and radio buttons often, we should announce diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp index b204a3fd3c58..4bc58608d2a2 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp @@ -18,12 +18,12 @@ namespace facebook::react { // members. This makes accessibilityTraits the single source of truth. // See: github.com/facebook/react-native/issues/57515 static AccessibilityTraits deriveAccessibilityTraits( - Role role, + std::optional role, const std::string& accessibilityRole, const std::optional& accessibilityState) { AccessibilityTraits traits = AccessibilityTraits::None; - if (role != Role::None) { - fromString(toString(role), traits); + if (role.value_or(Role::None) != Role::None) { + fromString(toString(*role), traits); } else if (!accessibilityRole.empty()) { fromString(accessibilityRole, traits); } @@ -179,16 +179,15 @@ AccessibilityProps::AccessibilityProps( auto* accessibilityRoleValue = rawProps.at("accessibilityRole"); auto* roleValue = rawProps.at("role"); - if (accessibilityRoleValue == nullptr || - !accessibilityRoleValue->hasValue()) { + if (accessibilityRoleValue == nullptr) { accessibilityRole = sourceProps.accessibilityRole; - } else { + } else if (accessibilityRoleValue->hasValue()) { fromRawValue(context, *accessibilityRoleValue, accessibilityRole); } - if (roleValue == nullptr || !roleValue->hasValue()) { + if (roleValue == nullptr) { role = sourceProps.role; - } else { + } else if (roleValue->hasValue()) { fromRawValue(context, *roleValue, role); } diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.h b/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.h index 53524bb4a015..0b055d60f9b8 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.h +++ b/packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.h @@ -56,7 +56,7 @@ class AccessibilityProps { bool onAccessibilityEscape{}; bool onAccessibilityAction{}; ImportantForAccessibility importantForAccessibility{ImportantForAccessibility::Auto}; - Role role{Role::None}; + std::optional role{}; std::string testId; #pragma mark - DebugStringConvertible diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/platform/android/react/renderer/components/view/HostPlatformViewProps.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/platform/android/react/renderer/components/view/HostPlatformViewProps.cpp index d2417698a5f6..2f2820422161 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/platform/android/react/renderer/components/view/HostPlatformViewProps.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/platform/android/react/renderer/components/view/HostPlatformViewProps.cpp @@ -395,58 +395,39 @@ inline static void updateNativeDrawableProp( result[propName] = nativeDrawableResult; } -inline static void updateAccessibilityStateProp( - folly::dynamic& result, - const std::optional& newState, - const std::optional& oldState) { - folly::dynamic resultState = folly::dynamic::object(); - - if (!newState.has_value() && oldState.has_value()) { - result["accessibilityState"] = resultState; - return; - } +static folly::dynamic toDynamic(const AccessibilityState& state) { + folly::dynamic result = folly::dynamic::object(); + result["disabled"] = state.disabled; - if (!oldState.has_value() || newState->disabled != oldState->disabled) { - resultState["disabled"] = newState->disabled; + // Omitting the key when `selected` is unset is what tells the platform the + // component is not selectable. BaseViewManager#setViewState falls back to + // `setSelected(false)` for an absent key, so the rendered result is + // unchanged from when this was a plain `bool`. + if (state.selected.has_value()) { + result["selected"] = state.selected.value(); } - if (!oldState.has_value() || newState->selected != oldState->selected) { - // Omitting the key when `selected` is unset is what tells the platform the - // component is not selectable. BaseViewManager#setViewState falls back to - // `setSelected(false)` for an absent key, so the rendered result is - // unchanged from when this was a plain `bool`. - if (newState->selected.has_value()) { - resultState["selected"] = newState->selected.value(); - } - } + result["busy"] = state.busy; - if (!oldState.has_value() || newState->busy != oldState->busy) { - resultState["busy"] = newState->busy; + if (state.expanded.has_value()) { + result["expanded"] = state.expanded.value(); } - if (!oldState.has_value() || newState->expanded != oldState->expanded) { - if (newState->expanded.has_value()) { - resultState["expanded"] = newState->expanded.value(); - } + switch (state.checked) { + case AccessibilityState::Unchecked: + result["checked"] = false; + break; + case AccessibilityState::Checked: + result["checked"] = true; + break; + case AccessibilityState::Mixed: + result["checked"] = "mixed"; + break; + case AccessibilityState::None: + result["checked"] = "none"; + break; } - - if (!oldState.has_value() || newState->checked != oldState->checked) { - switch (newState->checked) { - case AccessibilityState::Unchecked: - resultState["checked"] = false; - break; - case AccessibilityState::Checked: - resultState["checked"] = true; - break; - case AccessibilityState::Mixed: - resultState["checked"] = "mixed"; - break; - case AccessibilityState::None: - resultState["checked"] = "none"; - break; - } - } - result["accessibilityState"] = resultState; + return result; } static folly::dynamic toDynamic( @@ -540,7 +521,8 @@ folly::dynamic HostPlatformViewProps::getDiffProps( } if (role != oldProps->role) { - result["role"] = toString(role); + result["role"] = + role.has_value() ? toString(*role) : folly::dynamic(nullptr); } if (opacity != oldProps->opacity) { @@ -907,8 +889,9 @@ folly::dynamic HostPlatformViewProps::getDiffProps( // Accessibility if (accessibilityState != oldProps->accessibilityState) { - updateAccessibilityStateProp( - result, accessibilityState, oldProps->accessibilityState); + result["accessibilityState"] = accessibilityState.has_value() + ? toDynamic(*accessibilityState) + : folly::dynamic::object(); } if (accessibilityLabel != oldProps->accessibilityLabel) { @@ -941,7 +924,9 @@ folly::dynamic HostPlatformViewProps::getDiffProps( } if (accessibilityRole != oldProps->accessibilityRole) { - result["accessibilityRole"] = accessibilityRole; + result["accessibilityRole"] = accessibilityRole.empty() + ? folly::dynamic(nullptr) + : folly::dynamic(accessibilityRole); } if (accessibilityLanguage != oldProps->accessibilityLanguage) { @@ -962,7 +947,9 @@ folly::dynamic HostPlatformViewProps::getDiffProps( if (accessibilityValue.text.has_value()) { accessibilityValueObject["text"] = accessibilityValue.text.value(); } - result["accessibilityValue"] = accessibilityValueObject; + result["accessibilityValue"] = accessibilityValueObject.empty() + ? folly::dynamic(nullptr) + : accessibilityValueObject; } if (accessibilityActions != oldProps->accessibilityActions) { diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewPropsDiffTest.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewPropsDiffTest.cpp index 0c328359c855..7fe5a6dfdab3 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewPropsDiffTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewPropsDiffTest.cpp @@ -103,6 +103,55 @@ TEST(ViewPropsDiffTest, gradientColorHintEmitsNullColor) { "position", nullptr)))))}}); } +// BaseViewManager.setViewState replaces the whole state: an absent `selected` +// deselects the view and an absent `disabled` re-enables it. Props 2.0 sends an +// unset `checked` as "none", which Java ignores. +TEST(ViewPropsDiffTest, emitsTheWholeAccessibilityStateWhenOneMemberChanges) { + auto state = [](bool busy) { + return dynamic::object("selected", true)("disabled", true)("busy", busy); + }; + auto sentState = [&](bool busy) { + return dynamic::object( + "accessibilityState", state(busy)("checked", "none")); + }; + expectPayloads({ + {dynamic::object("accessibilityState", state(false)), sentState(false)}, + {dynamic::object("accessibilityState", state(true)), sentState(true)}, + }); +} + +// React sends a removed prop as null on top of the previous props. +// BaseViewManager clears the role only for null, and +// AccessibilityRole.fromValue("") throws. +TEST(ViewPropsDiffTest, removingAccessibilityRoleEmitsNull) { + expectPayloads({ + {dynamic::object("accessibilityRole", "button")}, + {dynamic::object("accessibilityRole", nullptr)}, + }); +} + +// BaseViewManager falls back to accessibilityRole only for a null role. +TEST(ViewPropsDiffTest, removingRoleEmitsNull) { + expectPayloads({ + {dynamic::object("role", "button")}, + {dynamic::object("role", nullptr)}, + }); +} + +// An explicit role="none" overrides accessibilityRole. +TEST(ViewPropsDiffTest, emitsAnExplicitRoleNone) { + expectPayloads( + {{dynamic::object("role", "none")("accessibilityRole", "button")}}); +} + +// BaseViewManager clears the value and the content description only for null. +TEST(ViewPropsDiffTest, removingAccessibilityValueEmitsNull) { + expectPayloads({ + {dynamic::object("accessibilityValue", dynamic::object("text", "50%"))}, + {dynamic::object("accessibilityValue", nullptr)}, + }); +} + } // namespace facebook::react #endif diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api index 6d9ac1caa787..862d04da9832 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api @@ -700,8 +700,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api index d2b41ea569c8..4e2950227c6a 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api @@ -1016,8 +1016,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api index 57f7bc098311..97365fcd71a0 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api @@ -700,8 +700,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage; diff --git a/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api index 57f7bc098311..97365fcd71a0 100644 --- a/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api @@ -700,8 +700,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api index 863038207ad0..3852766f9a17 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api @@ -3435,8 +3435,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api index dece37c06a37..0f3c0bc04cc8 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api @@ -3794,8 +3794,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api index 3e9a7b4744fc..a1f8d4990257 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api @@ -3427,8 +3427,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage; diff --git a/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api index a692b6489f17..d3bfac82c603 100644 --- a/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api @@ -3435,8 +3435,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api index cd608ca12bc3..20dd41f82953 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonDebugCxx.api @@ -321,8 +321,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api index c06d3fc44d2a..3b5c31a52434 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api @@ -570,8 +570,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api index 1da6bfffd246..1d69ab6fc014 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonNewarchCxx.api @@ -321,8 +321,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage; diff --git a/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api b/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api index 1da6bfffd246..1d69ab6fc014 100644 --- a/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api +++ b/scripts/cxx-api/api-snapshots/ReactCommonReleaseCxx.api @@ -321,8 +321,8 @@ class facebook::react::AccessibilityProps { public facebook::react::AccessibilityTraits accessibilityTraits; public facebook::react::AccessibilityValue accessibilityValue; public facebook::react::ImportantForAccessibility importantForAccessibility; - public facebook::react::Role role; public std::optional accessibilityState; + public std::optional role; public std::string accessibilityHint; public std::string accessibilityLabel; public std::string accessibilityLanguage;