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

ui/views/controls/tabbed_pane/tabbed_pane_mac_accessibility_unittest.mm

+// Copyright 2016 The Chromium Authors. All rights reserved.

[tapted, 2017/01/06 04:25:12]

nit: 2017 :)

[Patti Lor, 2017/01/09 22:39:27]

:O Thanks!

+// 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 {
+
+class TabbedPaneMacAccessibilityTest : public test::WidgetTest {
+ public:
+  TabbedPaneMacAccessibilityTest() {}
+
+  void SetUp() override {
+    test::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.
+    View* tab1_contents = new View();

[tapted, 2017/01/06 04:25:11]

nit: move `new View()` into the AddTab calls - no need for these tabX_contents
vars

[Patti Lor, 2017/01/09 22:39:27]

Done.

+    View* tab2_contents = new View();
+    tabbed_pane_->AddTab(base::ASCIIToUTF16("Tab 1"), tab1_contents);
+    tabbed_pane_->AddTab(base::ASCIIToUTF16("Tab 2"), tab2_contents);
+    tabbed_pane_->Layout();
+
+    // Keep references to each Tab and select the first tab for tests.
+    tabbed_pane_->SelectTabAt(0);

[tapted, 2017/01/06 04:25:12]

why not

tab1_ = tabbed_pane_->GetTabAt(0);
tab2_ = tabbed_pane_->GetTabAt(1);

[Patti Lor, 2017/01/09 22:39:27]

Deleted as suggested below.

+    tab1_ = tabbed_pane_->GetSelectedTab();
+    tabbed_pane_->SelectTabAt(1);
+    tab2_ = tabbed_pane_->GetSelectedTab();
+    tabbed_pane_->SelectTabAt(0);
+
+    widget_->GetContentsView()->AddChildView(tabbed_pane_);
+    widget_->Show();
+
+    tab1_center_ = tab1_->GetBoundsInScreen().CenterPoint();
+    tab2_center_ = tab2_->GetBoundsInScreen().CenterPoint();
+  }
+
+  void TearDown() override {
+    widget_->CloseNow();
+    test::WidgetTest::TearDown();
+  }
+
+  id AttributeValueAtPoint(NSString* attribute, gfx::Point point) {

[tapted, 2017/01/06 04:25:12]

nit: const-ref point

[Patti Lor, 2017/01/09 22:39:27]

Done.

+    id value =
+        [A11yElementAtPoint(point) accessibilityAttributeValue:attribute];
+    return value;
+  }
+
+  id A11yElementAtPoint(gfx::Point point) {

[tapted, 2017/01/06 04:25:12]

const-ref

[Patti Lor, 2017/01/09 22:39:27]

Done.

+    // Accessibility hit tests come in Cocoa screen coordinates.
+    NSPoint ns_point = gfx::ScreenPointToNSPoint(point);
+    return [widget_->GetNativeWindow() accessibilityHitTest:ns_point];
+  }
+
+  Widget* widget_;

[tapted, 2017/01/06 04:25:12]

these should be protected:

[Patti Lor, 2017/01/09 22:39:27]

Done.

+  TabbedPane* tabbed_pane_;
+  Tab* tab1_;

[tapted, 2017/01/06 04:25:12]

Actually I don't think we need these - can we add a helper method

Tab* GetTabAt(int index) {
  return tabbed_pane_->GetTabAt(index);
}

[Patti Lor, 2017/01/09 22:39:27]

Done.

+  Tab* tab2_;
+  gfx::Point tab1_center_;

[tapted, 2017/01/06 04:25:12]

same here,

gfx::Point TabCenterPoint(int index) {
  return GetTabAt(index)->GetBoundsInScreen().CenterPoint();
}

[Patti Lor, 2017/01/09 22:39:27]

Done.

+  gfx::Point tab2_center_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TabbedPaneMacAccessibilityTest);
+};
+
+// Test the Tab's a11y information compared to a Cocoa NSTabViewItem.
+TEST_F(TabbedPaneMacAccessibilityTest, Attributes) {

[tapted, 2017/01/06 04:25:11]

Maybe Attributes -> AppKitAtributes -- i.e. something to indicate we are
comparing against native controls.

[Patti Lor, 2017/01/09 22:39:27]

Went for AttributesMatchAppKit, let me know if you think that makes sense.
AppKitAttributes I think just sounds like "the Cocoa attributes".

+  // Create a Cocoa NSTabView to test against and select the first tab.
+  NSTabView* cocoa_tab_group =

[tapted, 2017/01/06 04:25:11]

any `alloc` typically needs to go into a scoped_nsobject straight away
(currently these are memory leaks). E.g.

scoped_nsobject<NSTabView> cocoa_tab_group([[NSTabView alloc] init..]);

[Patti Lor, 2017/01/09 22:39:27]

Used autorelease as suggested below.

+      [[NSTabView alloc] initWithFrame:NSMakeRect(50, 50, 100, 100)];
+  NSTabViewItem* cocoa_tab1 = [[NSTabViewItem alloc] init];

[tapted, 2017/01/06 04:25:12]

we'd need scoped_nsobject for these too, but instead of that I'd suggest
something like

NSArray* cocoa_tabs = @[
  [[[NSTabViewItem alloc] init] autorelease],
  [[[NSTabViewItem alloc] init] autorelease],
];

Then you can add a loop below

[Patti Lor, 2017/01/09 22:39:27]

Done.

+  NSTabViewItem* cocoa_tab2 = [[NSTabViewItem alloc] init];
+  [cocoa_tab1 setLabel:@"Tab 1"];
+  [cocoa_tab2 setLabel:@"Tab 2"];
+  [cocoa_tab_group addTabViewItem:cocoa_tab1];
+  [cocoa_tab_group addTabViewItem:cocoa_tab2];
+  [cocoa_tab_group selectFirstTabViewItem:nil];

[tapted, 2017/01/06 04:25:12]

was this necessary? perhaps either a comment

// Cocoa always <does thing that requires us to select>.

or

EXPECT_EQ([cocoa_tab_group selectedTabViewItem], cocoa_tab0]);

