a11y: Exclude children of nested keyboard accessible controls from a11y tree.

ui/views/accessibility/native_view_accessibility.cc

Index: ui/views/accessibility/native_view_accessibility.cc
diff --git a/ui/views/accessibility/native_view_accessibility.cc b/ui/views/accessibility/native_view_accessibility.cc
index 1d5c11a415de6507e384cb15a305141282dbc192..95e12318b0406445a38a1df493e227932f1bcb62 100644
--- a/ui/views/accessibility/native_view_accessibility.cc
+++ b/ui/views/accessibility/native_view_accessibility.cc
@@ -5,6 +5,7 @@
 #include "ui/views/accessibility/native_view_accessibility.h"
 
 #include "base/strings/utf_string_conversions.h"
+#include "ui/accessibility/platform/ax_platform_node.h"
 #include "ui/events/event_utils.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/views/controls/native/native_view_host.h"
@@ -13,12 +14,69 @@
 
 namespace views {
 
-#if !defined(PLATFORM_HAS_NATIVE_VIEW_ACCESSIBILITY_IMPL)
+namespace {
+
+bool IsAccessibilityFocusableWhenEnabled(View* view) {
+  return view->focus_behavior() != View::FocusBehavior::NEVER &&
+         view->IsDrawn();
+}
+
+// Used to determine if a View should be ignored by accessibility clients by
+// being a non-keyboard-focusable child of a keyboard-focusable ancestor.
+bool IsViewUnfocusableChildOfFocusableAncestor(View* view) {
+  if (IsAccessibilityFocusableWhenEnabled(view))
+    return false;
+
+  while (view->parent()) {
+    view = view->parent();
+    if (IsAccessibilityFocusableWhenEnabled(view))
+      return true;
+  }
+  return false;
+}
+
+// Convenience method for checking if a View should be ignored by a11y.
+bool IsViewA11yIgnored(View* view) {
+  return NativeViewAccessibility::GetForView(view)->GetData().role ==
+         ui::AX_ROLE_IGNORED;
+}
+
+// Recursively gets a list of unignored gfx::NativeViewAccessible children of a
+// given View. If a child is ignored, its children will be used instead.
+std::vector<gfx::NativeViewAccessible> GetUnignoredA11yChildren(View* view) {
+  std::vector<gfx::NativeViewAccessible> children;
+  for (int i = 0; i < view->child_count(); ++i) {
+    View* child = view->child_at(i);
+    if (IsViewA11yIgnored(child)) {
+      std::vector<gfx::NativeViewAccessible> grandchildren =
+          GetUnignoredA11yChildren(child);
+      children.insert(children.end(), grandchildren.begin(),
+                      grandchildren.end());
+    } else {
+      children.push_back(child->GetNativeViewAccessible());
+    }
+  }
+  return children;
+}
+
+}  // namespace
+
 // static
-NativeViewAccessibility* NativeViewAccessibility::Create(View* view) {
-  return new NativeViewAccessibility(view);
+std::unique_ptr<NativeViewAccessibility> NativeViewAccessibility::CreateForView(
+    View* view) {
+  return base::WrapUnique(Create(view));
+}
+
+// static
+NativeViewAccessibility* NativeViewAccessibility::GetForView(View* view) {
+  // Retrieving the gfx::NativeViewAccessible also ensures that a
+  // NativeViewAccessibility exists for the given View.
+  gfx::NativeViewAccessible native_object = view->GetNativeViewAccessible();
+  ui::AXPlatformNode* node =
+      ui::AXPlatformNode::FromNativeViewAccessible(native_object);
+  DCHECK(node);
+  return static_cast<NativeViewAccessibility*>(node->GetDelegate());
 }
-#endif  // !defined(PLATFORM_HAS_NATIVE_VIEW_ACCESSIBILITY_IMPL)
 
 NativeViewAccessibility::NativeViewAccessibility(View* view)
     : view_(view),
@@ -38,10 +96,6 @@ gfx::NativeViewAccessible NativeViewAccessibility::GetNativeObject() {
   return ax_node_ ? ax_node_->GetNativeViewAccessible() : nullptr;
 }
 
-void NativeViewAccessibility::Destroy() {
-  delete this;
-}
-
 void NativeViewAccessibility::NotifyAccessibilityEvent(ui::AXEvent event_type) {
   if (ax_node_)
     ax_node_->NotifyAccessibilityEvent(event_type);
@@ -80,19 +134,25 @@ const ui::AXNodeData& NativeViewAccessibility::GetData() {
                            base::UTF16ToUTF8(description));
 
   if (view_->IsAccessibilityFocusable())
-    data_.state |= (1 << ui::AX_STATE_FOCUSABLE);
+    data_.AddStateFlag(ui::AX_STATE_FOCUSABLE);
 
   if (!view_->enabled())
-    data_.state |= (1 << ui::AX_STATE_DISABLED);
+    data_.AddStateFlag(ui::AX_STATE_DISABLED);
 
   if (!view_->IsDrawn())
-    data_.state |= (1 << ui::AX_STATE_INVISIBLE);
+    data_.AddStateFlag(ui::AX_STATE_INVISIBLE);
+
+  // Make sure this element is excluded from the a11y tree if there's a
+  // focusable parent. All keyboard focusable elements should be leaf nodes.
+  // Exceptions to this rule will themselves be accessibility focusable.
+  if (IsViewUnfocusableChildOfFocusableAncestor(view_))
+    data_.role = ui::AX_ROLE_IGNORED;
 
   return data_;
 }
 
 int NativeViewAccessibility::GetChildCount() {
-  int child_count = view_->child_count();
+  int child_count = GetUnignoredA11yChildren(view_).size();
 
   std::vector<Widget*> child_widgets;
   PopulateChildWidgetVector(&child_widgets);
@@ -102,19 +162,24 @@ int NativeViewAccessibility::GetChildCount() {
 }
 
 gfx::NativeViewAccessible NativeViewAccessibility::ChildAtIndex(int index) {
-  // If this is a root view, our widget might have child widgets. Include
+  std::vector<gfx::NativeViewAccessible> a11y_children =
+      GetUnignoredA11yChildren(view_);
+  int total_child_count = a11y_children.size();
+
+  if (index >= total_child_count)
+    return nullptr;
+
+  // Include the child widgets that may be present if this is a RootView.
   std::vector<Widget*> child_widgets;
   PopulateChildWidgetVector(&child_widgets);
-  int child_widget_count = static_cast<int>(child_widgets.size());
 
-  if (index < view_->child_count()) {
-    return view_->child_at(index)->GetNativeViewAccessible();
-  } else if (index < view_->child_count() + child_widget_count) {
-    Widget* child_widget = child_widgets[index - view_->child_count()];
-    return child_widget->GetRootView()->GetNativeViewAccessible();
+  int child_view_count = total_child_count - child_widgets.size();
+  if (index < child_view_count) {
+    return a11y_children[index];
   }
 
-  return nullptr;
+  Widget* child_widget = child_widgets[index - child_view_count];
+  return child_widget->GetRootView()->GetNativeViewAccessible();
 }
 
 gfx::NativeWindow NativeViewAccessibility::GetTopLevelWidget() {
@@ -124,13 +189,17 @@ gfx::NativeWindow NativeViewAccessibility::GetTopLevelWidget() {
 }
 
 gfx::NativeViewAccessible NativeViewAccessibility::GetParent() {

[tapted, 2017/02/15 02:10:45]

yeah NativeViewAccessible is `typedef void*` on ChromeOS, so this should either
return null, or simply not get called on ChromeOS at all (i.e. disable the tests
on ChromeOS that get here, or just remove the test.cc files from the sources
list on cros)

[Patti Lor, 2017/02/21 03:29:17]

Yep - so I ended up just returning a new instance of NativeViewAccessibility
(consistent with the old behaviour) for CrOS in GetForView(), and verified that
this still returns null on that platform. I'm not sure if keeping all the tests
are useful (e.g. ChildAtIndex, GetParent, HitTestSync shouldn't be doing
anything other than comparing nullptrs), do you think it's worth ifdefing them
out or just leaving them? They still pass.

-  if (view_->parent())
-    return view_->parent()->GetNativeViewAccessible();
+  View* parent_view = view_->parent();
+  if (parent_view) {
+    if (IsViewA11yIgnored(parent_view))
+      return GetForView(parent_view)->GetParent();
+    return parent_view->GetNativeViewAccessible();
+  }
 
   // TODO: move this to NativeViewAccessibilityMac.
 #if defined(OS_MACOSX)
   if (view_->GetWidget())
-    return view_->GetWidget()->GetNativeView();
+    return (gfx::NativeViewAccessible)view_->GetWidget()->GetNativeView();

[tapted, 2017/02/15 02:10:45]

can the c-style cast be removed?

[Patti Lor, 2017/02/21 03:29:17]

Yep! Originally, I tried a static_cast instead, but that doesn't work because
gfx::NativeViewAccessible/id/struct objc_object* and gfx::NativeView/NSView*
"aren't related by inheritance", but now that https://crrev.com/2704263002 puts
this bit of Mac-specific code in an .mm file, it works without casting at all.

 #endif
 
   if (parent_widget_)
@@ -140,7 +209,7 @@ gfx::NativeViewAccessible NativeViewAccessibility::GetParent() {
 }
 
 gfx::Vector2d NativeViewAccessibility::GetGlobalCoordinateOffset() {
-  return gfx::Vector2d(0, 0);  // location is already in screen coordinates.
+  return gfx::Vector2d(0, 0);  // Location is already in screen coordinates.
 }
 
 gfx::NativeViewAccessible NativeViewAccessibility::HitTestSync(int x, int y) {
@@ -158,14 +227,20 @@ gfx::NativeViewAccessible NativeViewAccessibility::HitTestSync(int x, int y) {
       return child_root_view->GetNativeViewAccessible();
   }
 
+  if (GetChildCount() == 0) {
+    if (IsViewA11yIgnored(view_))
+      return nullptr;
+    return GetNativeObject();
+  }
+
   gfx::Point point(x, y);
   View::ConvertPointFromScreen(view_, &point);
   if (!view_->HitTestPoint(point))
     return nullptr;
 
-  // Check if the point is within any of the immediate children of this
-  // view. We don't have to search further because AXPlatformNode will
-  // do a recursive hit test if we return anything other than |this| or NULL.
+  // Check if the point is within any of the immediate children of this view. We
+  // don't have to search further because AXPlatformNodeWin will do a recursive
+  // hit test if we return anything other than GetNativeObject() or nullptr.
   for (int i = view_->child_count() - 1; i >= 0; --i) {
     View* child_view = view_->child_at(i);
     if (!child_view->visible())
@@ -247,6 +322,13 @@ void NativeViewAccessibility::SetParentWidget(Widget* parent_widget) {
   parent_widget_->AddObserver(this);
 }
 
+#if !defined(PLATFORM_HAS_NATIVE_VIEW_ACCESSIBILITY_IMPL)
+// static
+NativeViewAccessibility* NativeViewAccessibility::Create(View* view) {
+  return new NativeViewAccessibility(view);
+}
+#endif  // !defined(PLATFORM_HAS_NATIVE_VIEW_ACCESSIBILITY_IMPL)
+
 void NativeViewAccessibility::PopulateChildWidgetVector(
     std::vector<Widget*>* result_child_widgets) {
   // Only attach child widgets to the root view.