Bug 1601537 Comment 2 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

I've finally managed to figure out some of what's going on here and it's not fun. I even had to dig into Talkback logging and source code to work it out.

The crux of the issue is that even though text navigation is async (you perform an action on the focused accessible and then wait for a text traversal event), firing a traversal event with a different accessible from the focused accessible is not supported by Talkback. This bites us in two ways:

1. For paragraphs, divs, spans, etc., a11y focus goes to text leaf nodes. However, when you navigate text, we fire an a11y focus event then a text traversal event on the HyperTextAccessible parent (e.g. paragraph).
    -Firing a11y focus causes the entire HyperTextAccessible to be reported.
    - If we don't fire a11y focus, Talkback won't update the origin it navigates from. That is, it will just start from the last a11y focus, which will result in incorrect offsets. Unfortunately, Talkback doesn't update the origin on text traversal events.
    - Even if Talkback did update the origin, that breaks focus for the user if the HyperTextAccessible has multiple text nodes. For example, if you have a paragraph containing a link followed by some text and the user is focused on the text, navigating the text would throw focus onto the paragraph, at which point flicking right would take them to the link.
    - Firing a text traversal event on a HyperTextAccessible (e.g. paragraph) without a label seems to cause the text traversal event to be treated as having empty text:
        > ```
        > 12-10 16:53:17.755  2556  2556 D ProcessorEventQueue: Processing event: EventType: TYPE_VIEW_TEXT_TRAVERSED_AT_MOVEMENT_GRANULARITY; EventTime: 636765839; PackageName: org.mozilla.geckoview_example; MovementGranularity: 0; Action: 0 [ ClassName: android.view.View; Text: [First Second]; ContentDescription: ; ItemCount: -1; CurrentItemIndex: -1; IsEnabled: true; IsPassword: false; IsChecked: false; IsFullScreen: false; Scrollable: false; BeforeText: ; FromIndex: 0; ToIndex: 1; ScrollX: -1; ScrollY: -1; MaxScrollX: -1; MaxScrollY: -1; AddedCount: -1; RemovedCount: -1; ParcelableData: null ]; recordCount: 0
        > 12-10 16:53:17.756  2556  2556 I TextEventInterpreter: interpretation: Event=EVENT_TYPE_INPUT_CHANGE_INVALID Reason="Text is empty." 
        > ```
        I still can't figure out why this happens. As you can see from the log, there is indeed text in the event, and the [part of the Talkback code that logs this "Text is empty." message](https://github.com/google/talkback/blob/fa67c12f/compositor/src/main/java/TextEventInterpreter.java#L245) should definitely be getting this text. I've tried setting contentDescription and itemCount which parts of that code also check, but no change. And yet for a heading or a link, this works just fine.
        If I give the paragraph a label, this problem seems to go away. While I couldn't find anything scouring the Talkback code, my guess is that it ends up fetching some data like text from the node instead of the event? This is somewhat separate from the other issues here.
2. Normally, when you navigate to the end of the text in a node, Talkback moves into the next node. Unfortunately, as explained above, Talkback doesn't update the origin based on text traversed events, and we don't want to fire a11y focus because it reports the entire node. The way you have to manage this with Talkback is to return false from performAction when there's no more text to navigate, at which point Talkback [tries navigating in the next node](https://github.com/google/talkback/blob/fa67c12f/talkback/src/main/java/CursorGranularityManager.java#L346).
    - This is problematic for us because we can't perform the navigation synchronously in order to know whether there's more text, since we might have to go cross-process to do that. I guess we could use sync IPC, but ug. Or maybe we could cache the length of the text in the Java layer and use that to determine whether further navigation is possible.
I've finally managed to figure out some of what's going on here and it's not fun. I even had to dig into Talkback logging and source code to work it out.

The crux of the issue is that even though text navigation is async (you perform an action on the focused accessible and then wait for a text traversal event), firing a traversal event with a different accessible from the focused accessible is not supported by Talkback. This bites us in two ways:

1. For paragraphs, divs, spans, etc., a11y focus goes to text leaf nodes. However, when you navigate text, we fire an a11y focus event then a text traversal event on the HyperTextAccessible parent (e.g. paragraph).
    - Firing a11y focus causes the entire HyperTextAccessible to be reported.
    - If we don't fire a11y focus, Talkback won't update the origin it navigates from. That is, it will just start from the last a11y focus, which will result in incorrect offsets. Unfortunately, Talkback doesn't update the origin on text traversal events.
    - Even if Talkback did update the origin, that breaks focus for the user if the HyperTextAccessible has multiple text nodes. For example, if you have a paragraph containing a link followed by some text and the user is focused on the text, navigating the text would throw focus onto the paragraph, at which point flicking right would take them to the link.
    - Firing a text traversal event on a HyperTextAccessible (e.g. paragraph) without a label seems to cause the text traversal event to be treated as having empty text:
        > ```
        > 12-10 16:53:17.755  2556  2556 D ProcessorEventQueue: Processing event: EventType: TYPE_VIEW_TEXT_TRAVERSED_AT_MOVEMENT_GRANULARITY; EventTime: 636765839; PackageName: org.mozilla.geckoview_example; MovementGranularity: 0; Action: 0 [ ClassName: android.view.View; Text: [First Second]; ContentDescription: ; ItemCount: -1; CurrentItemIndex: -1; IsEnabled: true; IsPassword: false; IsChecked: false; IsFullScreen: false; Scrollable: false; BeforeText: ; FromIndex: 0; ToIndex: 1; ScrollX: -1; ScrollY: -1; MaxScrollX: -1; MaxScrollY: -1; AddedCount: -1; RemovedCount: -1; ParcelableData: null ]; recordCount: 0
        > 12-10 16:53:17.756  2556  2556 I TextEventInterpreter: interpretation: Event=EVENT_TYPE_INPUT_CHANGE_INVALID Reason="Text is empty." 
        > ```
        I still can't figure out why this happens. As you can see from the log, there is indeed text in the event, and the [part of the Talkback code that logs this "Text is empty." message](https://github.com/google/talkback/blob/fa67c12f/compositor/src/main/java/TextEventInterpreter.java#L245) should definitely be getting this text. I've tried setting contentDescription and itemCount which parts of that code also check, but no change. And yet for a heading or a link, this works just fine.
        If I give the paragraph a label, this problem seems to go away. While I couldn't find anything scouring the Talkback code, my guess is that it ends up fetching some data like text from the node instead of the event? This is somewhat separate from the other issues here.
2. Normally, when you navigate to the end of the text in a node, Talkback moves into the next node. Unfortunately, as explained above, Talkback doesn't update the origin based on text traversed events, and we don't want to fire a11y focus because it reports the entire node. The way you have to manage this with Talkback is to return false from performAction when there's no more text to navigate, at which point Talkback [tries navigating in the next node](https://github.com/google/talkback/blob/fa67c12f/talkback/src/main/java/CursorGranularityManager.java#L346).
    - This is problematic for us because we can't perform the navigation synchronously in order to know whether there's more text, since we might have to go cross-process to do that. I guess we could use sync IPC, but ug. Or maybe we could cache the length of the text in the Java layer and use that to determine whether further navigation is possible.

Back to Bug 1601537 Comment 2