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

Android: Role description is announced before the components text, rather than after #31042

Closed
blavalla opened this issue Feb 24, 2021 · 5 comments

Comments

@blavalla
Copy link
Contributor

blavalla commented Feb 24, 2021

Description

For some elements, when you add an accessibilityRole to them, screen readers are announcing that role at the beginning of its announcement (e.g. "Button, Like") rather than at the end (e.g. "Like, Button").

The reason for this is a complex, and requires some knowledge about how screen readers like Talkback work. I'll sum up those details below under "Android Details", but be warned, it'll be a long (but hopefully interesting) read!

In the core component library, this mostly impacts which automatically sets an accessibilityRole of "button".

React Native version:

v0.63

Expected Behavior

When focus is put on an element that has an accessibilityRole set, the role should be at the end of the announcement, rather than the beginning. The only things that should come after the role are "disabled" or usage hints such as "Double-tap to activate".

Android Details

The cause of this issue is related to Talkback's rules on what elements will be focusable, and which will not, as well as how it composes together an elements text and its role.

Talkback's focusability rules are not well documented, and require reading the source code to fully understand, but the one causing the issue here is one I'll refer to as automatic content grouping.

Automatic content grouping basically work like this. Talkback will walk the entire view hierarchy and parse content out of the AccessibilityNodeInfo's associated with the views. It then applies the following rules:

1.) If an AccessibilityNodeInfo is considered "actionable" (which Talkback defines as having clickable=true, longClickable=true, or focusable=true, or having AccessibilityActions for any of those), AND it has some content to read like a contentDescription or text, it will be considered focusable.
2.) If an AccessibilityNodeInfo is considered "actionable" AND it does not have content to read like a contentDescription or text Talkback will parse descendant elements looking for non-focusable descendants to use as content.

Item 2 is what is important here. If Talkback were to encounter an element like this:

<View clickable="true" contentDescription="Some Text">
   <Text>Some Other Text</Text>
</View>

It would deem that the should be focusable, due to rule #1 (it has clickable="true") and it has a contentDescription. So on focus, Talkback will read "Some Text", and "Some Other Text" will be ignored, and never announced.

But what if it didn't have a content description? For example:

<View clickable="true">
   <Text>Some Other Text</Text>
</View>

Talkback would parse and still deem it as focusable, due to clickable=true, and then go looking for content to announce. It would see the node, which by itself is not focusable, so it will use this elements text as its own.

On focus of talkback will then pull the content and read read "Some Other Text", with the focus on , and not on like you may expect.

This becomes particularly interesting when an element has multiple non-focusable descendants, for example:

<View clickable="true">
   <Text>Some Text</Text>
   <Text>Some More Text</Text>
</View>

When Talkback parses this tree, it will again determine that is focusable, and that both elements are not. And when focus is placed on it will announce "Some Text, Some More Text", grouping the text of all non-focusable descendants together.

Okay, so now you understand how automatic content grouping works. Now let's talk about how roles work.

When Talkback is walking the tree, and encounters an element with an accessibility role defined (which is really just the className property of the AccessibilityNodeInfo), it will look at the role, and add it to the very end of its focus announcement. Lets look at an example:

<View contentDescription="Some Text" accessibilityRole="button" clickable="true" />

When talkback looks at this element, it will determine that is is focusable due to rule #1, see that it has no descendants, but has a contentDescription set, and also see that it has a role set. So it will announce "Some Text, Button" appending the role text to the end of the announcement.

Now what about if it sees this view again, but with a role:

<View clickable="true" accessibilityRole="button">
   <Text>Some Text</Text>
</View>

Again, it will determine that it is focusable, see that it has no content description of its own, but does has non-focusable descendants with text, and see that it has a role. However, it now does things in the wrong order. It will first append the role text to the empty announcement, and then it will parse the descendants and append their text, ending up with an announcement of "Button, Some Text". This is the root of our bug.

Now that we can see the cause, the "fix" is fairly straightforward. If we had that same View hierarchy, but simply set an identical contentDescription on it, we avoid the problem altogether:

<View clickable="true" accessibilityRole="button" contentDescription="Some Text">
   <Text>Some Text</Text>
