Smoothly speak non-paragraph inline items.

Don't highlight whitespace-only nodes.

Bug: 753022
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Iaac62b135512ba95ea6dfaabc9450802e939e621
Reviewed-on: https://chromium-review.googlesource.com/719763
Commit-Queue: Katie Dektar <katydek@google.com>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509298}

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

@@ -6,32 +6,33 @@ var AutomationNode = chrome.automation.AutomationNode;
 var RoleType = chrome.automation.RoleType;
 
 /**
- * Gets the parent paragraph of a node, or null if it is not in a paragraph.
+ * Gets the first ancestor of a node which is a paragraph, or display:block,
+ * or the root node if none is found.
  * @param { AutomationNode } node The node to get the parent for.
  * @return { ?AutomationNode } the parent paragraph or null if there is none.
  */
-function getParagraphParent(node) {
+function getFirstBlockAncestor(node) {
   let parent = node.parent;
+  let root = node.root;
   while (parent != null) {
+    if (parent == root) {
+      return parent;
+    }
     if (parent.role == RoleType.PARAGRAPH) {
       return parent;
     }
+    if ((parent.display == 'block' || parent.display == 'inline-block') &&
+        parent.role != RoleType.STATIC_TEXT) {
+      return parent;
+    }
     parent = parent.parent;
   }
   return null;
 }
 
 /**
- * Determines whether a node is inside of a paragraph.
- * @param { AutomationNode } node The node to test
- * @return { boolean } whether the node is in a paragraph
- */
-function isInParagraph(node) {
-  return getParagraphParent(node) != null;
-}
-
-/**
- * Determines whether two nodes are in the same paragraph.
+ * Determines whether two nodes are in the same block-like ancestor, i.e.
+ * whether they are in the same paragraph.
  * @param { AutomationNode|undefined } first The first node to compare.
  * @param { AutomationNode|undefined } second The second node to compare.
  * @return { boolean } whether two nodes are in the same paragraph.
@@ -41,17 +42,33 @@ function inSameParagraph(first, second) {
     return false;
   }
   // TODO: Clean up this check after crbug.com/774308 is resolved.
-  // At that point we will only need to check for display:block.
-  if ((first.display == 'block' && first.role != RoleType.STATIC_TEXT &&
+  // At that point we will only need to check for display:block or inline-block.
+  if (((first.display == 'block' || first.display == 'inline-block') &&
+       first.role != RoleType.STATIC_TEXT &&
        first.role != RoleType.INLINE_TEXT_BOX) ||
-      (second.display == 'block' && second.role != RoleType.STATIC_TEXT &&
+      ((second.display == 'block' || second.display == 'inline-block') &&
+       second.role != RoleType.STATIC_TEXT &&
        second.role != RoleType.INLINE_TEXT_BOX)) {
-    // 'block' elements cannot be in the same paragraph.
+    // 'block' or 'inline-block' elements cannot be in the same paragraph.
     return false;
   }
-  let firstParent = getParagraphParent(first);
-  let secondParent = getParagraphParent(second);
-  return firstParent != undefined && firstParent == secondParent;
+  let firstBlock = getFirstBlockAncestor(first);
+  let secondBlock = getFirstBlockAncestor(second);
+  return firstBlock != undefined && firstBlock == secondBlock;
+}
+
+/**
+ * Determines whether a string is only whitespace.
+ * @param { string } name A string to test
+ * @return { boolean } whether the string is only whitespace
+ */
+function isWhitespace(name) {
+  if (name.length == 0) {
+    return true;
+  }
+  // Search for one or more whitespace characters
+  let re = /^\s+$/;
+  return re.exec(name) != null;
 }
 
 /**
@@ -77,7 +94,7 @@ function buildNodeGroup(nodes, index) {
     index += 1;
     node = next;
     next = nodes[index + 1];
-    if (node.name === undefined || node.name == '') {
+    if (node.name === undefined || isWhitespace(node.name)) {
       // Don't bother with unnamed or empty nodes.
       continue;
     }

chrome/browser/resources/chromeos/select_to_speak/paragraph_utils_unittest.gtestjs

@@ -21,31 +21,31 @@ SelectToSpeakParagraphUnitTest.prototype = {
   ]
 };
 
-TEST_F('SelectToSpeakParagraphUnitTest', 'IsInParagraph', function() {
+TEST_F('SelectToSpeakParagraphUnitTest', 'GetFirstBlockAncestor', function() {
   let root = {role: 'rootWebArea'};
-  let paragraph = {role: 'paragraph', parent: 'rootWebArea'};
-  let text1 = {role: 'staticText', parent: paragraph};
-  let text2 = {role: 'staticText', parent: 'rootWebArea'};
-  assertTrue(isInParagraph(text1));
-  assertFalse(isInParagraph(text2));
-});
-
-TEST_F('SelectToSpeakParagraphUnitTest', 'GetParagraphParent', function() {
-  let root = {role: 'rootWebArea'};
-  let paragraph = {role: 'paragraph', parent: 'rootWebArea'};
-  let text1 = {role: 'staticText', parent: paragraph};
-  let text2 = {role: 'staticText', parent: 'rootWebArea'};
-  assertEquals(paragraph, getParagraphParent(text1));
-  assertEquals(null, getParagraphParent(text2));
+  let paragraph = {role: 'paragraph', parent: root, root: root};
+  let text1 = {role: 'staticText', parent: paragraph, display: 'block',
+      root: root};
+  let text2 = {role: 'staticText', parent: root, root: root};
+  let text3 = {role: 'inlineTextBox', parent: text1, root: root};
+  let div = {role: 'genericContainer', parent: paragraph, display: 'block',
+      root: root};
+  let text4 = {role: 'staticText', parent: div, root: root};
+  assertEquals(paragraph, getFirstBlockAncestor(text1));
+  assertEquals(root, getFirstBlockAncestor(text2));
+  assertEquals(paragraph, getFirstBlockAncestor(text3));
+  assertEquals(div, getFirstBlockAncestor(text4));
 });
 
 TEST_F('SelectToSpeakParagraphUnitTest', 'InSameParagraph', function() {
   let root = {role: 'rootWebArea'};
-  let paragraph1 = {role: 'paragraph', parent: 'rootWebArea'};
-  let text1 = {role: 'staticText', parent: paragraph1};
-  let text2 = {role: 'staticText', parent: paragraph1};
-  let paragraph2 = {role: 'paragraph', parent: 'rootWebArea'};
-  let text3 = {role: 'staticText', parent: paragraph2};
+  let paragraph1 = {role: 'paragraph', display: 'block', parent: 'rootWebArea',
+      root: root};
+  let text1 = {role: 'staticText', parent: paragraph1, root: root};
+  let text2 = {role: 'staticText', parent: paragraph1, root: root};
+  let paragraph2 = {role: 'paragraph', display: 'block', parent: 'rootWebArea',
+      root: root};
+  let text3 = {role: 'staticText', parent: paragraph2, root: root};
   assertTrue(inSameParagraph(text1, text2));
   assertFalse(inSameParagraph(text1, text3));
 });
@@ -53,24 +53,38 @@ TEST_F('SelectToSpeakParagraphUnitTest', 'InSameParagraph', function() {
 TEST_F('SelectToSpeakParagraphUnitTest', 'BlockDivBreaksSameParagraph',
     function() {
   let root = {role: 'rootWebArea'};
-  let paragraph1 = {role: 'paragraph', parent: 'rootWebArea'};
-  let text1 = {role: 'staticText', parent: paragraph1};
-  let text2 = {role: 'image', parent: paragraph1, display: 'block'};
-  let text3 = {role: 'image', parent: paragraph1, display: 'inline'};
-  let text4 = {role: 'staticText', parent: paragraph1};
+  let paragraph1 = {role: 'paragraph', display: 'block', parent: 'rootWebArea',
+      root: root};
+  let text1 = {role: 'staticText', parent: paragraph1, root: root};
+  let text2 = {role: 'image', parent: paragraph1, display: 'block', root: root};
+  let text3 = {role: 'image', parent: paragraph1, display: 'inline', root: root};
+  let text4 = {role: 'staticText', parent: paragraph1, root: root};
   assertFalse(inSameParagraph(text1, text2));
   assertFalse(inSameParagraph(text2, text3));
   assertTrue(inSameParagraph(text3, text4));
 });
 
+TEST_F('SelectToSpeakParagraphUnitTest', 'IsWhitespace', function() {
+  assertTrue(isWhitespace(''));
+  assertTrue(isWhitespace(' '));
+  assertTrue(isWhitespace(' \n \t '));
+  assertFalse(isWhitespace('cats'));
+  assertFalse(isWhitespace(' cats '));
+});
+
 TEST_F('SelectToSpeakParagraphUnitTest', 'BuildNodeGroupStopsAtNewParagraph',
     function() {
   let root = {role: 'rootWebArea'};
-  let paragraph1 = {role: 'paragraph', parent: 'rootWebArea'};
-  let text1 = {role: 'staticText', parent: paragraph1, name: 'text1'};
-  let text2 = {role: 'staticText', parent: paragraph1, name: 'text2'};
-  let paragraph2 = {role: 'paragraph', parent: 'rootWebArea'};
-  let text3 = {role: 'staticText', parent: paragraph2, name: 'text3'};
+  let paragraph1 = {role: 'paragraph', display: 'block', parent: root,
+      root: root};
+  let text1 = {role: 'staticText', parent: paragraph1, name: 'text1',
+      root: root};
+  let text2 = {role: 'staticText', parent: paragraph1, name: 'text2',
+      root: root};
+  let paragraph2 = {role: 'paragraph', display: 'block', parent: root,
+      root: root};
+  let text3 = {role: 'staticText', parent: paragraph2, name: 'text3',
+      root: root};
   let result = buildNodeGroup([text1, text2, text3], 0);
   assertEquals('text1 text2', result.text);
   assertEquals(1, result.endIndex);
@@ -84,13 +98,19 @@ TEST_F('SelectToSpeakParagraphUnitTest', 'BuildNodeGroupStopsAtNewParagraph',
 TEST_F('SelectToSpeakParagraphUnitTest', 'BuildNodeGroupIncludesLinks',
     function() {
   let root = {role: 'rootWebArea'};
-  let paragraph1 = {role: 'paragraph', parent: 'rootWebArea'};
-  let text1 = {role: 'staticText', parent: paragraph1, name: 'text1'};
-  let link = {role: 'link', parent: paragraph1};
-  let linkText = {role: 'staticText', parent: link, name: 'linkText'};
-  let result = buildNodeGroup([text1, linkText], 0);
+  let paragraph1 = {role: 'paragraph', display: 'block', parent: root,
+      root: root};
+  let text1 = {role: 'staticText', parent: paragraph1, name: 'text1',
+      root: root};
+  // Whitespace-only nodes should be ignored.
+  let text2 = {role: 'staticText', parent: paragraph1, name: '\n',
+      root: root};
+  let link = {role: 'link', parent: paragraph1, root: root};
+  let linkText = {role: 'staticText', parent: link, name: 'linkText',
+      root: root};
+  let result = buildNodeGroup([text1, text2, linkText], 0);
   assertEquals('text1 linkText', result.text);
-  assertEquals(1, result.endIndex);
+  assertEquals(2, result.endIndex);
   assertEquals(2, result.nodes.length);
   assertEquals(0, result.nodes[0].startChar);
   assertEquals(text1, result.nodes[0].node);