[Patti Lor, 2017/01/09 22:39:27]

Oops, no - removed now.

+
+  // General a11y information.
+  EXPECT_NSEQ(
+      [cocoa_tab1 accessibilityAttributeValue:NSAccessibilityRoleAttribute],
+      AttributeValueAtPoint(NSAccessibilityRoleAttribute, tab1_center_));

[tapted, 2017/01/06 04:25:12]

we should probably guard against the possibility that these _both_ return nil
unexpectedly. Perhaps a check in AttributeValueAtPoint that does EXPECT_NE(nil,
value); before returning

[Patti Lor, 2017/01/09 22:39:27]

Done.

+  EXPECT_NSEQ(
+      [cocoa_tab1
+          accessibilityAttributeValue:NSAccessibilityRoleDescriptionAttribute],
+      AttributeValueAtPoint(NSAccessibilityRoleDescriptionAttribute,
+                            tab1_center_));
+  EXPECT_NSEQ(
+      [cocoa_tab1 accessibilityAttributeValue:NSAccessibilityTitleAttribute],
+      AttributeValueAtPoint(NSAccessibilityTitleAttribute, tab1_center_));
+
+  // Check value attribute matches up with whether tabs are actually selected.
+  // Cocoa uses an int, so convert it back into a NSString.

[tapted, 2017/01/06 04:25:12]

I'm not sure the conversion step makes sense. I.e. If the types are different,
why can't Views use an intValue as well? (could having different types cause
problems for us?).

Should [AXPlatformNodeCocoa AXValue] actually return an (NSValue*) rather than
an (NSString*)?

[Patti Lor, 2017/01/09 22:39:27]

Ooh, thanks for mentioning this. I'd already checked this wondering how this was
possible, but I must have got mixed up between the type of
"NSAccessibilityValueAttribute" and the type of object this attribute is meant
to be associated with it - the header file says