</View>

This doesn't fix the root cause, which is in Talkback's logic itself, but it works around the issue in a way that will not present the bug to the user.

In React Native, this issue happens most commonly on <Button>, which takes a title prop and then itself renders out something like this:

<Touchable>
   <Text>{title}</Text>
</Touchable>

You can see how this looks very similar to the examples above, with being considered a focusable element, and the within it containing the actual content. A fix would look something like this:

<Touchable accessibilityLabel={title}>
   <Text>{title}</Text>
</Touchable>

@blavalla
Copy link
Contributor Author

@kacieb, @lunaleaps, @nadiia, this mostly impacts in that it has a required "title" prop that does this automatically, but it could also impact the various components as well, if you set the accessibilityRole on the itself, and not on the content. I'm not sure if those are worth worrying about, as they can take arbitrary content, and parsing out all of the text from the entire tree that is inside them would be pretty difficult to do in a performant manner. Thoughts?

@kacieb
Copy link
Contributor

kacieb commented Feb 24, 2021

It sounds like we could fix this on specific components by setting an accessibilityLabel in JS on the outermost View/Touchable/thing that Android deems focusable (like you mentioned with Button). I think we should fix this anywhere possible.

As for the bigger bug, it sounds like there are two issues here. One is that some Text content could end up being entirely ignored if someone sets a mismatching accessibilityLabel (aka contentDescription on Android) on the outer focusable node:

If Talkback were to encounter an element like this:

<View clickable="true" contentDescription="Some Text">
  <Text>Some Other Text</Text>
</View>

It would deem that the should be focusable, due to rule #1 (it has clickable="true") and it has a contentDescription. So on focus, Talkback will read "Some Text", and "Some Other Text" will be ignored, and never announced.

and the other is that if you don't set an accessibilityLabel, the role and content will be read in the wrong order:

It will first append the role text to the empty announcement, and then it will parse the descendants and append their text, ending up with an announcement of "Button, Some Text". This is the root of our bug.

It doesn't sound ideal to rely on product engineers to correctly set the accessibilityLabel to match the content (if we end up needing to rely on this, we could probably at least write a lint rule for it), if there is a way to fix it natively so that it automatically works. You mentioned TalkBack does parsing to determine what needs to be read. Is there any way to get access to what is parsed so that we can make the "Button" role be read last? I agree that parsing it ourselves could be difficult.

@blavalla
Copy link
Contributor Author

@kacieb Having the contentDescription override the text isn't really a bug, as that is the expected behavior of that property on Android, and accessibilityLabel on iOS works the same way. The purpose of these properties is to be used in order to provide a clearer label than the text alone would provide. Maybe thats by adding additional context that is added by the position on screen or placement next to other elements for sighted users, or maybe it's just to make it more clear in a way we couldn't visually because there wasn't enough space.

But the 2nd issue, the ordering problem, is a straight-up bug in Talkback (that I have raised with that team a number of years back), but hasn't been fixed, so I don't expect that to change any time soon. By adding a contentDescription to work around this bug, even if/when the Talkback team does fix the issue, it won't cause any problems, as we would still be using the system as intended by setting a contentDescription on the focusable element.

As for if we can get access to the order that Talkback parses things, unfortunately not. While we can see their logic by looking at the source (let me know if you want any code pointers here), it's basically a one-way street. We can send talkback information in the form of AccessibilityEvents and by modifying the properties on the View and AccessibilityNodeInfo, but we can't change how it reacts to that information.

By adding a contentDescription we are changing the information we send Talkback, and basically triggering "logic path A" instead of "logic path B" in their code, which works around this issue by avoiding it entirely.

@fabOnReact
Copy link
Contributor

@blavalla I will work on this from the 23rd of March. Thanks

@fabOnReact
Copy link
Contributor

PR #33690 fixes this issue

@fabOnReact fabOnReact assigned fabOnReact and unassigned fabOnReact Aug 25, 2022
facebook-github-bot pushed a commit that referenced this issue Dec 2, 2022
…ustom contentDescription (#33690)

Summary:
The Implementation of the functionality consists of:

1) Checking that an element has no contentDescription and no text and has other content to announce (role, state, etc.) which causes this issue (for ex. the accessibilityRole is announced before the contentDescription for ex. "Button, My text children component")
2) If Talkback finds no content to announce on the current node, a custom contentDescription is generated from the child elements that respect the following conditions:

