From bbe2760c43e146fe15db2d25f983b1297295aa0e Mon Sep 17 00:00:00 2001
From: plundblad <plundblad@chromium.org>
Date: Thu, 30 Jul 2015 03:27:27 -0700
Subject: [PATCH] Fix some speech and braille output formatting in chromevox
 next.

This fixes the output for check boxes, menu items and visited links for
braille and adds support for locale aware plural handling to the formatting
language.

R=dtseng@chromium.org
BUG=None

Review URL: https://codereview.chromium.org/1269513002

Cr-Commit-Position: refs/heads/master@{#341092}
---
 .../chromeos/chromevox/common/braille_util.js      | 12 +--
 .../chromeos/chromevox/common/dom_util.js          |  2 +-
 .../chromeos/chromevox/cvox2/background/output.js  | 90 ++++++++++++++--------
 .../chromevox/cvox2/background/output_test.extjs   |  6 +-
 .../chromevox/strings/chromevox_strings.grd        | 24 +++---
 5 files changed, 83 insertions(+), 51 deletions(-)

diff --git a/chrome/browser/resources/chromeos/chromevox/common/braille_util.js b/chrome/browser/resources/chromeos/chromevox/common/braille_util.js
index 8c673da791c8..9ab921b0d228 100644
--- a/chrome/browser/resources/chromeos/chromevox/common/braille_util.js
+++ b/chrome/browser/resources/chromeos/chromevox/common/braille_util.js
@@ -60,17 +60,17 @@ cvox.BrailleUtil.TEMPLATE = {
   'base': 'c n v r s',
   'aria_role_alert': 'r: n',
   'aria_role_button': 'n r s',
-  'aria_role_checkbox': 'n r (s)',
-  'aria_role_menuitemcheckbox': 'n r (s)',
-  'aria_role_menuitemradio': 'n r (s)',
-  'aria_role_radio': 'n r (s)',
+  'aria_role_checkbox': 'n r s',
+  'aria_role_menuitemcheckbox': 'n r s',
+  'aria_role_menuitemradio': 'n r s',
+  'aria_role_radio': 'n r s',
   'aria_role_textbox': 'n: v r s',
   'input_type_button': 'n r s',
-  'input_type_checkbox': 'n r (s)',
+  'input_type_checkbox': 'n r s',
   'input_type_email': 'n: v r s',
   'input_type_number': 'n: v r s',
   'input_type_password': 'n: v r s',
-  'input_type_radio': 'n r (s)',
+  'input_type_radio': 'n r s',
   'input_type_search': 'n: v r s',
   'input_type_submit': 'n r s',
   'input_type_text': 'n: v r s',
diff --git a/chrome/browser/resources/chromeos/chromevox/common/dom_util.js b/chrome/browser/resources/chromeos/chromevox/common/dom_util.js
index ac2ba476e527..474f42adacb8 100644
--- a/chrome/browser/resources/chromeos/chromevox/common/dom_util.js
+++ b/chrome/browser/resources/chromeos/chromevox/common/dom_util.js
@@ -1390,7 +1390,7 @@ cvox.DomUtil.getStateMsgs = function(targetNode, primary) {
   } else if (targetNode.tagName == 'UL' ||
              targetNode.tagName == 'OL' ||
              role == 'list') {
-    info.push(['list_with_items',
+    info.push(['list_with_items_not_pluralized',
                cvox.ChromeVox.msgs.getNumber(
                    cvox.DomUtil.getListLength(targetNode))]);
   }
diff --git a/chrome/browser/resources/chromeos/chromevox/cvox2/background/output.js b/chrome/browser/resources/chromeos/chromevox/cvox2/background/output.js
index 8179d5b6135d..08b5d92da09e 100644
--- a/chrome/browser/resources/chromeos/chromevox/cvox2/background/output.js
+++ b/chrome/browser/resources/chromeos/chromevox/cvox2/background/output.js
@@ -18,6 +18,7 @@ goog.require('cvox.NavBraille');
 goog.require('cvox.Spannable');
 goog.require('cvox.ValueSelectionSpan');
 goog.require('cvox.ValueSpan');
+goog.require('goog.i18n.MessageFormat');
 
 goog.scope(function() {
 var Dir = AutomationUtil.Dir;
@@ -37,6 +38,12 @@ var Dir = AutomationUtil.Dir;
  * @ prefix: used to substitute a message. Note the ability to specify params to
  *     the message.  For example, '@tag_html' '@selected_index($text_sel_start,
  *     $text_sel_end').
+ * @@ prefix: similar to @, used to substitue a message, but also pulls the
+ *     localized string through goog.i18n.MessageFormat to support locale
+ *     aware plural handling.  The first argument should be a number which will
+ *     be passed as a COUNT named parameter to MessageFormat.
+ *     TODO(plundblad): Make subsequent arguments normal placeholder arguments
+ *     when needed.
  * = suffix: used to specify substitution only if not previously appended.
  *     For example, $name= would insert the name attribute only if no name
  * attribute had been inserted previously.
@@ -384,15 +391,15 @@ Output.RULES = {
       speak: '$value='
     },
     link: {
-      enter: '$name $visited $role',
-      stay: '$name= $visited $role',
-      speak: '$name= $visited $role'
+      enter: '$name $if($visited, @visited_link, $role)',
+      stay: '$name= $if($visited, @visited_link, $role)',
+      speak: '$name= $if($visited, @visited_link, $role)'
     },
     list: {
-      enter: '$role @list_with_items($countChildren(listItem))'
+      enter: '$role @@list_with_items($countChildren(listItem))'
     },
     listBox: {
-      enter: '$name $role @list_with_items($countChildren(listBoxOption))'
+      enter: '$name $role @@list_with_items($countChildren(listBoxOption))'
     },
     listBoxOption: {
       speak: '$name $role @describe_index($indexInParent, $parentChildCount)'
@@ -401,11 +408,10 @@ Output.RULES = {
       enter: '$role'
     },
     menu: {
-      enter: '$name $role @list_with_items($countChildren(menuItem))'
+      enter: '$name $role @@list_with_items($countChildren(menuItem))'
     },
     menuItem: {
-      speak: '$if($haspopup, @describe_menu_item_with_submenu($name), ' +
-          '@describe_menu_item($name)) ' +
+      speak: '$name $role $if($haspopup, @has_submenu) ' +
           '@describe_index($indexInParent, $parentChildCount)'
     },
     menuListOption: {
@@ -447,12 +453,12 @@ Output.RULES = {
       enter: '$name $role'
     },
     tree: {
-      enter: '$name $role @list_with_items($countChildren(treeItem))'
+      enter: '$name $role @@list_with_items($countChildren(treeItem))'
     },
     treeItem: {
-        enter: '$role $expanded $collapsed ' +
-            '@describe_index($indexInParent, $parentChildCount) ' +
-            '@describe_depth($hierarchicalLevel)'
+      enter: '$role $expanded $collapsed ' +
+          '@describe_index($indexInParent, $parentChildCount) ' +
+          '@describe_depth($hierarchicalLevel)'
     },
     window: {
       enter: '$name',
@@ -677,10 +683,8 @@ Output.prototype = {
           /** @type {number} */ (buff.getSpanEnd(selSpan));
       startIndex = valueStart + selSpan.startIndex;
       endIndex = valueStart + selSpan.endIndex;
-        buff.setSpan(new cvox.ValueSpan(0),
-                                  valueStart, valueEnd);
-      buff.setSpan(new cvox.ValueSelectionSpan(),
-                                  startIndex, endIndex);
+      buff.setSpan(new cvox.ValueSpan(0), valueStart, valueEnd);
+      buff.setSpan(new cvox.ValueSelectionSpan(), startIndex, endIndex);
     }
 
     var output = new cvox.NavBraille({
@@ -809,7 +813,7 @@ Output.prototype = {
         } else if (token == 'parentChildCount') {
           options.annotation.push(token);
           if (node.parent)
-          this.append_(buff, String(node.parent.children.length));
+            this.append_(buff, String(node.parent.children.length));
         } else if (token == 'state') {
           options.annotation.push(token);
           Object.getOwnPropertyNames(node.state).forEach(function(s) {
@@ -916,6 +920,9 @@ Output.prototype = {
           }
         }
       } else if (prefix == '@') {
+        var isPluralized = (token[0] == '@');
+        if (isPluralized)
+          token = token.slice(1);
         // Tokens can have substitutions.
         var pieces = token.split('+');
         token = pieces.reduce(function(prev, cur) {
@@ -926,28 +933,47 @@ Output.prototype = {
         }.bind(this), '');
         var msgId = token;
         var msgArgs = [];
-        var curMsg = tree.firstChild;
-
-        while (curMsg) {
-          var arg = curMsg.value;
-          if (arg[0] != '$') {
-            console.error('Unexpected value: ' + arg);
-            return;
+        if (!isPluralized) {
+          var curArg = tree.firstChild;
+          while (curArg) {
+            if (curArg.value[0] != '$') {
+              console.error('Unexpected value: ' + curArg.value);
+              return;
+            }
+            var msgBuff = [];
+            this.format_(node, curArg, msgBuff);
+            msgArgs = msgArgs.concat(msgBuff);
+            curArg = curArg.nextSibling;
           }
-          var msgBuff = [];
-          this.format_(node, curMsg, msgBuff);
-          msgArgs = msgArgs.concat(msgBuff);
-          curMsg = curMsg.nextSibling;
         }
-          var msg = cvox.ChromeVox.msgs.getMsg(msgId, msgArgs);
+        var msg = cvox.ChromeVox.msgs.getMsg(msgId, msgArgs);
         try {
           if (this.formatOptions_.braille)
             msg = cvox.ChromeVox.msgs.getMsg(msgId + '_brl', msgArgs) || msg;
         } catch(e) {}
 
-        if (msg) {
-          this.append_(buff, msg, options);
+        if (!msg) {
+          console.log('Could not get message ' + msgId);
+          return;
+        }
+
+        if (isPluralized) {
+          var arg = tree.firstChild;
+          if (!arg || arg.nextSibling) {
+            console.error('Pluralized messages take exactly one argument');
+            return;
+          }
+          if (arg.value[0] != '$') {
+            console.error('Unexpected value: ' + arg.value);
+            return;
+          }
+          var argBuff = [];
+          this.format_(node, arg, argBuff);
+          var namedArgs = {COUNT: Number(argBuff[0])};
+          msg = new goog.i18n.MessageFormat(msg).format(namedArgs);
         }
+
+        this.append_(buff, msg, options);
       } else if (prefix == '!') {
         this.speechProperties_[token] = true;
       }
@@ -979,7 +1005,7 @@ Output.prototype = {
 
     while (cursor.getNode() != range.getEnd().getNode()) {
       var node = cursor.getNode();
-        rangeBuff.push.apply(rangeBuff, formatNodeAndAncestors(node, prevNode));
+      rangeBuff.push.apply(rangeBuff, formatNodeAndAncestors(node, prevNode));
       prevNode = node;
       cursor = cursor.move(cursors.Unit.NODE,
                            cursors.Movement.DIRECTIONAL,
diff --git a/chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs b/chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs
index 157f6cfc955c..290da22a52d3 100644
--- a/chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs
+++ b/chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs
@@ -327,9 +327,9 @@ TEST_F('OutputE2ETest', 'Menu', function() {
     var range = cursors.Range.fromNode(el);
     var o = new Output().withSpeechAndBraille(range, null, 'navigate');
     assertEqualsJSON({string_:
-            '|Menu|with 1 items|a, menu item| 1 of 1 ', spans_: [
-        {value: 'name', start: 0, end: 0},
-        {value: 'role', start:1, end: 5}
+            '|Menu|with 1 item|a|Menu item| 1 of 1 ', spans_: [
+        {value: 'name', start: 18, end: 19},
+        {value: 'role', start:20, end: 29}
     ]}, o.toSpannableForTest());
   });
 });
diff --git a/chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd b/chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd
index 3a0578eb6469..fb6fb2e21fc9 100644
--- a/chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd
+++ b/chrome/browser/resources/chromeos/chromevox/strings/chromevox_strings.grd
@@ -543,7 +543,7 @@
         checked
       </message>
       <message desc="The checked state for a checkbox in braille." name="IDS_CHROMEVOX_CHECKBOX_CHECKED_STATE_BRL">
-        x
+        (x)
       </message>
       <message desc="Describes a HTML checkbox named 'name' in the unchecked state." name="IDS_CHROMEVOX_DESCRIBE_CHECKBOX_UNCHECKED">
         <ph name="name">$1</ph>, checkbox not checked
@@ -552,7 +552,7 @@
         not checked
       </message>
       <message desc="The unchecked state for a checkbox in braille." name="IDS_CHROMEVOX_CHECKBOX_UNCHECKED_STATE_BRL">
-        ''' '''
+        ( )
       </message>
       <message desc="Describes a HTML radio button named 'name' in the selected state." name="IDS_CHROMEVOX_DESCRIBE_RADIO_SELECTED">
         <ph name="name">$1</ph>, radio button selected
@@ -561,7 +561,7 @@
         selected
       </message>
       <message desc="The selected state for a radio button in braille." name="IDS_CHROMEVOX_RADIO_SELECTED_STATE_BRL">
-        x
+        (x)
       </message>
       <message desc="Describes a HTML radio button named 'name' in the unselected state." name="IDS_CHROMEVOX_DESCRIBE_RADIO_UNSELECTED">
         <ph name="name">$1</ph>, radio button unselected
@@ -570,7 +570,7 @@
         unselected
       </message>
       <message desc="The unselected state for a radio button in braille." name="IDS_CHROMEVOX_RADIO_UNSELECTED_STATE_BRL">
-        ''' '''
+        ( )
       </message>
       <message desc="Describes a menu named 'name'." name="IDS_CHROMEVOX_DESCRIBE_MENU">
         <ph name="name">$1</ph>, menu
@@ -629,6 +629,9 @@
       <message desc="Spoken through the a11y api after describing an element if it is part of a group." name="IDS_CHROMEVOX_DESCRIBE_INDEX">
         ''' <ph name="index">$1</ph> of <ph name="total">$2</ph> '''
       </message>
+      <message desc="Braille for describing an index of an element in a group." name="IDS_CHROMEVOX_DESCRIBE_INDEX_BRL">
+        <ph name="index">$1</ph>/<ph name="total">$2</ph>
+      </message>
       <message desc="Spoken through the a11y api when moving between treeitems of differing depth." name="IDS_CHROMEVOX_DESCRIBE_DEPTH">
         ''' level <ph name="depth">$1</ph> '''
       </message>
@@ -1365,13 +1368,13 @@
         Not checked
       </message>
       <message desc="Braille of element with the ARIA attribute aria-checked=false." name="IDS_CHROMEVOX_ARIA_CHECKED_FALSE_BRL">
-        ''' '''
+        ( )
       </message>
       <message desc="Describes an element with the ARIA attribute aria-checked=mixed." name="IDS_CHROMEVOX_ARIA_CHECKED_MIXED">
         Partially checked
       </message>
       <message desc="Braille of element with the ARIA attribute aria-checked=mixed." name="IDS_CHROMEVOX_ARIA_CHECKED_MIXED_BRL">
-        -
+        (-)
       </message>
       <message desc="Describes an element with the ARIA attribute aria-disabled=true." name="IDS_CHROMEVOX_ARIA_DISABLED_TRUE">
         Disabled
@@ -1455,13 +1458,13 @@
         Selected
       </message>
       <message desc="Braille of element with the ARIA attribute aria-selected=true." name="IDS_CHROMEVOX_ARIA_SELECTED_TRUE_BRL">
-        x
+        (x)
       </message>
       <message desc="Describes an element with the ARIA attribute aria-selected=false." name="IDS_CHROMEVOX_ARIA_SELECTED_FALSE">
         Not selected
       </message>
       <message desc="Braille of element with the ARIA attribute aria-selected=false." name="IDS_CHROMEVOX_ARIA_SELECTED_FALSE_BRL">
-        ''' '''
+        ( )
       </message>
       <message desc="Spoken to describe an &lt;a&gt; tag." name="IDS_CHROMEVOX_TAG_LINK">
         Link
@@ -1763,9 +1766,12 @@
       <message desc="In an editable text box, describes a line with only whitespace." name="IDS_CHROMEVOX_TEXT_BOX_WHITESPACE">
         Space
       </message>
-      <message desc="Further describes a list-like element with a number of items. e.g. This will be combined with other messages to produce: List with 3 items. NOTE(deboer):  This is questionable, it may be better to include 'List' in this message." name="IDS_CHROMEVOX_LIST_WITH_ITEMS">
+      <message desc="Further describes a list-like element with a number of items. e.g. This will be combined with other messages to produce: List with 3 items." name="IDS_CHROMEVOX_LIST_WITH_ITEMS_NOT_PLURALIZED">
         with <ph name="num">$1</ph> items
       </message>
+      <message desc="Further describes a list-like element with a number of items. e.g. This will be combined with other messages to produce: List with 3 items." name="IDS_CHROMEVOX_LIST_WITH_ITEMS">
+        with {COUNT, plural, =1 {# item}other {# items}}
+      </message>
       <message desc="Further describes a list-like element with a number of items in braille." name="IDS_CHROMEVOX_LIST_WITH_ITEMS_BRL">
         +<ph name="num">$1</ph>
       </message>
-- 
2.11.4.GIT