a11y: Add a11y information to views::Tab and manually ignore its a11y children.

ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm

Index: ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm
diff --git a/ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm b/ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm
new file mode 100644
index 0000000000000000000000000000000000000000..43edc1da645175710a585294a2fa953dc58b9a83
--- /dev/null
+++ b/ui/views/controls/tabbed_pane/tabbed_pane_accessibility_mac_unittest.mm
@@ -0,0 +1,159 @@
+// Copyright 2017 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.
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/strings/utf_string_conversions.h"
+#include "ui/gfx/geometry/point.h"
+#import "ui/gfx/mac/coordinate_conversion.h"
+#include "ui/views/controls/tabbed_pane/tabbed_pane.h"
+#include "ui/views/test/widget_test.h"
+#include "ui/views/widget/widget.h"
+#import "testing/gtest_mac.h"
+
+namespace views {
+

[tapted, 2017/01/10 16:22:19]

nit: remove blank line

[Patti Lor, 2017/01/11 05:51:44]

Done.

+namespace test {
+
+class TabbedPaneAccessibilityMacTest : public WidgetTest {
+ public:
+  TabbedPaneAccessibilityMacTest() {}
+
+  void SetUp() override {

[tapted, 2017/01/10 16:22:19]

nit: // WidgetTest:

(or testing::Test, but pkasting rolls differently)

[Patti Lor, 2017/01/11 05:51:44]

Done.

+    WidgetTest::SetUp();
+    widget_ = CreateTopLevelPlatformWidget();
+    widget_->SetBounds(gfx::Rect(50, 50, 100, 100));
+    tabbed_pane_ = new TabbedPane();
+    tabbed_pane_->SetSize(gfx::Size(100, 100));
+
+    // Create two tabs and position/size them.
+    tabbed_pane_->AddTab(base::ASCIIToUTF16("Tab 1"), new View());
+    tabbed_pane_->AddTab(base::ASCIIToUTF16("Tab 2"), new View());
+    tabbed_pane_->Layout();
+
+    widget_->GetContentsView()->AddChildView(tabbed_pane_);
+    widget_->Show();
+  }
+
+  void TearDown() override {
+    widget_->CloseNow();
+    WidgetTest::TearDown();
+  }
+
+  Tab* GetTabAt(int index) {
+    return static_cast<Tab*>(tabbed_pane_->tab_strip_->child_at(index));

[tapted, 2017/01/10 16:22:19]

So there's something a bit uglier that should work and avoid having to move
TabStrip into the header.

    // Note this requires View to be the first superclass of TabStrip.
    View* tab_strip = reinterpret_cast<View*>(tabbed_pane_->tab_strip_);
    return static_cast<Tab*>(tab_strip->child_at(index));


But there's not really a precedent for that under ui/views. I think moving
TabStrip to the header is the right approach.

[Patti Lor, 2017/01/11 05:51:44]

Yeah - this method is also needed in TabbedViewTest, so I agree.

+  }
+
+  id AttributeValueAtPoint(NSString* attribute, const gfx::Point& point) {
+    id value =
+        [A11yElementAtPoint(point) accessibilityAttributeValue:attribute];
+    EXPECT_NE(nil, value);
+    return value;
+  }
+
+  id A11yElementAtPoint(const gfx::Point& point) {
+    // Accessibility hit tests come in Cocoa screen coordinates.
+    NSPoint ns_point = gfx::ScreenPointToNSPoint(point);
+    return [widget_->GetNativeWindow() accessibilityHitTest:ns_point];
+  }
+
+  gfx::Point TabCenterPoint(int index) {
+    return GetTabAt(index)->GetBoundsInScreen().CenterPoint();
+  }
+
+ protected:
+  Widget* widget_;

[tapted, 2017/01/10 16:22:19]

nit: = nullptr  (since it's not set in the constructor - it's not a bug, but if,
say, someone forgets to call super::SetUp() they will get nicer errors)

[Patti Lor, 2017/01/11 05:51:44]

Done.

+  TabbedPane* tabbed_pane_;

[tapted, 2017/01/10 16:22:19]

= nullptr

[Patti Lor, 2017/01/11 05:51:44]

Done.

+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TabbedPaneAccessibilityMacTest);
+};
+
+// Test the Tab's a11y information compared to a Cocoa NSTabViewItem.
+TEST_F(TabbedPaneAccessibilityMacTest, AttributesMatchAppKit) {
+  // Create a Cocoa NSTabView to test against and select the first tab.
+  NSTabView* cocoa_tab_group =

[tapted, 2017/01/10 16:22:19]

this one still needs a scoped_nsobject (you could put an autorelease on the next
line, but by convention we tend to prefer scoped_nsobject -- there are extra
runtime overheads for autorelease, but it still gets used if the code is neater
with autorelease, like in the array initialization).

[Patti Lor, 2017/01/11 05:51:44]

Done, thanks for the explanation!

+      [[NSTabView alloc] initWithFrame:NSMakeRect(50, 50, 100, 100)];
+  NSArray* cocoa_tabs = @[
+    [[[NSTabViewItem alloc] init] autorelease],
+    [[[NSTabViewItem alloc] init] autorelease],
+  ];
+  for (size_t i = 0; i < [cocoa_tabs count]; ++i) {
+    [cocoa_tabs[i] setLabel:[NSString stringWithFormat:@"Tab %zu", i + 1]];
+    [cocoa_tab_group addTabViewItem:cocoa_tabs[i]];
+  }
+
+  // General a11y information.
+  EXPECT_NSEQ(
+      [cocoa_tabs[0] accessibilityAttributeValue:NSAccessibilityRoleAttribute],
+      AttributeValueAtPoint(NSAccessibilityRoleAttribute, TabCenterPoint(0)));
+  EXPECT_NSEQ(
+      [cocoa_tabs[0]
+          accessibilityAttributeValue:NSAccessibilityRoleDescriptionAttribute],
+      AttributeValueAtPoint(NSAccessibilityRoleDescriptionAttribute,
+                            TabCenterPoint(0)));
+  EXPECT_NSEQ(
+      [cocoa_tabs[0] accessibilityAttributeValue:NSAccessibilityTitleAttribute],
+      AttributeValueAtPoint(NSAccessibilityTitleAttribute, TabCenterPoint(0)));
+
+  // Check value attribute matches up with whether tabs are actually selected.

[tapted, 2017/01/10 16:22:19]

nit: Compare against native Cocoa values, and check the value attribute...

[Patti Lor, 2017/01/11 05:51:44]

Done.

+  for (int i : {0, 1}) {
+    NSNumber* cocoa_value = [cocoa_tabs[i]
+        accessibilityAttributeValue:NSAccessibilityValueAttribute];
+    EXPECT_EQ(i ? 0 : 1, [cocoa_value intValue]);

[tapted, 2017/01/10 16:22:19]

nit: comment like
    // Verify that only the second tab is selected.

[Patti Lor, 2017/01/11 05:51:44]

Done.

+    EXPECT_NSEQ(cocoa_value,
+                AttributeValueAtPoint(NSAccessibilityValueAttribute,
+                                      TabCenterPoint(i)));
+  }
+
+  // NSTabViewItem doesn't support NSAccessibilitySelectedAttribute, so don't
+  // compare against Cocoa here.
+  EXPECT_TRUE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                     TabCenterPoint(0)) boolValue]);
+  EXPECT_FALSE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                      TabCenterPoint(1)) boolValue]);
+}
+
+// Make sure tabs can be selected by writing the value attribute.
+TEST_F(TabbedPaneAccessibilityMacTest, WritableValue) {
+  id tab1_a11y = A11yElementAtPoint(TabCenterPoint(0));
+  id tab2_a11y = A11yElementAtPoint(TabCenterPoint(1));
+
+  // Only unselected tabs should be writable.
+  EXPECT_FALSE([tab1_a11y
+      accessibilityIsAttributeSettable:NSAccessibilityValueAttribute]);
+  EXPECT_TRUE([tab2_a11y
+      accessibilityIsAttributeSettable:NSAccessibilityValueAttribute]);
+
+  // Select the second tab. Since we are using a field that is actually an
+  // NSString, any valid string value provided should select the tab.
+  [tab2_a11y accessibilitySetValue:@"string"
+                      forAttribute:NSAccessibilityValueAttribute];
+  EXPECT_EQ(0, [AttributeValueAtPoint(NSAccessibilityValueAttribute,
+                                      TabCenterPoint(0)) intValue]);
+  EXPECT_EQ(1, [AttributeValueAtPoint(NSAccessibilityValueAttribute,
+                                      TabCenterPoint(1)) intValue]);
+  EXPECT_FALSE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                      TabCenterPoint(0)) boolValue]);
+  EXPECT_TRUE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                     TabCenterPoint(1)) boolValue]);
+  EXPECT_TRUE(GetTabAt(1)->selected());
+
+  // Try again with an empty string.
+  [tab1_a11y accessibilitySetValue:@""

[tapted, 2017/01/10 16:22:19]

I find this a bit counterintuitive. E.g. maybe setting an empty string, or the
string "0" should just be ignored. But I can see why we don't. I think that
should be called out here. E.g.

// It doesn't make sense to "deselect" a tab (i.e. without specifying another to
select). So any value passed to -accessibilitySetValue: should select that tab.
Try an empty string.

[Patti Lor, 2017/01/11 05:51:44]

Done - I also updated the comment on line 129, too, to say why we're using
NSString to test a field that "should" be a integer/bool.

+                      forAttribute:NSAccessibilityValueAttribute];
+  EXPECT_EQ(1, [AttributeValueAtPoint(NSAccessibilityValueAttribute,
+                                      TabCenterPoint(0)) intValue]);
+  EXPECT_EQ(0, [AttributeValueAtPoint(NSAccessibilityValueAttribute,
+                                      TabCenterPoint(1)) intValue]);
+  EXPECT_TRUE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                     TabCenterPoint(0)) boolValue]);
+  EXPECT_FALSE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                      TabCenterPoint(1)) boolValue]);
+  EXPECT_TRUE(GetTabAt(0)->selected());
+}
+
+}  // namespace test
+

[tapted, 2017/01/10 16:22:19]

nit: remove blank line

[Patti Lor, 2017/01/11 05:51:44]

Done.

+}  // namespace views