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

ui/views/controls/tabbed_pane/tabbed_pane.cc

Index: ui/views/controls/tabbed_pane/tabbed_pane.cc
diff --git a/ui/views/controls/tabbed_pane/tabbed_pane.cc b/ui/views/controls/tabbed_pane/tabbed_pane.cc
index f36f68083227ffe0517ac13fcd8a703def3e8bff..ab2dd740151177e741957f2d8cc79b5d141c67cf 100644
--- a/ui/views/controls/tabbed_pane/tabbed_pane.cc
+++ b/ui/views/controls/tabbed_pane/tabbed_pane.cc
@@ -8,6 +8,7 @@
 #include "base/macros.h"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "third_party/skia/include/core/SkPath.h"
+#include "ui/accessibility/ax_action_data.h"
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/base/default_style.h"
 #include "ui/base/material_design/material_design_controller.h"
@@ -27,6 +28,8 @@
 #include "ui/views/layout/layout_manager.h"
 #include "ui/views/widget/widget.h"
 
+namespace views {
+
 namespace {
 
 // TODO(markusheintz|msw): Use NativeTheme colors.
@@ -42,9 +45,26 @@ const gfx::Font::Weight kInactiveWeight = gfx::Font::Weight::NORMAL;
 
 const int kHarmonyTabStripTabHeight = 40;
 
-}  // namespace
+// views::Tab shouldn't expose any of its children in the a11y tree. Instead, it

[tapted, 2017/01/03 00:29:27]

nit: move comment into GetAccessibleNodeData. the comment here should just be
something like "The View containing the text for each tab in the tab strip."

[Patti Lor, 2017/01/06 02:02:29]

Done.

+// should provide the a11y information itself. Normally, non-keyboard-focusable
+// children of keyboard-focusable parents are ignored, but Tabs only mark the
+// currently selected tab as keyboard-focusable. This means all unselected Tabs
+// expose their children to the a11y tree. To fix, manually ignore the children.
+class TabLabel : public Label {
+ public:
+  TabLabel(const base::string16& tab_title)

[tapted, 2017/01/03 00:29:27]

nit; explicit

[Patti Lor, 2017/01/06 02:02:29]

Done.

+      : Label(tab_title,
+              ui::ResourceBundle::GetSharedInstance().GetFontListWithDelta(
+                  ui::kLabelFontSizeDelta,
+                  gfx::Font::NORMAL,
+                  kActiveWeight)) {}
+
+  void GetAccessibleNodeData(ui::AXNodeData* data) override {

[tapted, 2017/01/03 00:29:27]

nit: // Label:

[Patti Lor, 2017/01/06 02:02:30]

Done.

+    data->role = ui::AX_ROLE_IGNORED;
+  }
+};
 
-namespace views {
+}  // namespace
 
 // static
 const char TabbedPane::kViewClassName[] = "TabbedPane";
@@ -132,12 +152,7 @@ const char Tab::kViewClassName[] = "Tab";
 
 Tab::Tab(TabbedPane* tabbed_pane, const base::string16& title, View* contents)
     : tabbed_pane_(tabbed_pane),
-      title_(new Label(
-          title,
-          ui::ResourceBundle::GetSharedInstance().GetFontListWithDelta(
-              ui::kLabelFontSizeDelta,
-              gfx::Font::NORMAL,
-              kActiveWeight))),
+      title_(new TabLabel(title)),
       tab_state_(TAB_ACTIVE),
       contents_(contents) {
   // Calculate this now while the font list is guaranteed to be bold.
@@ -238,6 +253,24 @@ void Tab::SetState(TabState tab_state) {
   SchedulePaint();
 }
 
+void Tab::GetAccessibleNodeData(ui::AXNodeData* data) {
+  data->role = ui::AX_ROLE_TAB;
+  data->SetName(title()->text());
+  data->AddStateFlag(ui::AX_STATE_SELECTABLE);
+  if (selected())
+    data->AddStateFlag(ui::AX_STATE_SELECTED);
+}
+
+bool Tab::HandleAccessibleAction(const ui::AXActionData& action_data) {
+  if (action_data.action != ui::AX_ACTION_SET_SELECTION || !enabled())
+    return false;
+  // It's not clear what should happen if a tab is 'deselected', so having the
+  // AX_ACTION_SET_SELECTION action always selects the tab.
+  if (!selected())

[tapted, 2017/01/03 00:29:27]

Can we just ignore requests to SET_SELECTION to 0/false? Then I would remove
this check (just call SelectTab(this).

The hpothetical risk is that if we have Tab(0) selected, something might decide
to do a sequence like: Select(Tab(1)) && Unselect(Tab(0))

[Patti Lor, 2017/01/06 02:02:30]

Yep - so on Mac, I've done this by reporting selected tabs's AXValue as
'uneditable', so as long as the a11y client respects that the "if" here won't be
needed. I mostly just left that in there kinda "just in case". Have deleted it
now.

+    tabbed_pane_->SelectTab(this);
+  return true;
+}
+
 void Tab::OnFocus() {
   OnStateChanged();
   // When the tab gains focus, send an accessibility event indicating that the