Text controls should expose their value instead of their name via IAccessibleText

content/browser/accessibility/browser_accessibility_win.cc

 // Copyright (c) 2012 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.
 
 #include "content/browser/accessibility/browser_accessibility_win.h"
 
 #include <UIAutomationClient.h>
 #include <UIAutomationCoreApi.h>
 
 #include <algorithm>
@@ skipping 1984 matching lines @@
 // IAccessibleText methods.
 //
 
 STDMETHODIMP BrowserAccessibilityWin::get_nCharacters(LONG* n_characters) {
   if (!instance_active())
     return E_FAIL;
 
   if (!n_characters)
     return E_INVALIDARG;
 
-  *n_characters = TextForIAccessibleText().length();
+  *n_characters = hypertext().length();
   return S_OK;
 }
 
 STDMETHODIMP BrowserAccessibilityWin::get_caretOffset(LONG* offset) {
   if (!instance_active())
     return E_FAIL;
 
   if (!offset)
     return E_INVALIDARG;
 
@@ skipping 16 matching lines @@
     LONG* out_x,
     LONG* out_y,
     LONG* out_width,
     LONG* out_height) {
   if (!instance_active())
     return E_FAIL;
 
   if (!out_x || !out_y || !out_width || !out_height)
     return E_INVALIDARG;
 
-  const base::string16& text_str = TextForIAccessibleText();
+  const base::string16& text_str = hypertext();
   HandleSpecialTextOffset(text_str, &offset);
 
   if (offset < 0 || offset > static_cast<LONG>(text_str.size()))
     return E_INVALIDARG;
 
   gfx::Rect character_bounds;
   if (coordinate_type == IA2_COORDTYPE_SCREEN_RELATIVE) {
     character_bounds = GetGlobalBoundsForRange(offset, 1);
   } else if (coordinate_type == IA2_COORDTYPE_PARENT_RELATIVE) {
     character_bounds = GetLocalBoundsForRange(offset, 1);
@@ skipping 62 matching lines @@
 
 STDMETHODIMP BrowserAccessibilityWin::get_text(LONG start_offset,
                                                LONG end_offset,
                                                BSTR* text) {
   if (!instance_active())
     return E_FAIL;
 
   if (!text)
     return E_INVALIDARG;
 
-  const base::string16& text_str = TextForIAccessibleText();
+  const base::string16& text_str = hypertext();
 
   // Handle special text offsets.
   HandleSpecialTextOffset(text_str, &start_offset);
   HandleSpecialTextOffset(text_str, &end_offset);
 
   // The spec allows the arguments to be reversed.
   if (start_offset > end_offset) {
     LONG tmp = start_offset;
     start_offset = end_offset;
     end_offset = tmp;
@@ skipping 22 matching lines @@
     enum IA2TextBoundaryType boundary_type,
     LONG* start_offset,
     LONG* end_offset,
     BSTR* text) {
   if (!instance_active())
     return E_FAIL;
 
   if (!start_offset || !end_offset || !text)
     return E_INVALIDARG;
 
-  const base::string16& text_str = TextForIAccessibleText();
+  const base::string16& text_str = hypertext();
   HandleSpecialTextOffset(text_str, &offset);
   if (offset < 0)
     return E_INVALIDARG;
 
   LONG text_len = text_str.length();
   if (offset > text_len)
     return E_INVALIDARG;
 
   // The IAccessible2 spec says we don't have to implement the "sentence"
   // boundary type, we can just let the screenreader handle it.
@@ skipping 34 matching lines @@
 
   // The IAccessible2 spec says we don't have to implement the "sentence"
   // boundary type, we can just let the screenreader handle it.
   if (boundary_type == IA2_TEXT_BOUNDARY_SENTENCE) {
     *start_offset = 0;
     *end_offset = 0;
     *text = NULL;
     return S_FALSE;
   }
 
-  const base::string16& text_str = TextForIAccessibleText();
+  const base::string16& text_str = hypertext();
 
   *start_offset = FindBoundary(
       text_str, boundary_type, offset, ui::BACKWARDS_DIRECTION);
   *end_offset = offset;
   return get_text(*start_offset, *end_offset, text);
 }
 
 STDMETHODIMP BrowserAccessibilityWin::get_textAfterOffset(
     LONG offset,
     enum IA2TextBoundaryType boundary_type,
     LONG* start_offset,
     LONG* end_offset,
     BSTR* text) {
   if (!instance_active())
     return E_FAIL;
 
   if (!start_offset || !end_offset || !text)
     return E_INVALIDARG;
 
   // The IAccessible2 spec says we don't have to implement the "sentence"
   // boundary type, we can just let the screenreader handle it.
   if (boundary_type == IA2_TEXT_BOUNDARY_SENTENCE) {
     *start_offset = 0;
     *end_offset = 0;
     *text = NULL;
     return S_FALSE;
   }
 
-  const base::string16& text_str = TextForIAccessibleText();
+  const base::string16& text_str = hypertext();
 
   *start_offset = offset;
   *end_offset = FindBoundary(
       text_str, boundary_type, offset, ui::FORWARDS_DIRECTION);
   return get_text(*start_offset, *end_offset, text);
 }
 
 STDMETHODIMP BrowserAccessibilityWin::get_newText(IA2TextSegment* new_text) {
   if (!instance_active())
     return E_FAIL;
@@ skipping 72 matching lines @@
     LONG x, LONG y) {
   // TODO(dmazzoni): adjust this for the start and end index, too.
   return scrollToPoint(coordinate_type, x, y);
 }
 
 STDMETHODIMP BrowserAccessibilityWin::addSelection(LONG start_offset,
                                                    LONG end_offset) {
   if (!instance_active())
     return E_FAIL;
 
-  const base::string16& text_str = TextForIAccessibleText();
+  const base::string16& text_str = hypertext();
   HandleSpecialTextOffset(text_str, &start_offset);
   HandleSpecialTextOffset(text_str, &end_offset);
 
   manager()->SetTextSelection(*this, start_offset, end_offset);
   return S_OK;
 }
 
 STDMETHODIMP BrowserAccessibilityWin::removeSelection(LONG selection_index) {
   if (!instance_active())
     return E_FAIL;
 
   if (selection_index != 0)
     return E_INVALIDARG;
 
   manager()->SetTextSelection(*this, 0, 0);
   return S_OK;
 }
 
 STDMETHODIMP BrowserAccessibilityWin::setCaretOffset(LONG offset) {
   if (!instance_active())
     return E_FAIL;
 
-  const base::string16& text_str = TextForIAccessibleText();
+  const base::string16& text_str = hypertext();
   HandleSpecialTextOffset(text_str, &offset);
   manager()->SetTextSelection(*this, offset, offset);
   return S_OK;
 }
 
 STDMETHODIMP BrowserAccessibilityWin::setSelection(LONG selection_index,
                                                    LONG start_offset,
                                                    LONG end_offset) {
   if (!instance_active())
     return E_FAIL;
 
   if (selection_index != 0)
     return E_INVALIDARG;
 
-  const base::string16& text_str = TextForIAccessibleText();
+  const base::string16& text_str = hypertext();
   HandleSpecialTextOffset(text_str, &start_offset);
   HandleSpecialTextOffset(text_str, &end_offset);
 
   manager()->SetTextSelection(*this, start_offset, end_offset);
   return S_OK;
 }
 
 //
 // IAccessibleText methods not implemented.
 //
@@ skipping 74 matching lines @@
 
 // Currently, only text links are supported.
 STDMETHODIMP BrowserAccessibilityWin::get_anchor(long index, VARIANT* anchor) {
   if (!instance_active() || !IsHyperlink())
     return E_FAIL;
 
   // IA2 text links can have only one anchor, that is the text inside them.
   if (index != 0 || !anchor)
     return E_INVALIDARG;
 
-  BSTR hypertext = SysAllocString(TextForIAccessibleText().c_str());
-  DCHECK(hypertext);
+  BSTR ia2_hypertext = SysAllocString(hypertext().c_str());
+  DCHECK(ia2_hypertext);
   anchor->vt = VT_BSTR;
-  anchor->bstrVal = hypertext;
+  anchor->bstrVal = ia2_hypertext;
 
   // Returning S_FALSE is not mentioned in the IA2 Spec, but it might have been
   // an oversight.
-  if (!SysStringLen(hypertext))
+  if (!SysStringLen(ia2_hypertext))
     return S_FALSE;
 
   return S_OK;
 }
 
 // Currently, only text links are supported.
 STDMETHODIMP BrowserAccessibilityWin::get_anchorTarget(long index,
                                                        VARIANT* anchor_target) {
   if (!instance_active() || !IsHyperlink())
     return E_FAIL;
@@ skipping 554 matching lines @@
     int* out_x,
     int* out_y,
     int* out_width,
     int* out_height) {
   if (!instance_active())
     return E_FAIL;
 
   if (!out_x || !out_y || !out_width || !out_height)
     return E_INVALIDARG;
 
-  const base::string16& text_str = TextForIAccessibleText();
+  const base::string16& text_str = hypertext();
   if (start_index > text_str.size() ||
       end_index > text_str.size() ||
       start_index > end_index) {
     return E_INVALIDARG;
   }
 
   gfx::Rect bounds = GetGlobalBoundsForRange(
       start_index, end_index - start_index);
   *out_x = bounds.x();
   *out_y = bounds.y();
   *out_width = bounds.width();
   *out_height = bounds.height();
   return S_OK;
 }
 
 STDMETHODIMP BrowserAccessibilityWin::scrollToSubstring(
     unsigned int start_index,
     unsigned int end_index) {
   if (!instance_active())
     return E_FAIL;
 
-  const base::string16& text_str = TextForIAccessibleText();
+  const base::string16& text_str = hypertext();
   if (start_index > text_str.size() ||
       end_index > text_str.size() ||
       start_index > end_index) {
     return E_INVALIDARG;
   }
 
   manager()->ScrollToMakeVisible(*this, GetLocalBoundsForRange(
       start_index, end_index - start_index));
   manager()->ToBrowserAccessibilityManagerWin()->TrackScrollingObject(this);
 
@@ skipping 428 matching lines @@
   // something other than DOCUMENT so that the fact that it's a separate doc
   // is not exposed to AT.
   if (IsWebAreaForPresentationalIframe()) {
     win_attributes_->ia_role = ROLE_SYSTEM_GROUPING;
     win_attributes_->ia2_role = ROLE_SYSTEM_GROUPING;
   }
 }
 
 void BrowserAccessibilityWin::UpdateStep2ComputeHypertext() {
   if (!PlatformChildCount()) {
-    win_attributes_->hypertext += name();
+    if (IsControlWithValue())

[dmazzoni, 2015/10/29 20:15:29]

I like the idea behind this change, but I don't think IsControlWithValue is the
correct semantics.

Only a *text* control should expose its value via IAccessibleText.

So a combo box or search box role, absolutely - that's a valid fix. Maybe a
couple of others.

A slider or listbox? No, I don't buy it. You can't arrow through the characters
in the values of those roles, so it doesn't make sense to expose anything via
IAccessibleText for those controls. We already have accValue for those.

+      win_attributes_->hypertext += value();
+    else
+      win_attributes_->hypertext += name();
+
     return;
   }
 
   // Construct the hypertext for this node, which contains the concatenation
   // of all of the static text and widespace of this node's children and an
   // embedded object character for all the other children. Build up a map from
   // the character index of each embedded object character to the id of the
   // child object it points to.
   for (unsigned int i = 0; i < PlatformChildCount(); ++i) {
     BrowserAccessibilityWin* child =
         PlatformGetChild(i)->ToBrowserAccessibilityWin();
     DCHECK(child);
+    // Similar to Firefox, we don't expose text-only objects in IA2 hypertext.
     if (child->IsTextOnlyObject()) {
       win_attributes_->hypertext += child->name();
     } else {
       int32 char_offset = hypertext().size();
       int32 child_id = child->GetId();
       int32 index = hyperlinks().size();
       win_attributes_->hyperlink_offset_to_index[char_offset] = index;
       win_attributes_->hyperlinks.push_back(child_id);
       win_attributes_->hypertext += kEmbeddedCharacter;
     }
@@ skipping 254 matching lines @@
   if (child.IsTextOnlyObject()) {
     int32 hypertextOffset = 0;
     int32 index_in_parent = child.GetIndexInParent();
     DCHECK_GE(index_in_parent, 0);
     DCHECK_LT(index_in_parent, static_cast<int32>(PlatformChildCount()));
     for (uint32 i = 0; i < static_cast<uint32>(index_in_parent); ++i) {
       const BrowserAccessibilityWin* sibling =
           PlatformGetChild(i)->ToBrowserAccessibilityWin();
       DCHECK(sibling);
       if (sibling->IsTextOnlyObject())
-        hypertextOffset += sibling->TextForIAccessibleText().length();
+        hypertextOffset += sibling->hypertext().length();
       else
         ++hypertextOffset;
     }
     return hypertextOffset;
   }
 
   int32 hyperlink_index = GetHyperlinkIndexFromChild(child);
   if (hyperlink_index < 0)
     return -1;
 
@@ skipping 71 matching lines @@
     if (endpoint_object.IsDescendantOf(child)) {
       endpoint_index_in_common_parent = child->GetIndexInParent();
       break;
     }
   }
   DCHECK_GE(endpoint_index_in_common_parent, 0);
 
   if (endpoint_index_in_common_parent < index_in_common_parent)
     return 0;
   if (endpoint_index_in_common_parent > index_in_common_parent)
-    return TextForIAccessibleText().length();
+    return hypertext().length();
 
   NOTREACHED();
   return -1;
 }
 
 int BrowserAccessibilityWin::GetSelectionAnchor() const {
   int32 anchor_id = manager()->GetTreeData().sel_anchor_object_id;
   BrowserAccessibilityWin* anchor_object = manager()->GetFromID(
       anchor_id)->ToBrowserAccessibilityWin();
   if (!anchor_object)
@@ skipping 93 matching lines @@
   float fval;
   base::string16 value = this->value();
 
   if (value.empty() &&
       GetFloatAttribute(ui::AX_ATTR_VALUE_FOR_RANGE, &fval)) {
     value = base::UTF8ToUTF16(base::DoubleToString(fval));
   }
   return value;
 }
 
-base::string16 BrowserAccessibilityWin::TextForIAccessibleText() const {
-  switch (GetRole()) {
-    case ui::AX_ROLE_TEXT_FIELD:
-    case ui::AX_ROLE_MENU_LIST_OPTION:
-      return value();
-    default:
-      return hypertext();
-  }
-}
-
 bool BrowserAccessibilityWin::IsSameHypertextCharacter(size_t old_char_index,
                                                        size_t new_char_index) {
   CHECK(old_win_attributes_);
 
   // For anything other than the "embedded character", we just compare the
   // characters directly.
   base::char16 old_ch = old_win_attributes_->hypertext[old_char_index];
   base::char16 new_ch = win_attributes_->hypertext[new_char_index];
   if (old_ch != new_ch)
     return false;
@@ skipping 740 matching lines @@
     ia2_role = ia_role;
 
   win_attributes_->ia_role = ia_role;
   win_attributes_->ia_state = ia_state;
   win_attributes_->role_name = role_name;
   win_attributes_->ia2_role = ia2_role;
   win_attributes_->ia2_state = ia2_state;
 }
 
 }  // namespace content