APPKIT_EXTERN NSString *const NSAccessibilityValueAttribute;		//(id)         -
element's value

So this is meant to be attached to an id type (turns out it says so in the
online docs too). I've fixed this here for AXValue in ax_platform_node_mac.mm;
but there are some other ones that are wrong. Filed
https://bugs.chromium.org/p/chromium/issues/detail?id=678898 to fix the others
in a separate CL.

Anyway, the converting to int and to string stuff is gone now :)

+  EXPECT_NSEQ(

[tapted, 2017/01/06 04:25:12]

We should check the actual value too

e.g. 

for (int i : {0, 1}) {
  NSString* value = [cocoa_tab[i]
accessibilityAttributeValue:NSAccessibilityValueAttribute] stringValue];
  EXPECT_NSEQ(i ? @"0" : @"1", value);
  EXPECT_NSEQ(value, AttributeValueAtPoint(NSAccessibilityValueAttribute,
GetTabAt(i)));
}

[Patti Lor, 2017/01/09 22:39:27]

Done.

+      [[cocoa_tab1 accessibilityAttributeValue:NSAccessibilityValueAttribute]
+          stringValue],
+      AttributeValueAtPoint(NSAccessibilityValueAttribute, tab1_center_));
+  EXPECT_NSEQ(
+      [[cocoa_tab2 accessibilityAttributeValue:NSAccessibilityValueAttribute]
+          stringValue],
+      AttributeValueAtPoint(NSAccessibilityValueAttribute, tab2_center_));
+
+  // NSTabViewItem doesn't support NSAccessibilitySelectedAttribute, so don't
+  // compare against Cocoa here.
+  EXPECT_TRUE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                     tab1_center_) boolValue]);
+  EXPECT_FALSE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                      tab2_center_) boolValue]);
+}
+
+// Make sure tabs can be selected by writing the value attribute.
+TEST_F(TabbedPaneMacAccessibilityTest, WritableValue) {
+  id tab1_a11y = A11yElementAtPoint(tab1_center_);
+  id tab2_a11y = A11yElementAtPoint(tab2_center_);
+
+  // 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_NSEQ(
+      @"0", AttributeValueAtPoint(NSAccessibilityValueAttribute, tab1_center_));
+  EXPECT_NSEQ(
+      @"1", AttributeValueAtPoint(NSAccessibilityValueAttribute, tab2_center_));
+  EXPECT_FALSE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                      tab1_center_) boolValue]);
+  EXPECT_TRUE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                     tab2_center_) boolValue]);
+  EXPECT_TRUE(tab2_->selected());
+
+  // Try again with an empty string.
+  [tab1_a11y accessibilitySetValue:@""
+                      forAttribute:NSAccessibilityValueAttribute];
+  EXPECT_NSEQ(
+      @"1", AttributeValueAtPoint(NSAccessibilityValueAttribute, tab1_center_));
+  EXPECT_NSEQ(
+      @"0", AttributeValueAtPoint(NSAccessibilityValueAttribute, tab2_center_));
+  EXPECT_TRUE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                     tab1_center_) boolValue]);
+  EXPECT_FALSE([AttributeValueAtPoint(NSAccessibilitySelectedAttribute,
+                                      tab2_center_) boolValue]);
+  EXPECT_TRUE(tab1_->selected());
+}

[tapted, 2017/01/06 04:25:12]

Can we also test the AXAction "press" - this seems to be what Cocoa makes
available in NSTabView for changing tabs  (maybe add this to the
AppKitAttributes test case and compare with NSTabView behaviour)

[Patti Lor, 2017/01/09 22:39:27]

NSAccessibilityActions aren't hooked up yet to Views. I filed
https://bugs.chromium.org/p/chromium/issues/detail?id=679247 to track it (I
thought this existed already, but I couldn't find it), but I can add a disabled
test to check it if you'd like to have it all in the same CL?

+
+}  // namespace views