Skip to content

Commit 074e081

Browse files
Reland: [Android] Fix View accessibility payloads that Props 2.0 sends wrong or drops (#59387)
Summary: Reland of #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
1 parent 1d1fade commit 074e081

17 files changed

Lines changed: 111 additions & 75 deletions

‎packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ static BOOL RCTViewIsInteractiveAccessibilityElement(UIView *view, const ViewPro
6767

6868
// `role` wins over the legacy `accessibilityRole` when both are set, matching
6969
// how the traits themselves are resolved.
70-
if (props.role != Role::None) {
70+
if (props.role.value_or(Role::None) != Role::None) {
7171
static const std::unordered_set<Role> interactiveRoles{
7272
Role::Button,
7373
Role::Checkbox,
@@ -82,7 +82,7 @@ static BOOL RCTViewIsInteractiveAccessibilityElement(UIView *view, const ViewPro
8282
Role::Switch,
8383
Role::Tab,
8484
Role::Treeitem};
85-
return interactiveRoles.contains(props.role);
85+
return interactiveRoles.contains(*props.role);
8686
}
8787

8888
static const std::unordered_set<std::string_view> interactiveAccessibilityRoles{
@@ -1690,8 +1690,9 @@ - (NSString *)accessibilityValue
16901690
}
16911691

16921692
NSMutableArray *valueComponents = [NSMutableArray new];
1693-
NSString *roleString = (props.role != Role::None) ? [NSString stringWithUTF8String:toString(props.role).c_str()]
1694-
: [NSString stringWithUTF8String:props.accessibilityRole.c_str()];
1693+
NSString *roleString = (props.role.value_or(Role::None) != Role::None)
1694+
? [NSString stringWithUTF8String:toString(*props.role).c_str()]
1695+
: [NSString stringWithUTF8String:props.accessibilityRole.c_str()];
16951696

16961697
// In iOS, checkbox and radio buttons aren't recognized as traits. However,
16971698
// because our apps use checkbox and radio buttons often, we should announce

‎packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ namespace facebook::react {
1818
// members. This makes accessibilityTraits the single source of truth.
1919
// See: github.com/facebook/react-native/issues/57515
2020
static AccessibilityTraits deriveAccessibilityTraits(
21-
Role role,
21+
std::optional<Role> role,
2222
const std::string& accessibilityRole,
2323
const std::optional<AccessibilityState>& accessibilityState) {
2424
AccessibilityTraits traits = AccessibilityTraits::None;
25-
if (role != Role::None) {
26-
fromString(toString(role), traits);
25+
if (role.value_or(Role::None) != Role::None) {
26+
fromString(toString(*role), traits);
2727
} else if (!accessibilityRole.empty()) {
2828
fromString(accessibilityRole, traits);
2929
}
@@ -179,16 +179,15 @@ AccessibilityProps::AccessibilityProps(
179179
auto* accessibilityRoleValue = rawProps.at("accessibilityRole");
180180
auto* roleValue = rawProps.at("role");
181181

182-
if (accessibilityRoleValue == nullptr ||
183-
!accessibilityRoleValue->hasValue()) {
182+
if (accessibilityRoleValue == nullptr) {
184183
accessibilityRole = sourceProps.accessibilityRole;
185-
} else {
184+
} else if (accessibilityRoleValue->hasValue()) {
186185
fromRawValue(context, *accessibilityRoleValue, accessibilityRole);
187186
}
188187

189-
if (roleValue == nullptr || !roleValue->hasValue()) {
188+
if (roleValue == nullptr) {
190189
role = sourceProps.role;
191-
} else {
190+
} else if (roleValue->hasValue()) {
192191
fromRawValue(context, *roleValue, role);
193192
}
194193

‎packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class AccessibilityProps {
5656
bool onAccessibilityEscape{};
5757
bool onAccessibilityAction{};
5858
ImportantForAccessibility importantForAccessibility{ImportantForAccessibility::Auto};
59-
Role role{Role::None};
59+
std::optional<Role> role{};
6060
std::string testId;
6161

6262
#pragma mark - DebugStringConvertible

‎packages/react-native/ReactCommon/react/renderer/components/view/platform/android/react/renderer/components/view/HostPlatformViewProps.cpp‎

Lines changed: 37 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -395,58 +395,39 @@ inline static void updateNativeDrawableProp(
395395
result[propName] = nativeDrawableResult;
396396
}
397397

398-
inline static void updateAccessibilityStateProp(
399-
folly::dynamic& result,
400-
const std::optional<AccessibilityState>& newState,
401-
const std::optional<AccessibilityState>& oldState) {
402-
folly::dynamic resultState = folly::dynamic::object();
403-
404-
if (!newState.has_value() && oldState.has_value()) {
405-
result["accessibilityState"] = resultState;
406-
return;
407-
}
398+
static folly::dynamic toDynamic(const AccessibilityState& state) {
399+
folly::dynamic result = folly::dynamic::object();
400+
result["disabled"] = state.disabled;
408401

409-
if (!oldState.has_value() || newState->disabled != oldState->disabled) {
410-
resultState["disabled"] = newState->disabled;
402+
// Omitting the key when `selected` is unset is what tells the platform the
403+
// component is not selectable. BaseViewManager#setViewState falls back to
404+
// `setSelected(false)` for an absent key, so the rendered result is
405+
// unchanged from when this was a plain `bool`.
406+
if (state.selected.has_value()) {
407+
result["selected"] = state.selected.value();
411408
}
412409

413-
if (!oldState.has_value() || newState->selected != oldState->selected) {
414-
// Omitting the key when `selected` is unset is what tells the platform the
415-
// component is not selectable. BaseViewManager#setViewState falls back to
416-
// `setSelected(false)` for an absent key, so the rendered result is
417-
// unchanged from when this was a plain `bool`.
418-
if (newState->selected.has_value()) {
419-
resultState["selected"] = newState->selected.value();
420-
}
421-
}
410+
result["busy"] = state.busy;
422411

423-
if (!oldState.has_value() || newState->busy != oldState->busy) {
424-
resultState["busy"] = newState->busy;
412+
if (state.expanded.has_value()) {
413+
result["expanded"] = state.expanded.value();
425414
}
426415

427-
if (!oldState.has_value() || newState->expanded != oldState->expanded) {
428-
if (newState->expanded.has_value()) {
429-
resultState["expanded"] = newState->expanded.value();
430-
}
416+
switch (state.checked) {
417+
case AccessibilityState::Unchecked:
418+
result["checked"] = false;
419+
break;
420+
case AccessibilityState::Checked:
421+
result["checked"] = true;
422+
break;
423+
case AccessibilityState::Mixed:
424+
result["checked"] = "mixed";
425+
break;
426+
case AccessibilityState::None:
427+
result["checked"] = "none";
428+
break;
431429
}
432-
433-
if (!oldState.has_value() || newState->checked != oldState->checked) {
434-
switch (newState->checked) {
435-
case AccessibilityState::Unchecked:
436-
resultState["checked"] = false;
437-
break;
438-
case AccessibilityState::Checked:
439-
resultState["checked"] = true;
440-
break;
441-
case AccessibilityState::Mixed:
442-
resultState["checked"] = "mixed";
443-
break;
444-
case AccessibilityState::None:
445-
resultState["checked"] = "none";
446-
break;
447-
}
448-
}
449-
result["accessibilityState"] = resultState;
430+
return result;
450431
}
451432

452433
static folly::dynamic toDynamic(
@@ -540,7 +521,8 @@ folly::dynamic HostPlatformViewProps::getDiffProps(
540521
}
541522

542523
if (role != oldProps->role) {
543-
result["role"] = toString(role);
524+
result["role"] =
525+
role.has_value() ? toString(*role) : folly::dynamic(nullptr);
544526
}
545527

546528
if (opacity != oldProps->opacity) {
@@ -907,8 +889,9 @@ folly::dynamic HostPlatformViewProps::getDiffProps(
907889
// Accessibility
908890

909891
if (accessibilityState != oldProps->accessibilityState) {
910-
updateAccessibilityStateProp(
911-
result, accessibilityState, oldProps->accessibilityState);
892+
result["accessibilityState"] = accessibilityState.has_value()
893+
? toDynamic(*accessibilityState)
894+
: folly::dynamic::object();
912895
}
913896

914897
if (accessibilityLabel != oldProps->accessibilityLabel) {
@@ -941,7 +924,9 @@ folly::dynamic HostPlatformViewProps::getDiffProps(
941924
}
942925

943926
if (accessibilityRole != oldProps->accessibilityRole) {
944-
result["accessibilityRole"] = accessibilityRole;
927+
result["accessibilityRole"] = accessibilityRole.empty()
928+
? folly::dynamic(nullptr)
929+
: folly::dynamic(accessibilityRole);
945930
}
946931

947932
if (accessibilityLanguage != oldProps->accessibilityLanguage) {
@@ -962,7 +947,9 @@ folly::dynamic HostPlatformViewProps::getDiffProps(
962947
if (accessibilityValue.text.has_value()) {
963948
accessibilityValueObject["text"] = accessibilityValue.text.value();
964949
}
965-
result["accessibilityValue"] = accessibilityValueObject;
950+
result["accessibilityValue"] = accessibilityValueObject.empty()
951+
? folly::dynamic(nullptr)
952+
: accessibilityValueObject;
966953
}
967954

968955
if (accessibilityActions != oldProps->accessibilityActions) {

‎packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewPropsDiffTest.cpp‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,55 @@ TEST(ViewPropsDiffTest, gradientColorHintEmitsNullColor) {
103103
"position", nullptr)))))}});
104104
}
105105

106+
// BaseViewManager.setViewState replaces the whole state: an absent `selected`
107+
// deselects the view and an absent `disabled` re-enables it. Props 2.0 sends an
108+
// unset `checked` as "none", which Java ignores.
109+
TEST(ViewPropsDiffTest, emitsTheWholeAccessibilityStateWhenOneMemberChanges) {
110+
auto state = [](bool busy) {
111+
return dynamic::object("selected", true)("disabled", true)("busy", busy);
112+
};
113+
auto sentState = [&](bool busy) {
114+
return dynamic::object(
115+
"accessibilityState", state(busy)("checked", "none"));
116+
};
117+
expectPayloads({
118+
{dynamic::object("accessibilityState", state(false)), sentState(false)},
119+
{dynamic::object("accessibilityState", state(true)), sentState(true)},
120+
});
121+
}
122+
123+
// React sends a removed prop as null on top of the previous props.
124+
// BaseViewManager clears the role only for null, and
125+
// AccessibilityRole.fromValue("") throws.
126+
TEST(ViewPropsDiffTest, removingAccessibilityRoleEmitsNull) {
127+
expectPayloads({
128+
{dynamic::object("accessibilityRole", "button")},
129+
{dynamic::object("accessibilityRole", nullptr)},
130+
});
131+
}
132+
133+
// BaseViewManager falls back to accessibilityRole only for a null role.
134+
TEST(ViewPropsDiffTest, removingRoleEmitsNull) {
135+
expectPayloads({
136+
{dynamic::object("role", "button")},
137+
{dynamic::object("role", nullptr)},
138+
});
139+
}
140+
141+
// An explicit role="none" overrides accessibilityRole.
142+
TEST(ViewPropsDiffTest, emitsAnExplicitRoleNone) {
143+
expectPayloads(
144+
{{dynamic::object("role", "none")("accessibilityRole", "button")}});
145+
}
146+
147+
// BaseViewManager clears the value and the content description only for null.
148+
TEST(ViewPropsDiffTest, removingAccessibilityValueEmitsNull) {
149+
expectPayloads({
150+
{dynamic::object("accessibilityValue", dynamic::object("text", "50%"))},
151+
{dynamic::object("accessibilityValue", nullptr)},
152+
});
153+
}
154+
106155
} // namespace facebook::react
107156

108157
#endif

‎scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -700,8 +700,8 @@ class facebook::react::AccessibilityProps {
700700
public facebook::react::AccessibilityTraits accessibilityTraits;
701701
public facebook::react::AccessibilityValue accessibilityValue;
702702
public facebook::react::ImportantForAccessibility importantForAccessibility;
703-
public facebook::react::Role role;
704703
public std::optional<facebook::react::AccessibilityState> accessibilityState;
704+
public std::optional<facebook::react::Role> role;
705705
public std::string accessibilityHint;
706706
public std::string accessibilityLabel;
707707
public std::string accessibilityLanguage;

‎scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1016,8 +1016,8 @@ class facebook::react::AccessibilityProps {
10161016
public facebook::react::AccessibilityTraits accessibilityTraits;
10171017
public facebook::react::AccessibilityValue accessibilityValue;
10181018
public facebook::react::ImportantForAccessibility importantForAccessibility;
1019-
public facebook::react::Role role;
10201019
public std::optional<facebook::react::AccessibilityState> accessibilityState;
1020+
public std::optional<facebook::react::Role> role;
10211021
public std::string accessibilityHint;
10221022
public std::string accessibilityLabel;
10231023
public std::string accessibilityLanguage;

‎scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -700,8 +700,8 @@ class facebook::react::AccessibilityProps {
700700
public facebook::react::AccessibilityTraits accessibilityTraits;
701701
public facebook::react::AccessibilityValue accessibilityValue;
702702
public facebook::react::ImportantForAccessibility importantForAccessibility;
703-
public facebook::react::Role role;
704703
public std::optional<facebook::react::AccessibilityState> accessibilityState;
704+
public std::optional<facebook::react::Role> role;
705705
public std::string accessibilityHint;
706706
public std::string accessibilityLabel;
707707
public std::string accessibilityLanguage;

‎scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -700,8 +700,8 @@ class facebook::react::AccessibilityProps {
700700
public facebook::react::AccessibilityTraits accessibilityTraits;
701701
public facebook::react::AccessibilityValue accessibilityValue;
702702
public facebook::react::ImportantForAccessibility importantForAccessibility;
703-
public facebook::react::Role role;
704703
public std::optional<facebook::react::AccessibilityState> accessibilityState;
704+
public std::optional<facebook::react::Role> role;
705705
public std::string accessibilityHint;
706706
public std::string accessibilityLabel;
707707
public std::string accessibilityLanguage;

‎scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3430,8 +3430,8 @@ class facebook::react::AccessibilityProps {
34303430
public facebook::react::AccessibilityTraits accessibilityTraits;
34313431
public facebook::react::AccessibilityValue accessibilityValue;
34323432
public facebook::react::ImportantForAccessibility importantForAccessibility;
3433-
public facebook::react::Role role;
34343433
public std::optional<facebook::react::AccessibilityState> accessibilityState;
3434+
public std::optional<facebook::react::Role> role;
34353435
public std::string accessibilityHint;
34363436
public std::string accessibilityLabel;
34373437
public std::string accessibilityLanguage;

0 commit comments

Comments
 (0)