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"
@@ skipping 40 matching lines @@
   if (!ui::AXNodeData::IsFlagSet(GetData().state, ui::AX_STATE_FOCUSABLE))
     return false;
 
   if (focused)
     view_->RequestFocus();
   else if (view_->HasFocus())
     view_->GetFocusManager()->ClearFocus();
   return true;
 }
 
+#if defined(OS_MACOSX)
+bool NativeViewAccessibility::HasFocusableAncestor() {
+  View* parent = view_;
+  while ((parent = parent->parent())) {
+    if (parent->IsAccessibilityFocusable())
+      return true;
+  }
+  return false;
+}
+#endif
+
 // ui::AXPlatformNodeDelegate
 
 const ui::AXNodeData& NativeViewAccessibility::GetData() {
   data_ = ui::AXNodeData();
   data_.state = 0;
 
   // Views may misbehave if their widget is closed; return an unknown role
   // rather than possibly crashing.
   if (!view_->GetWidget() || view_->GetWidget()->IsClosed()) {
     data_.role = ui::AX_ROLE_UNKNOWN;
     data_.state = 1 << ui::AX_STATE_DISABLED;
     return data_;
   }
 
   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)
+  // Make sure this element is ignored if there's a focusable parent. All
+  // keyboard focusable elements should be leaf nodes in the accessibility tree.
+  if (HasFocusableAncestor())

[tapted, 2016/11/25 04:36:27]

should we do this only if the view itself is not accessibility focusable? I
think it's allowed to traverse things inside something that is focusable.

An example might be something like the omnibox which gets icons within it.

[Patti Lor, 2016/11/28 06:40:23]

That sounds reasonable - I don't think it fixes the Omnibox problems (see
below), but have added this in anyway.

The star was always working with VO, but the LocationIconView (which is the lock
icon/info icon and stuff to the left of the Omnibox) never did. A clean build
makes it show up to VO separately as image + label, then with the changes in
this CL, it's completely ignored. I've filed a bug to fix this in a follow up CL
here: https://bugs.chromium.org/p/chromium/issues/detail?id=668930, and I think
I know how to do it.

(To be clear, the change you've suggested above doesn't fix it, it's still
ignored.)

+    data_.role = ui::AX_ROLE_UNKNOWN;
+#endif
 
   return data_;
 }
 
 int NativeViewAccessibility::GetChildCount() {
   int child_count = view_->child_count();
 
   std::vector<Widget*> child_widgets;
   PopulateChildWidgetVector(&child_widgets);
   child_count += child_widgets.size();
@@ skipping 33 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
+  // 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