>If an AccessibilityNodeInfo is considered "actionable" (which Talkback defines as having clickable=true, longClickable=true, or focusable=true, or having AccessibilityActions for any of those), AND it has some content to read like a contentDescription or text, it will be considered focusable.
>If an AccessibilityNodeInfo is considered "actionable" AND it does not have content to read like a contentDescription or text Talkback will parse descendant elements looking for non-focusable descendants to use as content.

3) implementation of a method getTalkbackDescription to generate the above contentDescription from child elements
4) over-ride parent contentDescription (accessibilityLabel) with the value returned from getTalkbackDescription

Related [notes on Android: Role description is announced before the components text, rather than after https://github.com/facebook/react-native/issues/31042][51]. This issue fixes [https://github.com/facebook/react-native/issues/31042][50].

## Changelog

[Android] [Added] - Override default Talkback automatic content grouping and generate a custom contentDescription

Pull Request resolved: #33690

Test Plan:
**PR Branch**
[1]. Screenreader correctly announcing accessible/non-accessible items ([link][1])
[2]. Screenreader announces Pressability items ([link][2])
[3]. Button role is announced after child contentDescription with TouchableNativeFeedback ([link][3])
[4]. Testing for regressions in Accessibility Actions ([link][4])
[5]. Screenreader focuses on ScrollView Items ([link][5])
[6]. Recordings of complete test cases in rn-tester app main and pr branch ([link][6])
[9]. TouchableOpacity with TextInput child announces contentDescription before the Role ([link][9])
[10]. One of the child has accessibilityState (hasStateDescription triggers the announcement) ([link][10])
[11]. One of the child has accessibilityHint (hasText triggers the announcement) ([link][11])

**Main**
[15]. The View does not announce the child component Text when accessibilityLabel is missing (automatic content grouping) ([link][15])
[8]. TouchableOpacity with child EditText annouces placeholder text before and after the role ([link][8])

[1]: fabOnReact/react-native-notes#14 (comment) "Screenreader correctly announcing accessible/non-accessible items"
[2]: fabOnReact/react-native-notes#14 (comment) "Screenreader announces Pressability items"
[3]: fabOnReact/react-native-notes#14 (comment) "Button role is announced after child contentDescription"
[4]: fabOnReact/react-native-notes#14 (comment) "Testing for regressions in Accessibility Actions"
[5]: fabOnReact/react-native-notes#14 (comment) "Screenreader focuses on ScrollView Items"
[6]: fabOnReact/react-native-notes#14 (comment) "Recordings of complete test cases in rn-tester app main and pr branch"
[7]: fabOnReact/react-native-notes#14 (comment) "TouchableOpacity with child EditText annouces Button role before the child contentDescription"
[8]: fabOnReact/react-native-notes#14 (comment) "TouchableOpacity with child EditText annouces placholder text before and after the role"
[9]: fabOnReact/react-native-notes#14 (comment) "TouchableOpacity with TextInput child announces contentDescription before the Role"
[10]: fabOnReact/react-native-notes#14 (comment) "One of the child has accessibilityState (hasStateDescription triggers the announcement)"
[11]: fabOnReact/react-native-notes#14 (comment) "One of the child has accessibilityHint (hasText triggers the announcement)"

[15]: fabOnReact/react-native-notes#14 (comment) "The View does not announce the child component Text when accessibilityLabel is missing (automatic content grouping)"

[50]: #31042 "Android: Role description is announced before the components text, rather than after #31042"
[51]: fabOnReact/react-native-notes#14 "notes on Android: Role description is announced before the components text, rather than after #31042"

Reviewed By: cipolleschi

Differential Revision: D39177512

Pulled By: blavalla

fbshipit-source-id: 6bd0fba9c347bc14b3839e903184c86d2bcab3d2
@facebook facebook deleted a comment Jan 11, 2023
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this issue May 22, 2023
…ustom contentDescription (facebook#33690)

Summary:
The Implementation of the functionality consists of:

1) Checking that an element has no contentDescription and no text and has other content to announce (role, state, etc.) which causes this issue (for ex. the accessibilityRole is announced before the contentDescription for ex. "Button, My text children component")
2) If Talkback finds no content to announce on the current node, a custom contentDescription is generated from the child elements that respect the following conditions:

>If an AccessibilityNodeInfo is considered "actionable" (which Talkback defines as having clickable=true, longClickable=true, or focusable=true, or having AccessibilityActions for any of those), AND it has some content to read like a contentDescription or text, it will be considered focusable.
>If an AccessibilityNodeInfo is considered "actionable" AND it does not have content to read like a contentDescription or text Talkback will parse descendant elements looking for non-focusable descendants to use as content.

3) implementation of a method getTalkbackDescription to generate the above contentDescription from child elements
4) over-ride parent contentDescription (accessibilityLabel) with the value returned from getTalkbackDescription

Related [notes on Android: Role description is announced before the components text, rather than after https://github.com/facebook/react-native/issues/31042][51]. This issue fixes [https://github.com/facebook/react-native/issues/31042][50].

## Changelog

[Android] [Added] - Override default Talkback automatic content grouping and generate a custom contentDescription

Pull Request resolved: facebook#33690

Test Plan:
**PR Branch**
[1]. Screenreader correctly announcing accessible/non-accessible items ([link][1])
[2]. Screenreader announces Pressability items ([link][2])
[3]. Button role is announced after child contentDescription with TouchableNativeFeedback ([link][3])
[4]. Testing for regressions in Accessibility Actions ([link][4])
[5]. Screenreader focuses on ScrollView Items ([link][5])
[6]. Recordings of complete test cases in rn-tester app main and pr branch ([link][6])
[9]. TouchableOpacity with TextInput child announces contentDescription before the Role ([link][9])
[10]. One of the child has accessibilityState (hasStateDescription triggers the announcement) ([link][10])
[11]. One of the child has accessibilityHint (hasText triggers the announcement) ([link][11])

**Main**
[15]. The View does not announce the child component Text when accessibilityLabel is missing (automatic content grouping) ([link][15])
[8]. TouchableOpacity with child EditText annouces placeholder text before and after the role ([link][8])

[1]: fabOnReact/react-native-notes#14 (comment) "Screenreader correctly announcing accessible/non-accessible items"
[2]: fabOnReact/react-native-notes#14 (comment) "Screenreader announces Pressability items"
[3]: fabOnReact/react-native-notes#14 (comment) "Button role is announced after child contentDescription"
[4]: fabOnReact/react-native-notes#14 (comment) "Testing for regressions in Accessibility Actions"
[5]: fabOnReact/react-native-notes#14 (comment) "Screenreader focuses on ScrollView Items"
[6]: fabOnReact/react-native-notes#14 (comment) "Recordings of complete test cases in rn-tester app main and pr branch"
[7]: fabOnReact/react-native-notes#14 (comment) "TouchableOpacity with child EditText annouces Button role before the child contentDescription"
[8]: fabOnReact/react-native-notes#14 (comment) "TouchableOpacity with child EditText annouces placholder text before and after the role"
[9]: fabOnReact/react-native-notes#14 (comment) "TouchableOpacity with TextInput child announces contentDescription before the Role"
[10]: fabOnReact/react-native-notes#14 (comment) "One of the child has accessibilityState (hasStateDescription triggers the announcement)"
[11]: fabOnReact/react-native-notes#14 (comment) "One of the child has accessibilityHint (hasText triggers the announcement)"

[15]: fabOnReact/react-native-notes#14 (comment) "The View does not announce the child component Text when accessibilityLabel is missing (automatic content grouping)"

[50]: facebook#31042 "Android: Role description is announced before the components text, rather than after facebook#31042"
[51]: fabOnReact/react-native-notes#14 "notes on Android: Role description is announced before the components text, rather than after facebook#31042"

Reviewed By: cipolleschi

Differential Revision: D39177512

Pulled By: blavalla

fbshipit-source-id: 6bd0fba9c347bc14b3839e903184c86d2bcab3d2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants