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

ui/views/accessibility/native_view_accessibility_unittest.cc

Index: ui/views/accessibility/native_view_accessibility_unittest.cc
diff --git a/ui/views/accessibility/native_view_accessibility_unittest.cc b/ui/views/accessibility/native_view_accessibility_unittest.cc
index a67884894f0c3d1d8181c29905a856b827560e69..67d5c6ab656531226ee0d3ab031bb4770d2743c5 100644
--- a/ui/views/accessibility/native_view_accessibility_unittest.cc
+++ b/ui/views/accessibility/native_view_accessibility_unittest.cc
@@ -10,6 +10,7 @@
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/test/views_test_base.h"
+#include "ui/views/widget/widget.h"
 
 namespace views {
 namespace test {
@@ -23,6 +24,20 @@ class TestButton : public Button {
   TestButton() : Button(NULL) {}
 };
 
+// Adds a View with given bounds and focusability to a parent.
+View* AddNewChildWithBoundsAndFocusability(View& parent,

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

style guide doesn't allow non-const by-ref arguments -- View* parent

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

Done.

+                                           gfx::Rect bounds,

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

nit: const ref for bounds

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

Done.

+                                           bool focusable) {
+  View* child = new View;
+  child->SetBounds(bounds.x(), bounds.y(), bounds.width(), bounds.height());

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

child->SetBoundsRect(bounds);

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

Done.

+  parent.AddChildView(child);
+  if (focusable)
+    child->SetFocusBehavior(ClientView::FocusBehavior::ACCESSIBLE_ONLY);

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

nit:
child->SetFocusBehaviour(focusable ? View::FocusBehavior::ACCESSIBLE_ONLY :
...NEVER)

(+git cl-format)

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

Done.

+  else
+    child->SetFocusBehavior(ClientView::FocusBehavior::NEVER);
+  return child;
+}
+
 }  // namespace
 
 class NativeViewAccessibilityTest : public ViewsTestBase {
@@ -33,44 +48,53 @@ class NativeViewAccessibilityTest : public ViewsTestBase {
   void SetUp() override {
     ViewsTestBase::SetUp();
 
-    widget_ = new views::Widget;
-    views::Widget::InitParams params =
-        CreateParams(views::Widget::InitParams::TYPE_WINDOW);
+    widget_ = new Widget;
+    Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
     params.bounds = gfx::Rect(0, 0, 200, 200);
     widget_->Init(params);
 
     button_ = new TestButton();
     button_->SetSize(gfx::Size(20, 20));
-    button_accessibility_ = NativeViewAccessibility::Create(button_);
+    button_accessibility_ = NativeViewAccessibility::GetOrCreate(button_);
 
     label_ = new Label();
     button_->AddChildView(label_);
-    label_accessibility_ = NativeViewAccessibility::Create(label_);
+    label_accessibility_ = NativeViewAccessibility::GetOrCreate(label_);
 
     widget_->GetContentsView()->AddChildView(button_);
     widget_->Show();
   }
 
   void TearDown() override {
+    // Closing the Widget will destroy all its children and their associated
+    // NativeViewAccessibility instances.
     if (!widget_->IsClosed())
       widget_->Close();
-    button_accessibility_->Destroy();
-    button_accessibility_ = NULL;
-    label_accessibility_->Destroy();
-    label_accessibility_ = NULL;
+    button_accessibility_ = nullptr;
+    label_accessibility_ = nullptr;
     ViewsTestBase::TearDown();
   }
 
  protected:
-  views::Widget* widget_;
+  Widget* widget_;
   TestButton* button_;
   NativeViewAccessibility* button_accessibility_;
   Label* label_;
   NativeViewAccessibility* label_accessibility_;
+
+  DISALLOW_COPY_AND_ASSIGN(NativeViewAccessibilityTest);
 };
 
 TEST_F(NativeViewAccessibilityTest, RoleShouldMatch) {
   EXPECT_EQ(ui::AX_ROLE_BUTTON, button_accessibility_->GetData().role);
+  // Since the label is a subview of |button_|, and the button is keyboard
+  // focusable, the label is assumed to form part of the button and not have a
+  // role of its own.
+  EXPECT_EQ(ui::AX_ROLE_IGNORED, label_accessibility_->GetData().role);
+  // This will happen for all potentially keyboard-focusable Views with
+  // non-keyboard-focusable children, so if we make the button unfocusable, the
+  // label will be allowed to have its own role again.
+  button_->SetFocusBehavior(View::FocusBehavior::NEVER);
   EXPECT_EQ(ui::AX_ROLE_STATIC_TEXT, label_accessibility_->GetData().role);
 }
 
@@ -82,11 +106,16 @@ TEST_F(NativeViewAccessibilityTest, BoundsShouldMatch) {
 }
 
 TEST_F(NativeViewAccessibilityTest, LabelIsChildOfButton) {
+  EXPECT_EQ(0, button_accessibility_->GetChildCount());
+  EXPECT_EQ(nullptr, button_accessibility_->ChildAtIndex(0));
+  EXPECT_EQ(button_->GetNativeViewAccessible(),
+            label_accessibility_->GetParent());
+
+  // If |button_| is no longer focusable, |label_| should show up again.
+  button_->SetFocusBehavior(View::FocusBehavior::NEVER);
   EXPECT_EQ(1, button_accessibility_->GetChildCount());
   EXPECT_EQ(label_->GetNativeViewAccessible(),
             button_accessibility_->ChildAtIndex(0));
-  EXPECT_EQ(button_->GetNativeViewAccessible(),
-            label_accessibility_->GetParent());
 }
 
 // Verify Views with invisible ancestors have AX_STATE_INVISIBLE.
@@ -140,7 +169,7 @@ class TestNativeViewAccessibility : public NativeViewAccessibility {
 TEST_F(NativeViewAccessibilityTest, CrashOnWidgetDestroyed) {
   std::unique_ptr<Widget> parent_widget(new Widget);
   Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP);
-  params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
   params.bounds = gfx::Rect(50, 50, 650, 650);
   parent_widget->Init(params);
 
@@ -157,5 +186,212 @@ TEST_F(NativeViewAccessibilityTest, CrashOnWidgetDestroyed) {
   parent_widget.reset();
 }
 
+// Separate test class for a complicated test case for ignored accessibility
+// elements which are excluded from the accessibility tree.
+class NativeViewAccessibilityIgnoredElementsTest : public ViewsTestBase {
+ public:
+  NativeViewAccessibilityIgnoredElementsTest() {}
+  ~NativeViewAccessibilityIgnoredElementsTest() override {}

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

nit: I think it's safe to omit the destructor

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

Done.

+
+  void SetUp() override {
+    ViewsTestBase::SetUp();
+    // Set up an accessibility tree where X-nodes should be ignored. A Views
+    // hierarchy as shown on the left should expose the accessibility tree in
+    // the middle, and respond to hit tests as shown on the right. This test
+    // is here for completeness (in practice, it's unlikely a keyboard-focusable
+    // View will be needed as a descendant of another keyboard-focusable View).
+    //
+    //           A                    A
+    //         /   \                / | \        +++---+---+---++---+---+---+++
+    //       B       X1            B  F  E       ||| C | D | B || F | A | E |||
+    //     / | \    / | \         / \            +++---+---+---++---+---+---+++
+    //    C  D X2  X3 X4 E       C   D
+    //    |        |
+    //   X5        F
+    //
+    // Note: For hit tests, the parent's bounds will be divided equally amongst
+    // its children (including ignored elements). This means all Views belonging
+    // on the same level of the Views hierarchy will be the same size.
+    a_root_ = new View;
+    a_root_->SetSize(gfx::Size(400, 20));
+    a_root_->SetFocusBehavior(ClientView::FocusBehavior::ACCESSIBLE_ONLY);

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

ClientView -> View

(ClientView just happens to be a subclass of views::View. It's not really
related to focus/controls, but how a dialog widget is organised)

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

Done.

+
+    // Create Views in the left subtree from |a_root_|.
+    gfx::Rect new_bounds(0, 0, a_root_->width() / 2, a_root_->height());
+    View* b = AddNewChildWithBoundsAndFocusability(*a_root_, new_bounds, true);
+    // |a_root_| has 6 leaves, so divvy up space equally between them for hit
+    // tests.
+    new_bounds = gfx::Rect(0, 0, a_root_->width() / 6, a_root_->height());
+    View* c = AddNewChildWithBoundsAndFocusability(*b, new_bounds, true);
+    View* x5 = AddNewChildWithBoundsAndFocusability(*c, new_bounds, false);
+    // Update the |new_bounds| origin to position leaves next to each other.
+    new_bounds.set_x(new_bounds.x() + a_root_->width() / 6);
+    View* d = AddNewChildWithBoundsAndFocusability(*b, new_bounds, true);
+    new_bounds.set_x(new_bounds.x() + a_root_->width() / 6);
+    View* x2 = AddNewChildWithBoundsAndFocusability(*b, new_bounds, false);
+    new_bounds.set_x(new_bounds.x() + a_root_->width() / 6);
+
+    // Create Views in the right subtree from root |a_root_|.
+    new_bounds.set_size(b->size());
+    View* x1 =
+        AddNewChildWithBoundsAndFocusability(*a_root_, new_bounds, false);
+    new_bounds.set_origin(gfx::Point());
+    View* x3 = AddNewChildWithBoundsAndFocusability(*x1, new_bounds, false);
+    View* f = AddNewChildWithBoundsAndFocusability(*x3, new_bounds, true);
+    new_bounds.set_x(new_bounds.x() + a_root_->width() / 6);
+    View* x4 = AddNewChildWithBoundsAndFocusability(*x1, new_bounds, false);
+    new_bounds.set_x(new_bounds.x() + a_root_->width() / 6);
+    View* e = AddNewChildWithBoundsAndFocusability(*x1, new_bounds, true);
+
+    // Populate NativeViewAccessibility instances for convenience.
+    a_root_nva_ = NativeViewAccessibility::GetOrCreate(a_root_);
+    b_nva_ = NativeViewAccessibility::GetOrCreate(b);
+    c_b_nva_ = NativeViewAccessibility::GetOrCreate(c);
+    d_b_nva_ = NativeViewAccessibility::GetOrCreate(d);
+    e_x1_nva_ = NativeViewAccessibility::GetOrCreate(e);
+    f_x3_x1_nva_ = NativeViewAccessibility::GetOrCreate(f);
+    x1_nva_ = NativeViewAccessibility::GetOrCreate(x1);
+    x2_b_nva_ = NativeViewAccessibility::GetOrCreate(x2);
+    x3_x1_nva_ = NativeViewAccessibility::GetOrCreate(x3);
+    x4_x1_nva_ = NativeViewAccessibility::GetOrCreate(x4);
+    x5_c_b_nva_ = NativeViewAccessibility::GetOrCreate(x5);
+
+    // Add everything to a new widget, needed for hit testing and parent tests.
+    widget_ = new Widget;
+    Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
+    widget_->Init(params);
+    widget_->GetContentsView()->AddChildView(a_root_);
+    widget_->Show();
+  }
+
+  void TearDown() override {
+    widget_->Close();
+    ViewsTestBase::TearDown();
+  }
+
+ protected:
+  // Owned by |widget_|.
+  View* a_root_;
+
+  // Owned by their respective View, which are owned by |widget_|.
+  // The following NativeViewAccessibility objects are named after their
+  // corresponding node as shown in the diagrams in SetUp(), then a list of
+  // their ancestors (minus the root), separated by underscores.
+  NativeViewAccessibility* a_root_nva_;
+  NativeViewAccessibility* b_nva_;
+  NativeViewAccessibility* c_b_nva_;
+  NativeViewAccessibility* d_b_nva_;
+  NativeViewAccessibility* e_x1_nva_;
+  NativeViewAccessibility* f_x3_x1_nva_;
+  NativeViewAccessibility* x1_nva_;
+  NativeViewAccessibility* x2_b_nva_;
+  NativeViewAccessibility* x3_x1_nva_;
+  NativeViewAccessibility* x4_x1_nva_;
+  NativeViewAccessibility* x5_c_b_nva_;
+
+  Widget* widget_ = nullptr;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NativeViewAccessibilityIgnoredElementsTest);
+};
+
+// Check the number of a11y children returned accounts for ignored elements.
+TEST_F(NativeViewAccessibilityIgnoredElementsTest, GetChildCount) {
+  EXPECT_EQ(3, a_root_nva_->GetChildCount());
+  EXPECT_EQ(2, b_nva_->GetChildCount());
+  // Leaf elements will return 0.
+  EXPECT_EQ(0, c_b_nva_->GetChildCount());
+  EXPECT_EQ(0, x5_c_b_nva_->GetChildCount());
+  EXPECT_EQ(0, d_b_nva_->GetChildCount());
+  EXPECT_EQ(0, x2_b_nva_->GetChildCount());
+  EXPECT_EQ(2, x1_nva_->GetChildCount());
+  EXPECT_EQ(1, x3_x1_nva_->GetChildCount());
+  EXPECT_EQ(0, f_x3_x1_nva_->GetChildCount());
+  EXPECT_EQ(0, x4_x1_nva_->GetChildCount());
+  EXPECT_EQ(0, e_x1_nva_->GetChildCount());
+}
+
+// Make sure the correct a11y child element is returned at its corresponding
+// index. Unignored children with ignored parents should move up in the tree.
+TEST_F(NativeViewAccessibilityIgnoredElementsTest, ChildAtIndex) {
+  EXPECT_EQ(b_nva_->GetNativeObject(), a_root_nva_->ChildAtIndex(0));
+  EXPECT_EQ(f_x3_x1_nva_->GetNativeObject(), a_root_nva_->ChildAtIndex(1));
+  EXPECT_EQ(e_x1_nva_->GetNativeObject(), a_root_nva_->ChildAtIndex(2));
+  EXPECT_EQ(nullptr, a_root_nva_->ChildAtIndex(3));
+
+  EXPECT_EQ(c_b_nva_->GetNativeObject(), b_nva_->ChildAtIndex(0));
+  EXPECT_EQ(d_b_nva_->GetNativeObject(), b_nva_->ChildAtIndex(1));
+  EXPECT_EQ(nullptr, b_nva_->ChildAtIndex(2));
+
+  EXPECT_EQ(f_x3_x1_nva_->GetNativeObject(), x1_nva_->ChildAtIndex(0));
+  EXPECT_EQ(e_x1_nva_->GetNativeObject(), x1_nva_->ChildAtIndex(1));
+  EXPECT_EQ(nullptr, x1_nva_->ChildAtIndex(2));
+
+  EXPECT_EQ(f_x3_x1_nva_->GetNativeObject(), x3_x1_nva_->ChildAtIndex(0));
+
+  // Node with ignored children.
+  EXPECT_EQ(nullptr, c_b_nva_->ChildAtIndex(0));
+  // Node with 0 children.
+  EXPECT_EQ(nullptr, d_b_nva_->ChildAtIndex(0));
+  // Ignored node with 0 children.
+  EXPECT_EQ(nullptr, x2_b_nva_->ChildAtIndex(0));
+}
+
+// Check the a11y parents returned correspond to their unignored children.
+TEST_F(NativeViewAccessibilityIgnoredElementsTest, GetParent) {
+  EXPECT_EQ(a_root_->parent()->GetNativeViewAccessible(),
+            a_root_nva_->GetParent());
+  EXPECT_EQ(a_root_->GetNativeViewAccessible(), b_nva_->GetParent());
+  EXPECT_EQ(b_nva_->GetNativeObject(), c_b_nva_->GetParent());
+  EXPECT_EQ(c_b_nva_->GetNativeObject(), x5_c_b_nva_->GetParent());
+  EXPECT_EQ(b_nva_->GetNativeObject(), d_b_nva_->GetParent());
+  EXPECT_EQ(b_nva_->GetNativeObject(), x2_b_nva_->GetParent());
+  EXPECT_EQ(a_root_->GetNativeViewAccessible(), x1_nva_->GetParent());
+
+  // Skips X1.
+  EXPECT_EQ(a_root_->GetNativeViewAccessible(), x3_x1_nva_->GetParent());
+  EXPECT_EQ(a_root_->GetNativeViewAccessible(), x4_x1_nva_->GetParent());
+  EXPECT_EQ(a_root_->GetNativeViewAccessible(), e_x1_nva_->GetParent());
+
+  // Skips X3 and X1.
+  EXPECT_EQ(a_root_->GetNativeViewAccessible(), f_x3_x1_nva_->GetParent());
+}
+
+// Test that a11y hit tests work correctly with ignored elements. A hit on a
+// ignored element should return its first unignored ancestor.
+TEST_F(NativeViewAccessibilityIgnoredElementsTest, HitTestSync) {
+  // Move the hit point horizontally, incrementing by the leaf View width.
+  const int min_width = a_root_->GetBoundsInScreen().width() / 6;
+  int curr_x = a_root_->GetBoundsInScreen().x() + min_width / 2;
+  const int y = a_root_->GetBoundsInScreen().CenterPoint().y();
+
+  // HitTestSync isn't recursive, so manually do the recursion until arriving at
+  // the correct answer. I.e., the last NativeViewAccessible being tested for
+  // each |curr_x|, |y| pair below would be the correct return value if
+  // HitTestSync were recursive.
+  EXPECT_EQ(b_nva_->GetNativeObject(), a_root_nva_->HitTestSync(curr_x, y));
+  EXPECT_EQ(c_b_nva_->GetNativeObject(), b_nva_->HitTestSync(curr_x, y));
+  EXPECT_EQ(c_b_nva_->GetNativeObject(), c_b_nva_->HitTestSync(curr_x, y));
+  curr_x += min_width;
+  EXPECT_EQ(b_nva_->GetNativeObject(), a_root_nva_->HitTestSync(curr_x, y));
+  EXPECT_EQ(d_b_nva_->GetNativeObject(), b_nva_->HitTestSync(curr_x, y));
+  curr_x += min_width;
+  EXPECT_EQ(b_nva_->GetNativeObject(), a_root_nva_->HitTestSync(curr_x, y));
+  EXPECT_EQ(x2_b_nva_->GetNativeObject(), b_nva_->HitTestSync(curr_x, y));
+  EXPECT_EQ(nullptr, x2_b_nva_->HitTestSync(curr_x, y));
+  curr_x += min_width;
+  EXPECT_EQ(x1_nva_->GetNativeObject(), a_root_nva_->HitTestSync(curr_x, y));
+  EXPECT_EQ(x3_x1_nva_->GetNativeObject(), x1_nva_->HitTestSync(curr_x, y));
+  EXPECT_EQ(f_x3_x1_nva_->GetNativeObject(),
+            x3_x1_nva_->HitTestSync(curr_x, y));
+  curr_x += min_width;
+  EXPECT_EQ(x1_nva_->GetNativeObject(), a_root_nva_->HitTestSync(curr_x, y));
+  EXPECT_EQ(x4_x1_nva_->GetNativeObject(), x1_nva_->HitTestSync(curr_x, y));
+  EXPECT_EQ(nullptr, x4_x1_nva_->HitTestSync(curr_x, y));
+  curr_x += min_width;
+  EXPECT_EQ(x1_nva_->GetNativeObject(), a_root_nva_->HitTestSync(curr_x, y));
+  EXPECT_EQ(e_x1_nva_->GetNativeObject(), x1_nva_->HitTestSync(curr_x, y));
+}
+
 }  // namespace test
 }  // namespace views