Fixes for select-to-speak navigation.

+ Fix for issue where paragraph navigation breaks if inlineTextBox nodes are invalidated (such as page resize)
+ Fix issue when navigating to offscreen nodes. Text was clipped to visible bounds, which caused issues for nodes scrolled just outside of view.
+ Allow Search key + single click if STS is currently active.
+ Fix for navigating from within a nested block.

AX-Relnotes: N/A

Bug: 1158240
Change-Id: I8869392c268e989c35c656029bf0f238d13ed93f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587820
Commit-Queue: Joel Riley <joelriley@google.com>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837803}

chrome/browser/resources/chromeos/accessibility/select_to_speak/node_utils.js

@@ -495,7 +495,13 @@ class NodeUtils {
       return [];
     }
 
-    return NodeUtils.getAllNodesInParagraph(nextNode);
+    const nextNodes = NodeUtils.getNextNodesInParagraph(nextNode, direction);
+    if (direction === constants.Dir.FORWARD) {
+      nextNodes.unshift(nextNode);
+    } else {
+      nextNodes.push(nextNode);
+    }
+    return nextNodes;
   }
 
   /**

chrome/browser/resources/chromeos/accessibility/select_to_speak/node_utils_unittest.js

@@ -663,6 +663,38 @@ TEST_F(
       assertEquals(result[0], text2);
     });
 
+TEST_F(
+    'SelectToSpeakNodeUtilsUnitTest', 'getNextParagraphNestedBlocks',
+    function() {
+      const root = createMockNode({role: 'rootWebArea'});
+      const paragraph1 = createMockNode(
+          {role: 'paragraph', display: 'block', parent: root, root});
+      const text1 = createMockNode(
+          {role: 'staticText', parent: paragraph1, root, name: 'Before text'});
+      const nestedParagraph = createMockNode(
+          {role: 'paragraph', display: 'block', parent: paragraph1, root});
+      const text2 = createMockNode({
+        role: 'staticText',
+        parent: nestedParagraph,
+        root,
+        name: 'Middle text'
+      });
+      const text3 = createMockNode(
+          {role: 'staticText', parent: paragraph1, root, name: 'After text'});
+
+      // Getting next paragraph from nested paragraph only includes nodes in
+      // the forward direction (does not include itself)
+      let result = NodeUtils.getNextParagraph(text2, constants.Dir.FORWARD);
+      assertEquals(result.length, 1);
+      assertEquals(result[0], text3);
+
+      // Getting next paragraph from nested paragraph only includes nodes in
+      // the backward direction (does not include itself)
+      result = NodeUtils.getNextParagraph(text2, constants.Dir.BACKWARD);
+      assertEquals(result.length, 1);
+      assertEquals(result[0], text1);
+    });
+
 TEST_F('SelectToSpeakNodeUtilsUnitTest', 'getNextParagraphAndroid', function() {
   const root = createMockNode({role: 'application'});
   const container1 =

chrome/browser/resources/chromeos/accessibility/select_to_speak/paragraph_utils.js

@@ -313,11 +313,17 @@ class ParagraphUtils {
    * representing a paragraph of inline nodes.
    * @param {Array<!AutomationNode>} nodes List of automation nodes to use.
    * @param {number} index The index into nodes at which to start.
-   * @param {boolean} splitOnLanguage flag to determine if we should split nodes
+   * @param {{splitOnLanguage: (boolean|undefined),
+   *          clipOverflowWords: (boolean|undefined)}=} opt_options
+   *     splitOnLanguage: flag to determine if we should split nodes
    *                      up based on language.
+   *     clipOverflowWords: Whether to clip generated text.
    * @return {ParagraphUtils.NodeGroup} info about the node group
    */
-  static buildNodeGroup(nodes, index, splitOnLanguage) {
+  static buildNodeGroup(nodes, index, opt_options) {
+    const options = opt_options || {};
+    const splitOnLanguage = options.splitOnLanguage || false;
+    const clipOverflowWords = options.clipOverflowWords || false;
     let node = nodes[index];
     let next = nodes[index + 1];
     const blockParent = ParagraphUtils.getFirstBlockAncestor(nodes[index]);
@@ -362,9 +368,14 @@ class ParagraphUtils {
               new ParagraphUtils.NodeGroupItem(node, result.text.length, false);
         }
         if (newNode) {
+          if (clipOverflowWords) {
             result.text += ParagraphUtils.getNodeNameWithoutOverflowWords(
-                             newNode, blockParent) +
-              ' ';
+                newNode, blockParent);
+          } else {
+            result.text += ParagraphUtils.getNodeName(newNode.node);
+          }
+          // Add space between each node.
+          result.text += ' ';
           result.nodes.push(newNode);
         }
       }

