Fixes grammar output

Bug: 1002134
Change-Id: If72b6b9a153c249b099fd4307a17ac81f23914b3

Change-Id: If72b6b9a153c249b099fd4307a17ac81f23914b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1795575
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697287}

chrome/browser/resources/chromeos/chromevox/cvox2/background/editing.js

@@ -283,45 +283,30 @@ function AutomationRichEditableText(node) {
 
   this.updateIntraLineState_(this.line_);
 
-  /**
-   * @private {number|undefined}
-   */
+  /** @private {boolean} */
+  this.misspelled = false;
+  /** @private {boolean} */
+  this.grammarError = false;
+
+  /** @private {number|undefined} */
   this.fontSize_;
-  /**
-   * @private {string|undefined}
-   */
+  /** @private {string|undefined} */
   this.fontColor_;
-  /**
-   * @private {boolean|undefined}
-   */
+  /** @private {boolean|undefined} */
   this.linked_;
-  /**
-   * @private {boolean|undefined}
-   */
+  /** @private {boolean|undefined} */
   this.subscript_;
-  /**
-   * @private {boolean|undefined}
-   */
+  /** @private {boolean|undefined} */
   this.superscript_;
-  /**
-   * @private {boolean}
-   */
+  /** @private {boolean} */
   this.bold_ = false;
-  /**
-   * @private {boolean}
-   */
+  /** @private {boolean} */
   this.italic_ = false;
-  /**
-   * @private {boolean}
-   */
+  /** @private {boolean} */
   this.underline_ = false;
-  /**
-   * @private {boolean}
-   */
+  /** @private {boolean} */
   this.lineThrough_ = false;
-  /**
-   * @private {string|undefined}
-   */
+  /** @private {string|undefined} */
   this.fontFamily_;
 }
 
