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(OS_MACOSX)
+namespace {
+
+bool IsAccessibilityFocusableWhenEnabled(View* view) {
+  return view->focus_behavior() != View::FocusBehavior::NEVER &&
+         view->IsDrawn();
+}
+
+bool ViewHasFocusableAncestor(View* view) {
+  while ((view = view->parent())) {
+    if (IsAccessibilityFocusableWhenEnabled(view))
+      return true;
+  }
+  return false;
+}
+
+}  // namespace
+#endif
+
 #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)
 
 NativeViewAccessibility::NativeViewAccessibility(View* view)
     : view_(view),
       parent_widget_(nullptr),
@@ skipping 47 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_->visible())
-    data_.state |= (1 << ui::AX_STATE_INVISIBLE);
+    data_.AddStateFlag(ui::AX_STATE_INVISIBLE);
+
+#if defined(OS_MACOSX)

[dmazzoni, 2016/11/30 23:12:44]

I'd prefer to find a solution that works for all
platforms. In general hiding children of focusable
views sounds reasonable.

[Patti Lor, 2016/12/01 01:12:07]

See next comment.

+  // Make sure this element is ignored if there's a focusable parent. All
+  // keyboard focusable elements should be leaf nodes in the accessibility tree.
+  // Exceptions to this rule will themselves be accessibility focusable.
+  if (!IsAccessibilityFocusableWhenEnabled(view_) &&
+      ViewHasFocusableAncestor(view_))
+    data_.role = ui::AX_ROLE_UNKNOWN;

[dmazzoni, 2016/11/30 23:12:44]

Rather than setting the role to unknown, what if we just remove them
from the accessibility tree by modifying GetChildCount(), below?

[Patti Lor, 2016/12/01 01:12:07]

Yes - this was my approach in patchset 8. Removing things in the accessibility
tree has a couple implications, if we want to be consistent with the results
returned from GetChildCount(), ChildAtIndex(), GetParent(), etc, which I will
summarise here:
- Children of deleted elements will need to be moved up the tree.
- Hit tests will need to ignore those deleted elements.

The diagram in
https://codereview.chromium.org/2119413004/diff/140001/ui/views/accessibility...
(you'll need to scroll a bit) is probably the most useful in illustrating this.
(We can avoid having to implement this ourselves in this particular patchset
because Mac does this for us with its NSAccessibilityUnignoredAncestor()
function.) To make it platform independent, I could merge this patchset with
patchset 8, but the issue was that it also requires a way to retrieve a
NativeViewAccessibility from a View. I'd originally solved this by adding
View::GetNativeViewAccessibility(), but tapted pointed out this might be
difficult/confusing especially with GetNativeViewAccessible() there as well.
What do you think?

+#endif
 
   return data_;
 }
 
 int NativeViewAccessibility::GetChildCount() {
   int child_count = view_->child_count();

[dmazzoni, 2016/11/30 23:12:44]

Right here, you could add:

if (IsAccessibilityFocusableWhenEnabled(view_))
  return 0;

Would that work? If not, why not?

[Patti Lor, 2016/12/01 01:12:07]

See above comment.

 
   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
@@ skipping 27 matching lines @@
     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();
   }
 
   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, 2016/11/30 23:12:44]

Is this specific only to AXPlatformNodeWin?

[Patti Lor, 2016/12/01 01:12:07]

Yes, AFAICT, HitTestSync is only used for Windows (searching it in codesearch
only turns up 1 override in the Linux code which returns nullptr).

+  // 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 97 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