chrome/browser/resources/chromeos/accessibility/select_to_speak/paragraph_utils_overflow_test.js

@@ -43,12 +43,11 @@ TEST_F(
       this.runWithLoadedTree(
           this.generateHorizentalOverflowText(inputText), function(root) {
             const overflowText = root.find({
-              role: 'inlineTextBox',
+              role: chrome.automation.RoleType.INLINE_TEXT_BOX,
               attributes: {name: inputText},
             });
             var nodeGroup = ParagraphUtils.buildNodeGroup(
-                [overflowText], 0, false /* do not split on language */
-            );
+                [overflowText], 0 /* index */, {clipOverflowWords: true});
 
             // The output text should have the same length of the input text
             // plus a space character at the end.
@@ -72,12 +71,11 @@ TEST_F(
           function(root) {
             // Find the visible text.
             const visibleTextNode = root.find({
-              role: 'inlineTextBox',
+              role: chrome.automation.RoleType.INLINE_TEXT_BOX,
               attributes: {name: visibleText},
             });
             var nodeGroup = ParagraphUtils.buildNodeGroup(
-                [visibleTextNode], 0, false /* do not split on language */
-            );
+                [visibleTextNode], 0 /* index */, {clipOverflowWords: true});
             // The output text should have the same length of the visible text
             // plus a space character at the end.
             assertEquals(nodeGroup.text.length, visibleText.length + 1);
@@ -88,12 +86,11 @@ TEST_F(
 
             // Find the overflow text.
             const overflowTextNode = root.find({
-              role: 'inlineTextBox',
+              role: chrome.automation.RoleType.INLINE_TEXT_BOX,
               attributes: {name: overflowText},
             });
             var nodeGroup = ParagraphUtils.buildNodeGroup(
-                [overflowTextNode], 0, false /* do not split on language */
-            );
+                [overflowTextNode], 0 /* index */, {clipOverflowWords: true});
 
             // The output text should have the same length of the overflow text
             // plus a space character at the end.
@@ -110,12 +107,11 @@ TEST_F(
       this.runWithLoadedTree(
           this.generateEntirelyOverflowText(inputText), function(root) {
             const overflowText = root.find({
-              role: 'inlineTextBox',
+              role: chrome.automation.RoleType.INLINE_TEXT_BOX,
               attributes: {name: inputText},
             });
             var nodeGroup = ParagraphUtils.buildNodeGroup(
-                [overflowText], 0, false /* do not split on language */
-            );
+                [overflowText], 0 /* index */, {clipOverflowWords: true});
 
             // The output text should have the same length of the input text
             // plus a space character at the end.
@@ -129,12 +125,11 @@ TEST_F('SelectToSpeakParagraphOverflowTest', 'OutputsVisibleText', function() {
   const inputText = 'This text is visible';
   this.runWithLoadedTree(this.generateVisibleText(inputText), function(root) {
     const visibleText = root.find({
-      role: 'inlineTextBox',
+      role: chrome.automation.RoleType.INLINE_TEXT_BOX,
       attributes: {name: inputText},
     });
     var nodeGroup = ParagraphUtils.buildNodeGroup(
-        [visibleText], 0, false /* do not split on language */
-    );
+        [visibleText], 0 /* index */, {clipOverflowWords: true});
 
     // The output text should have the same length of the input text plus a
     // space character at the end.
@@ -143,3 +138,26 @@ TEST_F('SelectToSpeakParagraphOverflowTest', 'OutputsVisibleText', function() {
     assertEquals(nodeGroup.text.replace(/ /g, ''), inputText.replace(/ /g, ''));
   });
 });
+
+TEST_F(
+    'SelectToSpeakParagraphOverflowTest',
+    'DoesNotClipOverflowWordsWhenDisabled', function() {
+      const inputText = 'This text overflows entirely';
+      this.runWithLoadedTree(
+          this.generateEntirelyOverflowText(inputText), function(root) {
+            const overflowText = root.find({
+              role: chrome.automation.RoleType.INLINE_TEXT_BOX,
+              attributes: {name: inputText},
+            });
+            var nodeGroup = ParagraphUtils.buildNodeGroup(
+                [overflowText], 0 /* index */, {clipOverflowWords: false});
+
+            // The output text should have the same length of the input text
+            // plus a space character at the end.
+            assertEquals(nodeGroup.text.length, inputText.length + 1);
+            // The output text should have same non-empty words as the input
+            // text.
+            assertEquals(
+                nodeGroup.text.replace(/ /g, ''), inputText.replace(/ /g, ''));
+          });
+    });

chrome/browser/resources/chromeos/accessibility/select_to_speak/paragraph_utils_unittest.js

@@ -236,7 +236,7 @@ TEST_F(
       const text3 =
           {role: 'staticText', parent: paragraph2, name: 'text3', root};
       const result = ParagraphUtils.buildNodeGroup(
-          [text1, text2, text3], 0, false /* do not split on language */);
+          [text1, text2, text3], 0, {splitOnLanguage: false});
       assertEquals('text1 text2 ', result.text);
       assertEquals(1, result.endIndex);
       assertEquals(2, result.nodes.length);
@@ -278,7 +278,7 @@ TEST_F(
       };
 
       const result1 = ParagraphUtils.buildNodeGroup(
-          [text1, text2, text3], 0, splitOnLanguage);
+          [text1, text2, text3], 0, {splitOnLanguage});
       assertEquals('text1 text2 ', result1.text);
       assertEquals(1, result1.endIndex);
       assertEquals(2, result1.nodes.length);
@@ -289,7 +289,7 @@ TEST_F(
       assertEquals('en-US', result1.detectedLanguage);
 
       const result2 = ParagraphUtils.buildNodeGroup(
-          [text1, text2, text3], 2, splitOnLanguage);
+          [text1, text2, text3], 2, {splitOnLanguage});
       assertEquals('text3 ', result2.text);
       assertEquals(2, result2.endIndex);
       assertEquals(1, result2.nodes.length);
@@ -309,7 +309,7 @@ TEST_F(
       const text2 = {role: 'staticText', parent: root, name: 'text2', root};
       const text3 = {role: 'staticText', parent: root, name: 'text3', root};
       const result = ParagraphUtils.buildNodeGroup(
-          [text1, text2, text3], 0, splitOnLanguage);
+          [text1, text2, text3], 0, {splitOnLanguage});
       assertEquals('text1 text2 text3 ', result.text);
       assertEquals(2, result.endIndex);
       assertEquals(3, result.nodes.length);
@@ -339,7 +339,7 @@ TEST_F(
         detectedLanguage: 'fr-FR'
       };
       const result = ParagraphUtils.buildNodeGroup(
-          [text1, text2, text3], 0, splitOnLanguage);
+          [text1, text2, text3], 0, {splitOnLanguage});
       assertEquals('text1 text2 text3 ', result.text);
       assertEquals(2, result.endIndex);
       assertEquals(3, result.nodes.length);
@@ -377,7 +377,7 @@ TEST_F(
         detectedLanguage: 'fr-FR'
       };
       const result = ParagraphUtils.buildNodeGroup(
-          [text1, text2, text3, text4], 0, splitOnLanguage);
+          [text1, text2, text3, text4], 0, {splitOnLanguage});
       assertEquals('text1 text2 text3 text4 ', result.text);
       assertEquals(3, result.endIndex);
       assertEquals(4, result.nodes.length);
@@ -413,7 +413,7 @@ TEST_F(
         detectedLanguage: 'fr-FR'
       };
       const result = ParagraphUtils.buildNodeGroup(
-          [text1, text2, text3, text4], 0, splitOnLanguage);
+          [text1, text2, text3, text4], 0, {splitOnLanguage});
       assertEquals('text1 text2 text3 ', result.text);
       assertEquals(2, result.endIndex);
       assertEquals(3, result.nodes.length);
@@ -440,7 +440,7 @@ TEST_F(
       const linkText =
           {role: 'staticText', parent: link, name: 'linkText', root};
       const result = ParagraphUtils.buildNodeGroup(
-          [text1, text2, linkText], 0, false /* do not split on language */);
+          [text1, text2, linkText], 0, {splitOnLanguage: false});
       assertEquals('text1 linkText ', result.text);
       assertEquals(2, result.endIndex);
       assertEquals(2, result.nodes.length);
@@ -468,7 +468,7 @@ TEST_F(
       // If there is no value, it should use the name.
       searchBar.value = '';
       result = ParagraphUtils.buildNodeGroup(
-          [searchBar], 0, false /* do not split on language */);
+          [searchBar], 0, {splitOnLanguage: false});
       assertEquals('Address and search bar ', result.text);
     });
 
