Support IAccessibleHypertext.

content/browser/accessibility/browser_accessibility_win.cc

Index: content/browser/accessibility/browser_accessibility_win.cc
diff --git a/content/browser/accessibility/browser_accessibility_win.cc b/content/browser/accessibility/browser_accessibility_win.cc
index 7be2664a02702c296276d210fcf0a0a2615275c4..77098368a0e61f64493b0962e057c6fa43f295f2 100644
--- a/content/browser/accessibility/browser_accessibility_win.cc
+++ b/content/browser/accessibility/browser_accessibility_win.cc
@@ -1604,7 +1604,7 @@ STDMETHODIMP BrowserAccessibilityWin::get_nCharacters(LONG* n_characters) {
       role_ == WebAccessibility::ROLE_TEXTAREA) {
     *n_characters = value_.length();
   } else {
-    *n_characters = name_.length();
+    *n_characters = hypertext_.length();

[dmazzoni, 2011/10/28 17:33:53]

Can this method use TextForIAccessibleText instead?

[David Tseng, 2011/10/28 18:25:15]

Done.

   }
 
   return S_OK;
@@ -1849,6 +1849,51 @@ STDMETHODIMP BrowserAccessibilityWin::get_offsetAtPoint(
 }
 
 //
+// IAccessibleHypertext methods.
+//
+
+STDMETHODIMP BrowserAccessibilityWin::get_nHyperlinks(long* hyperlink_count) {
+  if (!instance_active_)
+    return E_FAIL;
+
+  *hyperlink_count = hyperlink_offset_to_index_.size();

[dmazzoni, 2011/10/28 17:33:53]

Need to check all pointer arguments and return E_INVALIDARG if they're null.

[David Tseng, 2011/10/28 18:25:15]

Done.

+  return S_OK;
+}
+
+STDMETHODIMP BrowserAccessibilityWin::get_hyperlink(
+    long index,
+    IAccessibleHyperlink** hyperlink) {
+  if (!instance_active_)
+    return E_FAIL;
+
+  if (index < 0 || index >= static_cast<long>(children().size()) || !hyperlink)

[dmazzoni, 2011/10/28 17:33:53]

You only have an embedded object character for non-static-text children, right?
So does it make sense for this to support getting hyperlinks to static text
children?

Instead of accessing children, could you access hyperlink_offset_to_index_ here?

[David Tseng, 2011/10/28 18:25:15]

The documentation states that clients are supposed to first call
get_hyperlinkIndex to get the hyperlink index corresponding to a provided
offset. That index is then passed into this method.

I guess I could just return the offset as the index in get_hyperlinkIndex and
check for it's existence in the map here.

[dmazzoni, 2011/11/01 05:08:34]

Clients can also call nHyperlinks and iterate over them in order from 0 to
nHyperlinks - 1, right? My concern is that your implementation breaks this -
nHyperlinks returns the number of object children (non-text), but get_hyperlink
returns a hyperlink based on its child index including all children.

+    return E_INVALIDARG;
+
+  *hyperlink = static_cast<IAccessibleHyperlink*>(
+      children_[index]->toBrowserAccessibilityWin()->NewReference());
+  return S_OK;
+}
+
+STDMETHODIMP BrowserAccessibilityWin::get_hyperlinkIndex(long char_index,
+    long* hyperlink_index) {
+  *hyperlink_index = -1;
+
+  if (!instance_active_)
+    return E_FAIL;
+
+  if (char_index < 0 || char_index >= static_cast<long>(hypertext_.size()))
+    return E_INVALIDARG;
+
+  std::map<int32, int32>::iterator it =
+      hyperlink_offset_to_index_.find(char_index);
+  if (it == hyperlink_offset_to_index_.end())
+    return E_FAIL;
+
+  *hyperlink_index = it->second;
+  return S_OK;
+}
+
+//
 // IAccessibleValue methods.
 //
 
@@ -2242,6 +2287,9 @@ STDMETHODIMP BrowserAccessibilityWin::QueryService(
 
   if (guidService == IID_IAccessible ||
       guidService == IID_IAccessible2 ||
+      guidService == IID_IAccessibleAction ||
+      guidService == IID_IAccessibleHyperlink ||
+      guidService == IID_IAccessibleHypertext ||
       guidService == IID_IAccessibleImage ||
       guidService == IID_IAccessibleTable ||
       guidService == IID_IAccessibleTable2 ||
@@ -2268,12 +2316,7 @@ HRESULT WINAPI BrowserAccessibilityWin::InternalQueryInterface(
     const _ATL_INTMAP_ENTRY* entries,
     REFIID iid,
     void** object) {
-  if (iid == IID_IAccessibleText) {
-    if (ia_role_ != ROLE_SYSTEM_LINK && ia_role_ != ROLE_SYSTEM_TEXT) {
-      *object = NULL;
-      return E_NOINTERFACE;
-    }
-  } else if (iid == IID_IAccessibleImage) {
+  if (iid == IID_IAccessibleImage) {
     if (ia_role_ != ROLE_SYSTEM_GRAPHIC) {
       *object = NULL;
       return E_NOINTERFACE;
@@ -2417,6 +2460,19 @@ void BrowserAccessibilityWin::Initialize() {
     relation->AddTarget(title_elem_id);
     relations_.push_back(relation);
   }
+
+  // Construct the hypertext for this node.
+  hyperlink_offset_to_index_.clear();
+  hypertext_.clear();
+  for (unsigned int i = 0; i < children().size(); ++i) {
+    BrowserAccessibility* child = children()[i];
+    if (child->role() == WebAccessibility::ROLE_STATIC_TEXT) {
+      hypertext_ += child->name();
+    } else {
+      hyperlink_offset_to_index_[hypertext_.size()] = i;
+      hypertext_ += L"\xfffc";

[dmazzoni, 2011/10/28 17:33:53]

Define a constant for this

[David Tseng, 2011/10/28 18:25:15]

Done.

+    }
+  }
 }
 
 void BrowserAccessibilityWin::SendNodeUpdateEvents() {
@@ -2516,8 +2572,10 @@ string16 BrowserAccessibilityWin::Escape(const string16& str) {
 const string16& BrowserAccessibilityWin::TextForIAccessibleText() {
   if (IsEditableText()) {
     return value_;
-  } else {
+  } else if (role_ == WebAccessibility::ROLE_STATIC_TEXT) {
     return name_;
+  } else {
+    return hypertext_;
   }
 }