Improve accessible name calculation on Windows.

This depends on: http://codereview.chromium.org/10662003/

Based on the HTML to Platform Accessibility APIs Implementation Guide
(http://www.w3.org/TR/html-aapi/), add some new tests to assert the
correct behavior for computing the text alternative for a
checkbox, button, and text field, then fix the implementation to
match the test.

BUG=124314
TBR=dtseng

Review URL: https://chromiumcodereview.appspot.com/10823073

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@150070 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 208 insertions, 11 deletions

diff --git a/content/browser/accessibility/browser_accessibility_win.cc b/content/browser/accessibility/browser_accessibility_win.cc
index f1896ec..6874897 100644
--- a/content/browser/accessibility/browser_accessibility_win.cc
+++ b/content/browser/accessibility/browser_accessibility_win.cc
@@ -2726,6 +2726,80 @@ void BrowserAccessibilityWin::PreInitialize() {
     }
   }
 
+  // The calculation of the accessible name of an element has been
+  // standardized in the HTML to Platform Accessibility APIs Implementation
+  // Guide (http://www.w3.org/TR/html-aapi/). In order to return the
+  // appropriate accessible name on Windows, we need to apply some logic
+  // to the fields we get from WebKit.
+  //
+  // TODO(dmazzoni): move most of this logic into WebKit.
+  //
+  // WebKit gives us:
+  //
+  //   name: the default name, e.g. inner text
+  //   title ui element: a reference to a <label> element on the same
+  //       page that labels this node.
+  //   description: accessible labels that override the default name:
+  //       aria-label or aria-labelledby or aria-describedby
+  //   help: the value of the "title" attribute
+  //
+  // On Windows, the logic we apply lets some fields take precedence and
+  // always returns the primary name in "name" and the secondary name,
+  // if any, in "description".
+
+  string16 description, help, title_attr;
+  int title_elem_id = 0;
+  GetIntAttribute(AccessibilityNodeData::ATTR_TITLE_UI_ELEMENT, &title_elem_id);
+  GetStringAttribute(AccessibilityNodeData::ATTR_DESCRIPTION, &description);
+  GetStringAttribute(AccessibilityNodeData::ATTR_HELP, &help);
+
+  // WebKit annoyingly puts the title in the description if there's no other
+  // description, which just confuses the rest of the logic. Put it back.
+  // Now "help" is always the value of the "title" attribute, if present.
+  if (GetHtmlAttribute("title", &title_attr) &&
+      description == title_attr &&
+      help.empty()) {
+    help = description;
+    description = L"";
+    string_attributes_[AccessibilityNodeData::ATTR_DESCRIPTION] = L"";
+    string_attributes_[AccessibilityNodeData::ATTR_HELP] = help;
+  }
+
+  // Now implement the main logic: the descripion should become the name if
+  // it's nonempty, and the help should become the description if
+  // there's no description - or the name if there's no name or description.
+  if (!description.empty()) {
+    name_ = description;
+    description = L"";
+    string_attributes_[AccessibilityNodeData::ATTR_DESCRIPTION] = description;
+  }
+  if (!help.empty() && description.empty()) {
+    description = help;
+    string_attributes_[AccessibilityNodeData::ATTR_DESCRIPTION] = help;
+    string_attributes_[AccessibilityNodeData::ATTR_HELP] = L"";
+  }
+  if (!description.empty() && name_.empty() && !title_elem_id) {
+    name_ = description;
+    description = L"";
+    string_attributes_[AccessibilityNodeData::ATTR_DESCRIPTION] = L"";
+  }
+
+  // If it's a text field, also consider the placeholder.
+  string16 placeholder;
+  if (role_ == AccessibilityNodeData::ROLE_TEXT_FIELD &&
+      HasState(AccessibilityNodeData::STATE_FOCUSABLE) &&
+      GetHtmlAttribute("placeholder", &placeholder)) {
+    if (name_.empty() && !title_elem_id) {
+      name_ = placeholder;
+    } else if (description.empty()) {
+      description = placeholder;
+      string_attributes_[AccessibilityNodeData::ATTR_DESCRIPTION] = description;
+    }
+  }
+
+  // For certain roles (listbox option, static text, and list marker)
+  // WebKit stores the main accessible text in the "value" - swap it so
+  // that it's the "name".
   if (name_.empty() &&
       (role_ == AccessibilityNodeData::ROLE_LISTBOX_OPTION ||
        role_ == AccessibilityNodeData::ROLE_STATIC_TEXT ||
@@ -2733,12 +2807,6 @@ void BrowserAccessibilityWin::PreInitialize() {
     name_.swap(value_);
   }
 
-  // If this object doesn't have a name but it does have a description,
-  // use the description as its name - because some screen readers only
-  // announce the name.
-  if (name_.empty())
-    GetStringAttribute(AccessibilityNodeData::ATTR_DESCRIPTION, &name_);
-
   // If this doesn't have a value and is linked then set its value to the url
   // attribute. This allows screen readers to read an empty link's destination.
   string16 url;
@@ -2751,9 +2819,7 @@ void BrowserAccessibilityWin::PreInitialize() {
   relations_.clear();
 
   // Handle title UI element.
-  int title_elem_id;
-  if (GetIntAttribute(AccessibilityNodeData::ATTR_TITLE_UI_ELEMENT,
-                      &title_elem_id)) {
+  if (title_elem_id) {
     // Add a labelled by relationship.
     CComObject<BrowserAccessibilityRelation>* relation;
     HRESULT hr = CComObject<BrowserAccessibilityRelation>::CreateInstance(
diff --git a/content/browser/accessibility/dump_accessibility_tree_browsertest.cc b/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
index 74edad9..b5a4eeb 100644
--- a/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
+++ b/content/browser/accessibility/dump_accessibility_tree_browsertest.cc
@@ -240,6 +240,15 @@ IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityAWithImg) {
   RunTest(FILE_PATH_LITERAL("a-with-img.html"));
 }
 
+IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityButtonNameCalc) {
+  RunTest(FILE_PATH_LITERAL("button-name-calc.html"));
+}
+
+IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest,
+                       AccessibilityCheckboxNameCalc) {
+  RunTest(FILE_PATH_LITERAL("checkbox-name-calc.html"));
+}
+
 // TODO(dimich): Started to fail in Chrome r149732 (crbug 140397)
 #if defined(OS_WIN)
 #define MAYBE_AccessibilityContenteditableDescendants \
@@ -248,6 +257,7 @@ IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityAWithImg) {
 #define MAYBE_AccessibilityContenteditableDescendants \
     AccessibilityContenteditableDescendants
 #endif
+
 IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest,
                        MAYBE_AccessibilityContenteditableDescendants) {
   RunTest(FILE_PATH_LITERAL("contenteditable-descendants.html"));
@@ -257,6 +267,11 @@ IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityFooter) {
   RunTest(FILE_PATH_LITERAL("footer.html"));
 }
 
+IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest,
+                       AccessibilityInputTextNameCalc) {
+  RunTest(FILE_PATH_LITERAL("input-text-name-calc.html"));
+}
+
 IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityListMarkers) {
   RunTest(FILE_PATH_LITERAL("list-markers.html"));
 }
