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

ui/views/accessibility/native_view_accessibility.cc

 // Copyright (c) 2013 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 "ui/views/accessibility/native_view_accessibility.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "ui/events/event_utils.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/views/controls/native/native_view_host.h"
 #include "ui/views/view.h"
 #include "ui/views/widget/widget.h"
 
 namespace views {
 
-#if !defined(PLATFORM_HAS_NATIVE_VIEW_ACCESSIBILITY_IMPL)
+namespace {
+
+typedef std::map<View*, NativeViewAccessibility*> NativeViewAccessibilityMap;

[tapted, 2017/01/11 18:27:39]

Although this feels natural for me... I think the preference is for `using` now.
i.e.

using NativeViewAccessibilityMap = std::map<View*, NativeViewAccessibility*>;

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

Done - sorry, you've told me this before ><

+
 // static

[tapted, 2017/01/11 18:27:39]

remove comment

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

Done.

-NativeViewAccessibility* NativeViewAccessibility::Create(View* view) {
-  return new NativeViewAccessibility(view);
+NativeViewAccessibilityMap& GetNativeViewAccessibilityMap() {
+  CR_DEFINE_STATIC_LOCAL(NativeViewAccessibilityMap, views_to_nva, ());
+  return views_to_nva;
 }
-#endif  // !defined(PLATFORM_HAS_NATIVE_VIEW_ACCESSIBILITY_IMPL)
+
+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::GetOrCreate(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::GetOrCreate(View* view) {
+  NativeViewAccessibilityMap& views_to_nva = GetNativeViewAccessibilityMap();
+  NativeViewAccessibilityMap::iterator it = views_to_nva.find(view);
+  if (it != views_to_nva.end())
+    return (*it).second;
+  NativeViewAccessibility* nva = Create(view);
+  views_to_nva.insert(std::make_pair(view, nva));
+  return nva;
+}

[tapted, 2017/01/11 18:27:39]

since the mapped_type is a primitive type, I think you can just do:

  NativeViewAccessibility*& nva = GetNativeViewAccessibilityMap()[view];
  if (!nva)
    nva = Create(view);
  return nva;

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

Deleted, not needed any more.

 
 NativeViewAccessibility::NativeViewAccessibility(View* view)
     : view_(view),
       parent_widget_(nullptr),
       ax_node_(nullptr) {
   ax_node_ = ui::AXPlatformNode::Create(this);
 }
 
 NativeViewAccessibility::~NativeViewAccessibility() {
   if (ax_node_)
     ax_node_->Destroy();
   if (parent_widget_)
     parent_widget_->RemoveObserver(this);
+
+  // Remove this instance from the map when it's destroyed. Don't bother
+  // checking if the erase was successful because tests may create an instance
+  // directly instead of using GetOrCreate(), so it wouldn't exist in the map.
+  GetNativeViewAccessibilityMap().erase(view_);
 }
 
 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);
 }
 
 bool NativeViewAccessibility::SetFocused(bool focused) {
   if (!ui::AXNodeData::IsFlagSet(GetData().state, ui::AX_STATE_FOCUSABLE))
     return false;
 
   if (focused)
@@ skipping 18 matching lines @@
   }
 
   view_->GetAccessibleNodeData(&data_);
   data_.location = gfx::RectF(view_->GetBoundsInScreen());
   base::string16 description;
   view_->GetTooltipText(gfx::Point(), &description);
   data_.AddStringAttribute(ui::AX_ATTR_DESCRIPTION,
                            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);
   child_count += child_widgets.size();
 
   return child_count;
 }
 
 gfx::NativeViewAccessible NativeViewAccessibility::ChildAtIndex(int index) {
-  // If this is a root view, our widget might have child widgets. Include
+  if (GetChildCount() == 0)
+    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());

[tapted, 2017/01/11 18:27:39]

i don't know why this static_cast is here - I don't think it's needed

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

Done.

[tapted, 2017/01/11 18:27:39]

We can avoid multiple calls to GetUnignoredA11yChildren here by not calling
GetChildCount() above and below. PopulateChildWidgetVector is already repeated,
so I think it's fine to repeat GetUnignoredA11yChildren as well.

e.g., here, do

std::vector<gfx::NVA> a11y_children = GetUnignored..
int child_view_count = a11y_children.size();
if (index >= child_view_count + child_widget_count)
  return nullptr;  (note: also covers GetChildCount() == 0)

if (index < child_view_count)
  return a11y_children[index];

Widget* child_widget = child_widgets[index - child_view_count];
return child_widget->GetRootView()->GetNativeViewAccessible();

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

Done.

-  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()];
+  int child_view_count = GetChildCount() - child_widget_count;
+  if (index < child_view_count) {
+    return GetUnignoredA11yChildren(view_)[index];
+  } else if (index < (child_view_count + child_widget_count)) {
+    Widget* child_widget = child_widgets[index - child_view_count];
     return child_widget->GetRootView()->GetNativeViewAccessible();
   }
 
   return nullptr;
 }
 
 gfx::NativeWindow NativeViewAccessibility::GetTopLevelWidget() {
   if (view_->GetWidget())
     return view_->GetWidget()->GetTopLevelWidget()->GetNativeWindow();
   return nullptr;
 }
 
 gfx::NativeViewAccessible NativeViewAccessibility::GetParent() {
-  if (view_->parent())
-    return view_->parent()->GetNativeViewAccessible();
+  View* parent_view = view_->parent();
+  if (parent_view) {
+    if (IsViewA11yIgnored(parent_view))
+      return GetOrCreate(parent_view)->GetParent();
+    return parent_view->GetNativeViewAccessible();
+  }
 
   // TODO: move this to NativeViewAccessibilityMac.
 #if defined(OS_MACOSX)
   if (view_->GetWidget())
     return view_->GetWidget()->GetNativeView();
 #endif
 
   if (parent_widget_)
     return parent_widget_->GetRootView()->GetNativeViewAccessible();
 
   return nullptr;
 }
 
 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) {
   if (!view_ || !view_->GetWidget())
     return nullptr;
 
   // Search child widgets first, since they're on top in the z-order.
   std::vector<Widget*> child_widgets;
   PopulateChildWidgetVector(&child_widgets);
   for (Widget* child_widget : child_widgets) {
     View* child_root_view = child_widget->GetRootView();
     gfx::Point point(x, y);
     View::ConvertPointFromScreen(child_root_view, &point);
     if (child_root_view->HitTestPoint(point))
       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

[dmazzoni, 2017/01/11 07:39:37]

I think all platforms should be doing this, not just AXPlatformNodeWin.
I just checked AXPlatformNodeMac and it seems to be calling
hit test recursively on each child that matches.

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

Do you mean -accessibilityHitTest? It doesn't seem as if it's calling
NVA::HitTestSync though, I think it just has its own implementation which is
recursive. I'm not sure if it should be changed to use HitTestSync though, I
think the difference would be that on Mac Widgets aren't included in the hit
test.

+  // 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())
       continue;
 
     gfx::Point point_in_child_coords(point);
     view_->ConvertPointToTarget(view_, child_view, &point_in_child_coords);
     if (child_view->HitTestPoint(point_in_child_coords))
       return child_view->GetNativeViewAccessible();
   }
@@ skipping 61 matching lines @@
   }
 }
 
 void NativeViewAccessibility::SetParentWidget(Widget* parent_widget) {
   if (parent_widget_)
     parent_widget_->RemoveObserver(this);
   parent_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.
   Widget* widget = view_->GetWidget();
   if (!widget || widget->GetRootView() != view_)
     return;
 
   std::set<Widget*> child_widgets;
   Widget::GetAllOwnedWidgets(widget->GetNativeView(), &child_widgets);
   for (auto iter = child_widgets.begin(); iter != child_widgets.end(); ++iter) {
@@ skipping 16 matching lines @@
               child_widget_platform_node->GetDelegate());
       if (child_widget_view_accessibility->parent_widget() != widget)
         child_widget_view_accessibility->SetParentWidget(widget);
     }
 
     result_child_widgets->push_back(child_widget);
   }
 }
 
 }  // namespace views