Refine braille output

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview Provides output services for ChromeVox.
  */
 
 goog.provide('Output');
 goog.provide('Output.EventType');
@@ skipping 632 matching lines @@
   /** @override */
   run: function() {
     cvox.ChromeVox.earcons.playEarcon(cvox.Earcon[this.earconId]);
   }
 };
 
 /**
  * Annotation for text with a selection inside it.
  * @param {number} startIndex
  * @param {number} endIndex
- * @param {number=} opt_offset
  * @constructor
  */
-Output.SelectionSpan = function(startIndex, endIndex, opt_offset) {
+Output.SelectionSpan = function(startIndex, endIndex) {
   // TODO(dtseng): Direction lost below; should preserve for braille panning.
   this.startIndex = startIndex < endIndex ? startIndex : endIndex;
   this.endIndex = endIndex > startIndex ? endIndex : startIndex;
-  this.offset = opt_offset || 0;
 };
 
 /**
  * Wrapper for automation nodes as annotations.  Since the
  * {@code AutomationNode} constructor isn't exposed in the API, this class is
  * used to allow instanceof checks on these annotations.
  * @param {!AutomationNode} node
  * @constructor
  */
 Output.NodeSpan = function(node) {
@@ skipping 194 matching lines @@
    * @param {string} formatStr
    * @param {!AutomationNode=} opt_node An optional node to apply the
    *     formatting to.
    * @return {!Output} |this| for chaining
    */
   formatForBraille: function(formatStr, opt_node) {
     var node = opt_node || null;
 
     this.formatOptions_ = {speech: false, braille: true, auralStyle: false};
     this.format_(node, formatStr, this.brailleBuffer_);
-
     return this;
   },
 
   /**
    * Triggers callback for a speech event.
    * @param {function()} callback
    * @return {Output}
    */
   onSpeechEnd: function(callback) {
     this.speechEndCallback_ = function(opt_cleanupOnly) {
@@ skipping 39 matching lines @@
       if (i == this.speechBuffer_.length - 1)
         speechProps['endCallback'] = this.speechEndCallback_;
 
       cvox.ChromeVox.tts.speak(
           buff.toString(), queueMode, speechProps);
       queueMode = cvox.QueueMode.QUEUE;
     }
 
     // Braille.
     if (this.brailleBuffer_.length) {
-      var buff = this.createBrailleOutput_();
+      var buff = this.mergeBraille_(this.brailleBuffer_);
       var selSpan =
           buff.getSpanInstanceOf(Output.SelectionSpan);
       var startIndex = -1, endIndex = -1;
       if (selSpan) {
         var valueStart = buff.getSpanStart(selSpan);
         var valueEnd = buff.getSpanEnd(selSpan);
         startIndex = valueStart + selSpan.startIndex;
         endIndex = valueStart + selSpan.endIndex;
-        buff.setSpan(new cvox.ValueSpan(selSpan.offset), valueStart, valueEnd);
+        buff.setSpan(new cvox.ValueSpan(0), valueStart, valueEnd);
         buff.setSpan(new cvox.ValueSelectionSpan(), startIndex, endIndex);
       }
 
       var output = new cvox.NavBraille({
         text: buff,
         startIndex: startIndex,
         endIndex: endIndex
       });
 
       cvox.ChromeVox.braille.write(output);
@@ skipping 76 matching lines @@
       // Obtain the operator token.
       token = tree.value;
 
       // Set suffix options.
       var options = {};
       options.annotation = [];
       options.isUnique = token[token.length - 1] == '=';
       if (options.isUnique)
         token = token.substring(0, token.length - 1);
 
-      // Annotate braille output with the corresponding automation nodes
-      // to support acting on nodes based on location in the output.
-      if (node && this.formatOptions_.braille)
-        options.annotation.push(new Output.NodeSpan(node));
-
       // Process token based on prefix.
       var prefix = token[0];
       token = token.slice(1);
 
       // All possible tokens based on prefix.
       if (prefix == '$') {
         if (token == 'value') {
           var text = node.value;
           if (!node.state.editable && node.name == text)
             return;
 
           var selectedText = '';
           if (text !== undefined) {
             if (node.textSelStart !== undefined) {
               options.annotation.push(new Output.SelectionSpan(
                   node.textSelStart,
                   node.textSelEnd));
 
               selectedText =
                   node.value.substring(node.textSelStart, node.textSelEnd);
             }
           }
           options.annotation.push(token);
           if (selectedText) {
             this.append_(buff, selectedText, options);
-            this.append_(buff, Msgs.getMsg('selected'), options);
+            this.append_(buff, Msgs.getMsg('selected'));
           } else {
             this.append_(buff, text, options);
           }
         } else if (token == 'name') {
           options.annotation.push(token);
           var earcon = node ? this.findEarcon_(node, opt_prevNode) : null;
           if (earcon)
             options.annotation.push(earcon);
           this.append_(buff, node.name, options);
         } else if (token == 'urlFilename') {
@@ skipping 409 matching lines @@
       if (enteredRoleSet[formatPrevNode.role] ||
           node.role == formatPrevNode.role ||
           localStorage['useVerboseMode'] == 'false')
         continue;
 
       var roleBlock = getMergedRoleBlock(formatPrevNode.role);
       if (roleBlock.leave && localStorage['useVerboseMode'] == 'true')
         this.format_(formatPrevNode, roleBlock.leave, buff, prevNode);
     }
 
-    var enterOutputs = [];
+    // Customize for braille node annotations.
+    var originalBuff = buff;
     var enterRole = {};
     for (var j = uniqueAncestors.length - 1, formatNode;
          (formatNode = uniqueAncestors[j]);
          j--) {
       var roleBlock = getMergedRoleBlock(formatNode.role);
       if (roleBlock.enter) {
         if (enterRole[formatNode.role])
           continue;
+
+        if (this.formatOptions_.braille)
+          buff = [];
+
         enterRole[formatNode.role] = true;
         this.format_(formatNode, roleBlock.enter, buff, prevNode);
+
+        if (this.formatOptions_.braille && buff.length) {
+          var nodeSpan = this.mergeBraille_(buff);
+          nodeSpan.setSpan(new Output.NodeSpan(formatNode), 0, nodeSpan.length);
+          originalBuff.push(nodeSpan);
+        }
       }
     }
   },
 
   /**
    * @param {!AutomationNode} node
    * @param {!AutomationNode} prevNode
    * @param {EventType|Output.EventType} type
    * @param {!Array<Spannable>} buff
    * @private
    */
   node_: function(node, prevNode, type, buff) {
+    var originalBuff = buff;
+
+    if (this.formatOptions_.braille)
+      buff = [];
+
     // Navigate is the default event.
     var eventBlock = Output.RULES[type] || Output.RULES['navigate'];
     var roleBlock = eventBlock[node.role] || {};
     var parentRole = (Output.ROLE_INFO_[node.role] || {}).inherits;
     var parentRoleBlock = eventBlock[parentRole || ''] || {};
     var speakFormat = roleBlock.speak ||
         parentRoleBlock.speak ||
         eventBlock['default'].speak;
 
     this.format_(node, speakFormat, buff, prevNode);
+
+    // Restore braille and add an annotation for this node.
+    if (this.formatOptions_.braille) {
+      var nodeSpan = this.mergeBraille_(buff);
+      nodeSpan.setSpan(new Output.NodeSpan(node), 0, nodeSpan.length);
+      originalBuff.push(nodeSpan);
+    }
   },
 
   /**
    * @param {!cursors.Range} range
    * @param {cursors.Range} prevRange
    * @param {EventType|Output.EventType} type
    * @param {!Array<Spannable>} buff
    * @private
    */
   subNode_: function(range, prevRange, type, buff) {
     if (!prevRange)
       prevRange = range;
     var dir = cursors.Range.getDirection(prevRange, range);
     var node = range.start.node;
     var prevNode = prevRange.getBound(dir).node;
     if (!node || !prevNode)
       return;
 
     var options = {annotation: ['name'], isUnique: true};
-    var startIndex = range.start.index;
-    var endIndex = range.end.index;
+    var rangeStart = range.start.index;
+    var rangeEnd = range.end.index;
     if (this.formatOptions_.braille) {
       options.annotation.push(new Output.NodeSpan(node));
       var selStart = node.textSelStart;
       var selEnd = node.textSelEnd;
+
       if (selStart !== undefined &&
-          (selEnd >= startIndex && selStart <= endIndex)) {
+          selEnd >= rangeStart && selStart <= rangeEnd) {
+        // Editable text selection.
+
+        // |rangeStart| and |rangeEnd| are indices set by the caller and are
+        // assumed to be inside of the range. In braille, we only ever expect to
+        // get ranges surrounding a line as anything smaller doesn't make sense.
+
+        // |selStart| and |selEnd| reflect the editable selection. The relative
+        // selStart and relative selEnd for the current line are then just the
+        // difference between |selStart|, |selEnd| with |rangeStart|.
+        // See editing_test.js for examples.
         options.annotation.push(new Output.SelectionSpan(
-            selStart - startIndex,
-            selEnd - startIndex,
-            startIndex));
+            selStart - rangeStart, selEnd - rangeStart));
+      } else {
+        // Non-editable text selection.
+        options.annotation.push(new Output.SelectionSpan(rangeStart, rangeEnd));
       }
     }
 
     if (this.outputContextFirst_)
       this.ancestry_(node, prevNode, type, buff);
     var earcon = this.findEarcon_(node, prevNode);
     if (earcon)
       options.annotation.push(earcon);
-    this.append_(buff, range.start.getText().substring(startIndex, endIndex),
-        options);
+    var text = '';
+
+    if (this.formatOptions_.braille && !node.state.editable) {
+      // In braille, we almost always want to show the entire contents and
+      // simply place the cursor under the SelectionSpan we set above.
+      text = range.start.getText();
+    } else {
+      // This is output for speech or non-editable braille.

[dmazzoni, 2016/11/10 23:31:29]

This appears backwards from your logic. Above, you have if
(this.formatOptions_.braille && !node.state.editable), but your comment says
that the else case is for non-editable braille

[David Tseng, 2016/11/10 23:54:59]

Changed comment.
The code is right. In editable braille, we want to restrict to the start/end
bounds of the range which is always a line. The if clause covers when we're in
non-editable braille and moving by characters and words not showing just the
spoken word.

[David Tseng, 2016/11/11 02:36:27]

Alright, to clarify things, I added a test and constrained more of this
SelectionSpan. We are now only setting the SelectionSpan on non-editable content
when that selection is less than the full contents of the range (it's really
distracting to read braille when everything's underlined).

Also, I filed a bug to clean up output.js in general in phase4. We can split a
lot of these things up (e.g. BaseOutput, OutputParser, BrailleOutput,
SpeechOutput, OutputRule, OutputRuleGroup, etc); should look a lot prettier
afterwards..

+      text = range.start.getText().substring(rangeStart, rangeEnd);
+    }
+
+    this.append_(buff, text, options);
+
     if (!this.outputContextFirst_)
       this.ancestry_(node, prevNode, type, buff);
 
     var loc =
-        range.start.node.boundsForRange(startIndex, endIndex);
+        range.start.node.boundsForRange(rangeStart, rangeEnd);
     if (loc)
       this.locations_.push(loc);
   },
 
   /**
    * Appends output to the |buff|.
    * @param {!Array<Spannable>} buff
    * @param {string|!Spannable} value
    * @param {{isUnique: (boolean|undefined),
    *      annotation: !Array<*>}=} opt_options
@@ skipping 76 matching lines @@
       index++;
     }
 
     if (currentNode != root)
       throw 'Unbalanced parenthesis: ' + inputStr;
 
     return root;
   },
 
   /**
-   * Converts the currently rendered braille buffers to a single spannable.
+   * Converts the braille |spans| buffer to a single spannable.
+   * @param {!Array<Spannable>} spans
    * @return {!Spannable}
    * @private
    */
-  createBrailleOutput_: function() {
-    var result = new Spannable();
+  mergeBraille_: function(spans) {
     var separator = '';  // Changes to space as appropriate.
-    this.brailleBuffer_.forEach(function(cur) {
-      // If this chunk is empty, don't add it since it won't result
-      // in any output on the braille display, but node spans would
-      // start before the separator in that case, which is not desired.
-      // The exception is if this chunk contains a selectionm, in which
-      // case it will result in a cursor which has to be preserved.
-      // In this case, having separators, potentially both before and after
-      // the empty string is correct.
-      if (cur.length == 0 && !cur.getSpanInstanceOf(Output.SelectionSpan))
-        return;
-      var spansToExtend = [];
-      var spansToRemove = [];
-      // Nodes that have node spans both on the character to the left
-      // of the separator and to the right should also cover the separator.
-      // We extend the left span to cover both the separator and what the
-      // right span used to cover, removing the right span, mostly for
-      // ease of writing tests and debug.
-      // Note that getSpan(position) never returns zero length spans
-      // (because they don't cover any position).  Still, we want to include
-      // these because they can be included (the selection span in an empty
-      // text field is an example), which is why we write the below code
-      // using getSpansInstanceOf and check the endpoints (isntead of doing
-      // the opposite).
-      result.getSpansInstanceOf(Output.NodeSpan).forEach(function(leftSpan) {
-        if (result.getSpanEnd(leftSpan) < result.length)
-          return;
-        var newEnd = result.length;
-        cur.getSpansInstanceOf(Output.NodeSpan).forEach(function(rightSpan) {
-          if (cur.getSpanStart(rightSpan) == 0 &&
-              leftSpan.node === rightSpan.node) {
-            newEnd = Math.max(
-                newEnd,
-                result.length + separator.length +
-                    cur.getSpanEnd(rightSpan));
-            spansToRemove.push(rightSpan);
-          }
-        });
-        if (newEnd > result.length)
-          spansToExtend.push({span: leftSpan, end: newEnd});
-      });
+    var prevHasInlineNode = false;
+    return spans.reduce(function(result, cur) {
+      // Ignore empty spans except when they contain a selection.
+      var hasSelection = cur.getSpanInstanceOf(Output.SelectionSpan);
+      if (cur.length == 0 && !hasSelection)
+        return result;
+
+      // For empty selections, we just add the space separator to account for
+      // showing the braille cursor.
+      if (cur.length == 0 && hasSelection) {
+        result.append(cur);
+        result.append(Output.SPACE);
+        separator = '';
+        return result;
+      }
+
+      // Keep track of if there's an inline node associated with
+      // |cur|.
+      var hasInlineNode = cur.getSpansInstanceOf(Output.NodeSpan)
+          .some(function(s) {
+            return s.node && s.node.display == 'inline';
+          });
+
+      // Now, decide whether we should include separators between the previous
+      // span and |cur|.
+      // Never separate chunks without something already there at this point.
+      if (result.length == 0 || hasInlineNode || prevHasInlineNode)

[dmazzoni, 2016/11/10 23:31:29]

I'm not sure we want no separator if either of them has an inline
node anywhere, that might be too aggressively sticking things
together with no whitespace. But I'm fine with landing this
and improving as we find such cases.

[David Tseng, 2016/11/11 02:36:27]

I found good examples of block <-> inline and inline <-> block elements that
look bad with separators. Namely, there's double spaces between them without
this code.

For example,
google chrome lnk  some more text


is what I see for an inline link and block static text.

+        separator = '';
+      else
+        separator = Output.SPACE;
+
+      prevHasInlineNode = hasInlineNode;
       result.append(separator);
       result.append(cur);
-      spansToExtend.forEach(function(elem) {
-        result.setSpan(
-            elem.span,
-            result.getSpanStart(elem.span),
-            elem.end);
-      });
-      spansToRemove.forEach(result.removeSpan.bind(result));
-      separator = Output.SPACE;
-    });
-    return result;
+      return result;
+    }, new Spannable());
   },
 
   /**
    * Find the earcon for a given node (including ancestry).
    * @param {!AutomationNode} node
    * @param {!AutomationNode=} opt_prevNode
    * @return {Output.Action}
    */
   findEarcon_: function(node, opt_prevNode) {
     if (node === opt_prevNode)
@@ skipping 44 matching lines @@
       prev.append(cur);
       return prev;
     }, null);
   },
 
   /**
    * Gets the output buffer for braille.
    * @return {!Spannable}
    */
   get brailleOutputForTest() {
-    return this.createBrailleOutput_();
+    return this.mergeBraille_(this.brailleBuffer_);
   }
 };
 
 });  // goog.scope