diff --git a/content/browser/accessibility/dump_accessibility_tree_helper.cc b/content/browser/accessibility/dump_accessibility_tree_helper.cc
index cd54007..187b0fa 100644
--- a/content/browser/accessibility/dump_accessibility_tree_helper.cc
+++ b/content/browser/accessibility/dump_accessibility_tree_helper.cc
@@ -10,6 +10,7 @@
 
 namespace {
 const int kIndentSpaces = 4;
+const char* kSkipString = "@NO_DUMP";
 }
 
 DumpAccessibilityTreeHelper::DumpAccessibilityTreeHelper() {
@@ -31,7 +32,11 @@ void DumpAccessibilityTreeHelper::RecursiveDumpAccessibilityTree(
     prefix[i] = ' ';
   prefix[indent] = '\0';
 
-  *contents += ToString(node, prefix.get());
+  string16 line = ToString(node, prefix.get());
+  if (line.find(ASCIIToUTF16(kSkipString)) != string16::npos)
+    return;
+
+  *contents += line;
   for (size_t i = 0; i < node->children().size(); ++i) {
     RecursiveDumpAccessibilityTree(node->children()[i], contents,
                                    indent + kIndentSpaces);
diff --git a/content/browser/accessibility/dump_accessibility_tree_helper_win.cc b/content/browser/accessibility/dump_accessibility_tree_helper_win.cc
index 6aae2be..df9ec76 100644
--- a/content/browser/accessibility/dump_accessibility_tree_helper_win.cc
+++ b/content/browser/accessibility/dump_accessibility_tree_helper_win.cc
@@ -42,10 +42,12 @@ string16 DumpAccessibilityTreeHelper::ToString(
   IAccessibleStateToStringVector(acc_obj->ia_state(), &state_strings);
   IAccessible2StateToStringVector(acc_obj->ia2_state(), &state_strings);
 
-  // Get the description and attributes.
+  // Get the description, help, and attributes.
   string16 description;
   acc_obj->GetStringAttribute(content::AccessibilityNodeData::ATTR_DESCRIPTION,
                               &description);
+  string16 help;
+  acc_obj->GetStringAttribute(content::AccessibilityNodeData::ATTR_HELP, &help);
   const std::vector<string16>& ia2_attributes = acc_obj->ia2_attributes();
 
   // Build the line.
diff --git a/content/test/data/accessibility/button-name-calc-expected-mac.txt b/content/test/data/accessibility/button-name-calc-expected-mac.txt
new file mode 100644
index 0000000..e929435
--- /dev/null
+++ b/content/test/data/accessibility/button-name-calc-expected-mac.txt
@@ -0,0 +1 @@
+#<skip -- need to validate/generate>
diff --git a/content/test/data/accessibility/button-name-calc-expected-win.txt b/content/test/data/accessibility/button-name-calc-expected-win.txt
new file mode 100644
index 0000000..4459150
--- /dev/null
+++ b/content/test/data/accessibility/button-name-calc-expected-win.txt
@@ -0,0 +1,8 @@
+ROLE_SYSTEM_DOCUMENT name='' READONLY FOCUSABLE description=''
+    IA2_ROLE_SECTION name='' READONLY description=''
+        ROLE_SYSTEM_PUSHBUTTON name='InnerText0' FOCUSABLE description=''
+        ROLE_SYSTEM_PUSHBUTTON name='InnerText1' FOCUSABLE description='Title1'
+        ROLE_SYSTEM_PUSHBUTTON name='AriaLabel2' FOCUSABLE description='Title2'
+        ROLE_SYSTEM_PUSHBUTTON name='LabelledBy3' FOCUSABLE description='Title3'
+        ROLE_SYSTEM_PUSHBUTTON name='LabelledBy4' FOCUSABLE description='DescribedBy4'
+        ROLE_SYSTEM_PUSHBUTTON name='InnerText5' FOCUSABLE description='DescribedBy5'
diff --git a/content/test/data/accessibility/button-name-calc.html b/content/test/data/accessibility/button-name-calc.html
new file mode 100644
index 0000000..f7a7c74c
--- /dev/null
+++ b/content/test/data/accessibility/button-name-calc.html
@@ -0,0 +1,22 @@
+<!--
+@WIN-ALLOW:description*
+-->
+<html>
+<body>
+  <button id="c0">InnerText0</button>
+  <button id="c1" title="Title1">InnerText1</button>
+  <button id="c2" title="Title2" aria-label="AriaLabel2">InnerText2</button>
+  <button id="c3" title="Title3" aria-label="AriaLabel3"
+         aria-labelledby="lb3">InnerText3</button>
+  <button id="c4" title="Title4" aria-label="AriaLabel4"
+         aria-labelledby="lb4" aria-describedby="db4">InnerText4</button>
+  <button id="c5" aria-describedby="db5">InnerText5</button>
+
+  <p aria-label="@NO_DUMP">
+    <span id="lb3">LabelledBy3</span>
+    <span id="lb4">LabelledBy4</span>
+    <span id="db4">DescribedBy4</span>
+    <span id="db5">DescribedBy5</span>
+  </p>
+</body>
+</html>
diff --git a/content/test/data/accessibility/checkbox-name-calc-expected-mac.txt b/content/test/data/accessibility/checkbox-name-calc-expected-mac.txt
new file mode 100644
index 0000000..e929435
--- /dev/null
+++ b/content/test/data/accessibility/checkbox-name-calc-expected-mac.txt
@@ -0,0 +1 @@
+#<skip -- need to validate/generate>
diff --git a/content/test/data/accessibility/checkbox-name-calc-expected-win.txt b/content/test/data/accessibility/checkbox-name-calc-expected-win.txt
new file mode 100644
index 0000000..0e31ace
--- /dev/null
+++ b/content/test/data/accessibility/checkbox-name-calc-expected-win.txt
@@ -0,0 +1,8 @@
+ROLE_SYSTEM_DOCUMENT name='' READONLY FOCUSABLE description=''
+    IA2_ROLE_SECTION name='' READONLY description=''
+        ROLE_SYSTEM_CHECKBUTTON name='Title0' FOCUSABLE description=''
+        ROLE_SYSTEM_CHECKBUTTON name='Label1' FOCUSABLE description='Title1'
+        ROLE_SYSTEM_CHECKBUTTON name='AriaLabel2' FOCUSABLE description='Title2'
+        ROLE_SYSTEM_CHECKBUTTON name='LabelledBy3' FOCUSABLE description='Title3'
+        ROLE_SYSTEM_CHECKBUTTON name='LabelledBy4' FOCUSABLE description='DescribedBy4'
+        ROLE_SYSTEM_CHECKBUTTON name='DescribedBy5' FOCUSABLE description=''
diff --git a/content/test/data/accessibility/checkbox-name-calc.html b/content/test/data/accessibility/checkbox-name-calc.html
new file mode 100644
index 0000000..54436dd
--- /dev/null
+++ b/content/test/data/accessibility/checkbox-name-calc.html
@@ -0,0 +1,26 @@
+<!--
+@WIN-ALLOW:description*
+-->
+<html>
+<body>
+  <input id="c0" type="checkbox" title="Title0">
+  <input id="c1" type="checkbox" title="Title1">
+  <input id="c2" type="checkbox" title="Title2" aria-label="AriaLabel2">
+  <input id="c3" type="checkbox" title="Title3" aria-label="AriaLabel3"
+         aria-labelledby="lb3">
+  <input id="c4" type="checkbox" title="Title4" aria-label="AriaLabel4"
+         aria-labelledby="lb4" aria-describedby="db4">
+  <input id="c5" type="checkbox" aria-describedby="db5">
+
+  <p aria-label="@NO_DUMP">
+    <label for="c1">Label1</label>
+    <label for="c2">Label2</label>
+    <label for="c3">Label3</label>
+    <label for="c4">Label4</label>
+    <span id="lb3">LabelledBy3</span>
+    <span id="lb4">LabelledBy4</span>
+    <span id="db4">DescribedBy4</span>
+    <span id="db5">DescribedBy5</span>
+  </p>
+</body>
+</html>
diff --git a/content/test/data/accessibility/input-text-name-calc-expected-mac.txt b/content/test/data/accessibility/input-text-name-calc-expected-mac.txt
new file mode 100644
index 0000000..e929435
--- /dev/null
+++ b/content/test/data/accessibility/input-text-name-calc-expected-mac.txt
@@ -0,0 +1 @@
+#<skip -- need to validate/generate>
diff --git a/content/test/data/accessibility/input-text-name-calc-expected-win.txt b/content/test/data/accessibility/input-text-name-calc-expected-win.txt
new file mode 100644
index 0000000..929910a
--- /dev/null
+++ b/content/test/data/accessibility/input-text-name-calc-expected-win.txt
@@ -0,0 +1,9 @@
+ROLE_SYSTEM_DOCUMENT name='' READONLY FOCUSABLE description=''
+    IA2_ROLE_SECTION name='' READONLY description=''
+        ROLE_SYSTEM_TEXT name='Title0' FOCUSABLE description=''
+        ROLE_SYSTEM_TEXT name='Label1' FOCUSABLE description='Title1'
+        ROLE_SYSTEM_TEXT name='AriaLabel2' FOCUSABLE description='Title2'
+        ROLE_SYSTEM_TEXT name='LabelledBy3' FOCUSABLE description='Title3'
+        ROLE_SYSTEM_TEXT name='Placeholder4' FOCUSABLE description=''
+        ROLE_SYSTEM_TEXT name='Title5' FOCUSABLE description='Placeholder5'
+        ROLE_SYSTEM_TEXT name='LabelledBy6' FOCUSABLE description='DescribedBy6'
diff --git a/content/test/data/accessibility/input-text-name-calc.html b/content/test/data/accessibility/input-text-name-calc.html
new file mode 100644
index 0000000..5843b8c
--- /dev/null
+++ b/content/test/data/accessibility/input-text-name-calc.html
@@ -0,0 +1,33 @@
+<!--
+@WIN-ALLOW:description*
+-->
+<html>
+<body>
+  <input id="c0" type="text" title="Title0">
+  <input id="c1" type="text" title="Title1">
+  <input id="c2" type="text" title="Title2" aria-label="AriaLabel2">
+  <input id="c3" type="text" title="Title3" aria-label="AriaLabel3"
+         aria-labelledby="lb3">
+
+  <input id="c4" type="text" placeholder="Placeholder4">
+  <input id="c5" type="text" placeholder="Placeholder5" title="Title5">
+
+  <input id="c6" type="text" title="Title6"
+         aria-label="AriaLabel6" aria-labelledby="lb6"
+         aria-describedby="db6">
+
+  <p aria-label="@NO_DUMP">
+    <label for="c1">Label1</label>
+    <label for="c2">Label2</label>
+    <label for="c3">Label3</label>
+
+    <label for="c6">Label6</label>
+
+    <span id="lb3">LabelledBy3</span>
+
+    <span id="lb6">LabelledBy6</span>
+
+    <span id="db6">DescribedBy6</span>
+  </p>
+</body>
+</html>