Bug 1604101: Fix Android a11y text navigation between nodes. r=MarcoZ

Talkback users expect that when you navigate past the end of the text in a node, Talkback will move into the next node and navigate there.
However, even though text navigation is async (client performs an action on the focused accessible and then waits for a text traversal event), firing a traversal event with a different accessible from the focused accessible is not supported by Talkback.
Firing a11y focus on the new node (as we did previously) doesn't fix this, but instead causes the entire node to be reported, among other weird behaviour.

1. Don't fire a11y focus for text traversal.
    Aside from Talkback reporting the entire node, this was also confusing Talkback, causing it to try to navigate several times into the new node.
2. When navigating text, cache whether we're at either edge.
    We do this because we need to be able to synchronously query whether we're at the edge, but we do navigation async.
    Special handling is needed for words at the end because words don't include trailing space.
3. When performing a text navigation action, check if we're already at the edge using the cache described above.
    If we are, synchronously return false, as Talkback expects.
    Talkback will then move to the next/previous node itself and navigate the text there.

Differential Revision: https://phabricator.services.mozilla.com/D57926

--HG--
extra : moz-landing-system : lando

accessible/android/AccessibleWrap.cpp

@@ -161,7 +161,7 @@ nsresult AccessibleWrap::HandleAccEvent(AccEvent* aEvent) {
       if (sessionAcc && newPosition) {
         if (vcEvent->Reason() == nsIAccessiblePivot::REASON_POINT) {
           sessionAcc->SendHoverEnterEvent(newPosition);
-        } else {
+        } else if (vcEvent->BoundaryType() == nsIAccessiblePivot::NO_BOUNDARY) {
           sessionAcc->SendAccessibilityFocusedEvent(newPosition);
         }
 

accessible/android/Platform.cpp

@@ -132,7 +132,7 @@ void a11y::ProxyVirtualCursorChangeEvent(
 
   if (aReason == nsIAccessiblePivot::REASON_POINT) {
     sessionAcc->SendHoverEnterEvent(WrapperFor(aNewPosition));
-  } else {
+  } else if (aBoundaryType == nsIAccessiblePivot::NO_BOUNDARY) {
     RefPtr<AccessibleWrap> wrapperForNewPosition = WrapperFor(aNewPosition);
     sessionAcc->SendAccessibilityFocusedEvent(wrapperForNewPosition);
   }

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/AccessibilityTest.kt

@@ -529,6 +529,147 @@ class ZZAccessibilityTest : BaseSessionTest() {
         waitUntilTextTraversed(0, 18, nodeId) // "Lorem ipsum dolor "
     }
 
+    @Test fun testMoveByCharacterAtEdges() {
+        var nodeId = AccessibilityNodeProvider.HOST_VIEW_ID
+        sessionRule.session.loadTestPath(LOREM_IPSUM_HTML_PATH)
+        waitForInitialFocus()
+
+        // Move to the first link containing "anim id".
+        val bundle = Bundle()
+        bundle.putString(AccessibilityNodeInfo.ACTION_ARGUMENT_HTML_ELEMENT_STRING, "LINK")
+        provider.performAction(nodeId, AccessibilityNodeInfo.ACTION_NEXT_HTML_ELEMENT, bundle)
+        sessionRule.waitUntilCalled(object : EventDelegate {
+            @AssertCalled(count = 1)
+            override fun onAccessibilityFocused(event: AccessibilityEvent) {
+                nodeId = getSourceId(event)
+                val node = createNodeInfo(nodeId)
+                assertThat("Accessibility focus on link", node.text as String, startsWith("anim id"))
+            }
+        })
+
+        var success: Boolean
+        // Navigate forward through "anim id" character by character.
+        for (start in 0..6) {
+            success = provider.performAction(nodeId,
+                    AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY,
+                    moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_CHARACTER))
+            assertThat("Next char should succeed", success, equalTo(true))
+            waitUntilTextTraversed(start, start + 1, nodeId)
+        }
+
+        // Try to navigate forward past end.
+        success = provider.performAction(nodeId,
+                AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY,
+                moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_CHARACTER))
+        assertThat("Next char should fail at end", success, equalTo(false))
+
+        // We're already on "d". Navigate backward through "anim i".
+        for (start in 5 downTo 0) {
+            success = provider.performAction(nodeId,
+                    AccessibilityNodeInfo.ACTION_PREVIOUS_AT_MOVEMENT_GRANULARITY,
+                    moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_CHARACTER))
+            assertThat("Prev char should succeed", success, equalTo(true))
+            waitUntilTextTraversed(start, start + 1, nodeId)
+        }
+
+        // Try to navigate backward past start.
+        success = provider.performAction(nodeId,
+                AccessibilityNodeInfo.ACTION_PREVIOUS_AT_MOVEMENT_GRANULARITY,
+                moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_CHARACTER))
+        assertThat("Prev char should fail at start", success, equalTo(false))
+    }
+
+    @Test fun testMoveByWordAtEdges() {
+        var nodeId = AccessibilityNodeProvider.HOST_VIEW_ID
+        sessionRule.session.loadTestPath(LOREM_IPSUM_HTML_PATH)
+        waitForInitialFocus()
+
+        // Move to the first link containing "anim id".
+        val bundle = Bundle()
+        bundle.putString(AccessibilityNodeInfo.ACTION_ARGUMENT_HTML_ELEMENT_STRING, "LINK")
+        provider.performAction(nodeId, AccessibilityNodeInfo.ACTION_NEXT_HTML_ELEMENT, bundle)
+        sessionRule.waitUntilCalled(object : EventDelegate {
+            @AssertCalled(count = 1)
+            override fun onAccessibilityFocused(event: AccessibilityEvent) {
+                nodeId = getSourceId(event)
+                val node = createNodeInfo(nodeId)
+                assertThat("Accessibility focus on link", node.text as String, startsWith("anim id"))
+            }
+        })
+
+        var success: Boolean
+        success = provider.performAction(nodeId,
+                AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY,
+                moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_WORD))
+        assertThat("Next word should succeed", success, equalTo(true))
+        waitUntilTextTraversed(0, 4, nodeId) // "anim"
+
+        success = provider.performAction(nodeId,
+                AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY,
+                moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_WORD))
+        assertThat("Next word should succeed", success, equalTo(true))
+        waitUntilTextTraversed(5, 7, nodeId) // "id"
+
+        // Try to navigate forward past end.
+        success = provider.performAction(nodeId,
+                AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY,
+                moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_WORD))
+        assertThat("Next word should fail at end", success, equalTo(false))
+
+        success = provider.performAction(nodeId,
+                AccessibilityNodeInfo.ACTION_PREVIOUS_AT_MOVEMENT_GRANULARITY,
+                moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_WORD))
+        assertThat("Prev word should succeed", success, equalTo(true))
+        waitUntilTextTraversed(0, 4, nodeId) // "anim"
+
+        // Try to navigate backward past start.
+        success = provider.performAction(nodeId,
+                AccessibilityNodeInfo.ACTION_PREVIOUS_AT_MOVEMENT_GRANULARITY,
+                moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_WORD))
+        assertThat("Prev word should fail at start", success, equalTo(false))
+    }
+
+    @Test fun testMoveAtEndOfTextTrailingWhitespace() {
+        var nodeId = AccessibilityNodeProvider.HOST_VIEW_ID
+        sessionRule.session.loadTestPath(LOREM_IPSUM_HTML_PATH)
+        waitForInitialFocus(true)
+
+        sessionRule.waitUntilCalled(object : EventDelegate {
+            @AssertCalled(count = 1)
+            override fun onAccessibilityFocused(event: AccessibilityEvent) {
+                nodeId = getSourceId(event)
+                val node = createNodeInfo(nodeId)
+                assertThat("Accessibility focus on first text leaf", node.text as String, startsWith("Lorem ipsum"))
+            }
+        })
+
+        // Initial move backward to move to last word.
+        var success = provider.performAction(nodeId,
+                AccessibilityNodeInfo.ACTION_PREVIOUS_AT_MOVEMENT_GRANULARITY,
+                moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_WORD))
+        assertThat("Prev word should succeed", success, equalTo(true))
+        waitUntilTextTraversed(418, 424, nodeId) // "mollit"
+
+        // Try to move forward past last word.
+        success = provider.performAction(nodeId,
+                AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY,
+                moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_WORD))
+        assertThat("Next word should fail at last word", success, equalTo(false))
+
+        // Move forward by character (onto trailing space).
+        success = provider.performAction(nodeId,
+                AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY,
+                moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_CHARACTER))
+        assertThat("Next char should succeed", success, equalTo(true))
+        waitUntilTextTraversed(424, 425, nodeId) // " "
+
+        // Try to move forward past last character.
+        success = provider.performAction(nodeId,
+                AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY,
+                moveByGranularityArguments(AccessibilityNodeInfo.MOVEMENT_GRANULARITY_CHARACTER))
+        assertThat("Next char should fail at last char", success, equalTo(false))
+    }
+
     @Test fun testHeadings() {
         var nodeId = AccessibilityNodeProvider.HOST_VIEW_ID;
         loadTestPage("test-headings")

mobile/android/geckoview/src/main/java/org/mozilla/geckoview/SessionAccessibility.java

@@ -271,7 +271,19 @@ public class SessionAccessibility {
                         nativeProvider.click(virtualViewId);
                     } else if (granularity > 0) {
                         boolean extendSelection = arguments.getBoolean(AccessibilityNodeInfo.ACTION_ARGUMENT_EXTEND_SELECTION_BOOLEAN);
-                        nativeProvider.navigateText(virtualViewId, granularity, mStartOffset, mEndOffset, action == AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY, extendSelection);
+                        boolean next = action == AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY;
+                        // We must return false if we're already at the edge.
+                        if (next) {
+                            if (mAtEndOfText) {
+                                return false;
+                            }
+                            if (granularity == AccessibilityNodeInfo.MOVEMENT_GRANULARITY_WORD && mAtLastWord) {
+                                return false;
+                            }
+                        } else if (mAtStartOfText) {
+                            return false;
+                        }
+                        nativeProvider.navigateText(virtualViewId, granularity, mStartOffset, mEndOffset, next, extendSelection);
                     }
                     return true;
                 case AccessibilityNodeInfo.ACTION_SET_SELECTION:
@@ -545,6 +557,9 @@ public class SessionAccessibility {
     private int mFocusedNode = 0;
     private int mStartOffset = -1;
     private int mEndOffset = -1;
+    private boolean mAtStartOfText = false;
+    private boolean mAtEndOfText = false;
+    private boolean mAtLastWord = false;
     // Viewport cache
     final SparseArray<GeckoBundle> mViewportCache = new SparseArray<>();
     // Focus cache
@@ -806,6 +821,9 @@ public class SessionAccessibility {
             case AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUSED:
                 mStartOffset = -1;
                 mEndOffset = -1;
+                mAtStartOfText = false;
+                mAtEndOfText = false;
+                mAtLastWord = false;
                 mAccessibilityFocusedNode = sourceId;
                 break;
             case AccessibilityEvent.TYPE_VIEW_FOCUSED:
@@ -818,6 +836,24 @@ public class SessionAccessibility {
             case AccessibilityEvent.TYPE_VIEW_TEXT_TRAVERSED_AT_MOVEMENT_GRANULARITY:
                 mStartOffset = event.getFromIndex();
                 mEndOffset = event.getToIndex();
+                // We must synchronously return false for text navigation
+                // actions if the user attempts to navigate past the edge.
+                // Because we do navigation async, we can't query this
+                // on demand when the action is performed. Therefore, we cache
+                // whether we're at either edge here.
+                mAtStartOfText = mStartOffset == 0;
+                CharSequence text = event.getText().get(0);
+                mAtEndOfText = mEndOffset >= text.length();
+                mAtLastWord = mAtEndOfText;
+                if (!mAtLastWord) {
+                    // Words exclude trailing spaces. To figure out whether
+                    // we're at the last word, we need to get the text after
+                    // our end offset and check if it's just spaces.
+                    CharSequence afterText = text.subSequence(mEndOffset, text.length());
+                    if (TextUtils.getTrimmedLength(afterText) == 0) {
+                        mAtLastWord = true;
+                    }
+                }
                 break;
         }