@@ -435,31 +420,8 @@ AutomationRichEditableText.prototype = {
       if (!container)
         return;
 
-      if (container.markerTypes) {
-        // Only consider markers that start or end at the selection bounds.
-        var markerStartIndex = -1, markerEndIndex = -1;
-        var localStartOffset = cur.localStartOffset;
-        for (var i = 0; i < container.markerStarts.length; i++) {
-          if (container.markerStarts[i] == localStartOffset) {
-            markerStartIndex = i;
-            break;
-          }
-        }
-
-        var localEndOffset = cur.localEndOffset;
-        for (var i = 0; i < container.markerEnds.length; i++) {
-          if (container.markerEnds[i] == localEndOffset) {
-            markerEndIndex = i;
-            break;
-          }
-        }
-
-        if (markerStartIndex > -1)
-          this.speakTextMarker_(container.markerTypes[markerStartIndex]);
-
-        if (markerEndIndex > -1)
-          this.speakTextMarker_(container.markerTypes[markerEndIndex], true);
-      }
+      this.speakTextMarker_(
+          container, cur.localStartOffset, cur.localEndOffset);
 
       if (localStorage['announceRichTextAttributes'] == 'true')
         this.speakTextStyle_(container);
@@ -607,21 +569,31 @@ AutomationRichEditableText.prototype = {
   },
 
   /**
-   * @param {number} markerType
-   * @param {boolean=} opt_end
+   * @param {AutomationNode!} container
+   * @param {number} selStart
+   * @param {number} selEnd
    * @private
    */
-  speakTextMarker_: function(markerType, opt_end) {
-    // TODO(dtseng): Plumb through constants to automation.
+  speakTextMarker_: function(container, selStart, selEnd) {
+    var markersWithinSelection = 0;
+    if (container.markerTypes) {
+      for (var i = 0; i < container.markerTypes.length; i++) {
+        // See if our selection intersects with this marker.
+        if (container.markerStarts[i] >= selStart ||
+            selEnd < container.markerEnds[i])
+          markersWithinSelection |= container.markerTypes[i];
+      }
+    }
+
     var msgs = [];
-    if (markerType & 1) {
-      if (localStorage['indicateMisspell'] == 'announce')
-        msgs.push(opt_end ? 'misspelling_end' : 'misspelling_start');
+    if (this.misspelled == !(markersWithinSelection & 1)) {
+      this.misspelled = !this.misspelled;
+      msgs.push(this.misspelled ? 'misspelling_start' : 'misspelling_end');
+    }
+    if (this.grammarError == !(markersWithinSelection & 2)) {
+      this.grammarError = !this.grammarError;
+      msgs.push(this.grammarError ? 'grammar_start' : 'grammar_end');
     }
-    if (markerType & 2)
-      msgs.push(opt_end ? 'grammar_end' : 'grammar_start');
-    if (markerType & 4)
-      msgs.push(opt_end ? 'text_match_end' : 'text_match_start');
 
     if (msgs.length) {
       msgs.forEach(function(msg) {
@@ -913,6 +885,23 @@ editing.EditableLine = function(
   this.end_ = new Cursor(endNode, endIndex);
   this.end_ = this.end_.deepEquivalent || this.end_;
 
+  // Update |startIndex| and |endIndex| if the calls above to
+  // cursors.Cursor.deepEquivalent results in cursors to different container
+  // nodes. The cursors can point directly to inline text boxes, in which case
+  // we should not adjust the container start or end index.
+  if (startNode.role == RoleType.STATIC_TEXT && this.start_.node != startNode &&
+      this.start_.node.parent != startNode) {
+    startIndex = this.start_.index == cursors.NODE_INDEX ?
+        this.start_.node.name.length :
+        this.start_.index;
+  }
+  if (endNode.role == RoleType.STATIC_TEXT && this.end_.node != endNode &&
+      this.end_.node.parent != endNode) {
+    endIndex = this.end_.index == cursors.NODE_INDEX ?
+        this.end_.node.name.length :
+        this.end_.index;
+  }
+
   /** @private {number} */
   this.localContainerStartOffset_ = startIndex;
   /** @private {number} */

chrome/browser/resources/chromeos/chromevox/cvox2/background/editing_test.extjs

@@ -1178,3 +1178,47 @@ TEST_F('ChromeVoxEditingTest', 'BackwardWordDeleteAcrossParagraphs', function()
     input.focus();
   });
 });
+
+TEST_F('ChromeVoxEditingTest', 'GrammarErrors', function() {
+  var mockFeedback = this.createMockFeedback();
+  this.runWithLoadedTree(function() {/*!
+    <div contenteditable="true" role="textbox">
+      This <span aria-invalid="grammar">are</span> a test
+    </div>
+    <button id="go">Go</button>
+
+    <script>
+      document.getElementById('go').addEventListener('click', function() {
+        var sel = getSelection();
+        sel.modify('move', 'forward', 'character');
+      }, true);
+    </script>
+  */}, function(root) {
+    var input = root.find({role: RoleType.TEXT_FIELD});
+    var go = root.find({role: RoleType.BUTTON});
+    var moveByChar = go.doDefault.bind(go);
+
+    this.listenOnce(input, 'focus', function() {
+      mockFeedback.call(moveByChar)
+          .expectSpeech('h')
+          .call(moveByChar)
+          .expectSpeech('i')
+          .call(moveByChar)
+          .expectSpeech('s')
+          .call(moveByChar)
+          .expectSpeech(' ')
+
+          .call(moveByChar)
+          .expectSpeech('a', 'Grammar error')
+          .call(moveByChar)
+          .expectSpeech('r')
+          .call(moveByChar)
+          .expectSpeech('e')
+          .call(moveByChar)
+          .expectSpeech(' ', 'Leaving grammar error')
+
+          .replay();
+    });
+    input.focus();
+  });
+});

chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd

@@ -2974,25 +2974,18 @@ If you're done with the tutorial, use ChromeVox to navigate to the Close button
       <message desc="Description of a command that toggles between 6 and 8 dot braille." name="IDS_CHROMEVOX_TOGGLE_BRAILLE_TABLE">
         Toggle between 6 and 8 dot braille
       </message>
-      <message desc="Spoken when a user navigates to the beginning of a misspelled word." name="IDS_CHROMEVOX_MISSPELLING_START">
-        Misspelling start
+      <message desc="Spoken when a user navigates into a misspelled word." name="IDS_CHROMEVOX_MISSPELLING_START">
+        Misspelled
       </message>
-      <message desc="Spoken when a user navigates to the end of a misspelled word." name="IDS_CHROMEVOX_MISSPELLING_END">
-        Misspelling end
+      <message desc="Spoken when a user navigates out of a misspelled word." name="IDS_CHROMEVOX_MISSPELLING_END">
+        Leaving misspelled
       </message>
-      <message desc="Spoken when a user navigates to the beginning of a grammar error." name="IDS_CHROMEVOX_GRAMMAR_START">
-        Grammar error start
+      <message desc="Spoken when a user navigates into a grammar error." name="IDS_CHROMEVOX_GRAMMAR_START">
+        Grammar error
       </message>
-      <message desc="Spoken when a user navigates to the end of a grammar error." name="IDS_CHROMEVOX_GRAMMAR_END">
-        Grammar error end
+      <message desc="Spoken when a user navigates out of a grammar error." name="IDS_CHROMEVOX_GRAMMAR_END">
+        Leaving grammar error
       </message>
-      <message desc="Spoken when a user navigates to the beginning of a text match from a find in page." name="IDS_CHROMEVOX_TEXT_MATCH_START">
-        Text match start
-      </message>
-      <message desc="Spoken when a user navigates to the end of a text match from a find in page." name="IDS_CHROMEVOX_TEXT_MATCH_END">
-        Text match end
-      </message>
-
       <message desc="Spoken to describe text that is superscript." name="IDS_CHROMEVOX_SUPERSCRIPT">
         Superscript
       </message>