@@ -481,6 +481,6 @@ TEST_F('SelectToSpeakParagraphUnitTest', 'BuildNodeGroupWithSvg', function() {
   const inline2 = {role: 'inlineTextBox', parent: text2, root, name: 'world!'};
 
   const result = ParagraphUtils.buildNodeGroup(
-      [inline1, inline2], 0, false /* do not split on language */);
+      [inline1, inline2], 0, {splitOnLanguage: false});
   assertEquals('Hello, world! ', result.text);
 });

chrome/browser/resources/chromeos/accessibility/select_to_speak/select_to_speak.js

@@ -331,13 +331,11 @@ class SelectToSpeak {
         // more items are being read.
         return;
       }
-      if (this.navigationControlFlag_ && nodes.length === 1 &&
-          nodes[0].role === RoleType.INLINE_TEXT_BOX &&
-          (rect.width === 0 || rect.height === 0)) {
+      if (this.shouldShowNavigationControls_() && nodes.length > 0 &&
+          (rect.width <= SelectToSpeak.PARAGRAPH_SELECTION_MAX_SIZE ||
+           rect.height <= SelectToSpeak.PARAGRAPH_SELECTION_MAX_SIZE)) {
         // If this is a single click (zero sized selection) on a text node, then
         // expand to entire paragraph.
-        // TODO(crbug.com/1143823): Navigate to the sentence instead of whole
-        // block.
         nodes = NodeUtils.getAllNodesInParagraph(nodes[0]);
       }
       this.startSpeechQueue_(nodes, {clearFocusRing: true});
@@ -578,7 +576,7 @@ class SelectToSpeak {
    * @private
    */
   onNullSelection_() {
-    if (!this.navigationControlFlag_) {
+    if (!this.shouldShowNavigationControls_()) {
       this.null_selection_tone_.play();
       return;
     }
@@ -1007,12 +1005,9 @@ class SelectToSpeak {
     // Use current block parent as starting point to navigate from. If it is not
     // a valid block, then use one of the nodes that are currently activated.
     let node = this.currentBlockParent_;
-    if ((node === null || node.isRootNode) &&
-        this.navigationState_.currentNodes.length > 0) {
-      const nodeIdx = direction === constants.Dir.FORWARD ?
-          this.navigationState_.currentNodes.length - 1 :
-          0;
-      node = this.navigationState_.currentNodes[nodeIdx];
+    if ((node === null || node.isRootNode) && this.currentNodeGroup_ &&
+        this.currentNodeGroup_.nodes.length > 0) {
+      node = this.currentNodeGroup_.nodes[0].node;
     }
     if (node === null) {
       // Could not find any nodes to navigate from.
@@ -1158,9 +1153,16 @@ class SelectToSpeak {
     if (currentNodeGroupStartNodeIndex >= currentNodes.length) {
       return;
     }
+    // When navigation controls are enabled, disable the
+    // clipping of overflow words. When overflow words are clipped, words
+    // scrolled out of view are clipped, which is undesirable for our navigation
+    // features as we generate node groups for next/previous paragraphs which
+    // may be fully or partially scrolled out of view.
     const nodeGroup = ParagraphUtils.buildNodeGroup(
-        currentNodes, currentNodeGroupStartNodeIndex,
-        this.enableLanguageDetectionIntegration_);
+        currentNodes, currentNodeGroupStartNodeIndex, {
+          splitOnLanguage: this.enableLanguageDetectionIntegration_,
+          clipOverflowWords: !this.shouldShowNavigationControls_(),
+        });
 
     // |currentCharIndex| is the start char index of the word to be spoken in
     // the nodeGroup text. If the |currentCharIndex| is non-zero, that means we
@@ -1218,7 +1220,7 @@ class SelectToSpeak {
         nodeGroup.detectedLanguage) {
       options.lang = nodeGroup.detectedLanguage;
     }
-    if (this.navigationControlFlag_) {
+    if (this.shouldShowNavigationControls_()) {
       options.rate = this.getSpeechRate_();
     }
 
@@ -1256,8 +1258,16 @@ class SelectToSpeak {
           this.testCurrentNode_();
         }
       } else if (event.type === 'interrupted' || event.type === 'cancelled') {
-        if (!this.shouldShowNavigationControls_() ||
-            !this.pauseCompleteCallback_) {
+        if (!this.shouldShowNavigationControls_()) {
+          this.onStateChanged_(SelectToSpeakState.INACTIVE);
+        }
+        if (this.state_ === SelectToSpeakState.SELECTING) {
+          // Do not go into inactive state if navigation controls are enabled
+          // and we're currently making a new selection. This enables users
+          // to select new nodes while STS is active without first exiting.
+          return;
+        }
+        if (!this.pauseCompleteCallback_) {
           // Auto dismiss when navigation control is not enabled. In addition,
           // if the interrupted or cancelled events are not triggered by
           // |this.pause_| (e.g., from stopAll_), we should leave STS as
@@ -1821,3 +1831,12 @@ SelectToSpeak.READ_SELECTION_KEY_CODE = KeyCode.S;
  * @const {number}
  */
 SelectToSpeak.NODE_STATE_TEST_INTERVAL_MS = 500;
+
+/**
+ * Max size in pixels for a region selection to be considered a paragraph
+ * selection vs a selection of specific nodes. Generally paragraph
+ * selection is a single click (size 0), though allow for a little
+ * jitter.
+ * @const {number}
+ */
+SelectToSpeak.PARAGRAPH_SELECTION_MAX_SIZE = 5;