Ash Tray: Do not announce certain buttons as toggle buttons for ChromeVox.

Certain feature pod buttons (e.g. accesibility) are not togglable, but are
currently announced as such for ChromeVox spoken feedback.

BUG=1009727

Change-Id: I2db6c3a53bb04ba677457ff5590574d2c384b503
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848454
Commit-Queue: Tim Song <tengs@chromium.org>
Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704913}

ash/system/accessibility/accessibility_feature_pod_controller.cc

@@ -24,7 +24,7 @@ AccessibilityFeaturePodController::~AccessibilityFeaturePodController() =
     default;
 
 FeaturePodButton* AccessibilityFeaturePodController::CreateButton() {
-  auto* button = new FeaturePodButton(this);
+  auto* button = new FeaturePodButton(this, /*is_togglable=*/false);
   button->SetID(ash::VIEW_ID_ACCESSIBILITY_TRAY_ITEM);
   button->SetLabel(
       l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_ACCESSIBILITY));

ash/system/ime/ime_feature_pod_controller.cc

@@ -64,7 +64,7 @@ IMEFeaturePodController::~IMEFeaturePodController() {
 }
 
 FeaturePodButton* IMEFeaturePodController::CreateButton() {
-  button_ = new FeaturePodButton(this);
+  button_ = new FeaturePodButton(this, /*is_togglable=*/false);
   button_->SetVectorIcon(kUnifiedMenuKeyboardIcon);
   button_->SetLabel(l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_IME_SHORT));
   button_->SetIconAndLabelTooltips(GetTooltipString());

ash/system/locale/locale_feature_pod_controller.cc

@@ -24,7 +24,7 @@ LocaleFeaturePodController::LocaleFeaturePodController(
 LocaleFeaturePodController::~LocaleFeaturePodController() = default;
 
 FeaturePodButton* LocaleFeaturePodController::CreateButton() {
-  auto* button = new FeaturePodButton(this);
+  auto* button = new FeaturePodButton(this, /*is_togglable=*/false);
   const bool visible =
       !Shell::Get()->system_tray_model()->locale()->locale_list().empty();
   button->SetVisible(visible);

ash/system/unified/feature_pod_button.cc

@@ -42,8 +42,9 @@ void ConfigureFeaturePodLabel(views::Label* label) {
 
 }  // namespace
 
-FeaturePodIconButton::FeaturePodIconButton(views::ButtonListener* listener)
-    : views::ImageButton(listener) {
+FeaturePodIconButton::FeaturePodIconButton(views::ButtonListener* listener,
+                                           bool is_togglable)
+    : views::ImageButton(listener), is_togglable_(is_togglable) {
   SetPreferredSize(kUnifiedFeaturePodIconSize);
   SetBorder(views::CreateEmptyBorder(kUnifiedFeaturePodIconPadding));
   SetImageHorizontalAlignment(ALIGN_CENTER);
@@ -55,6 +56,9 @@ FeaturePodIconButton::FeaturePodIconButton(views::ButtonListener* listener)
 FeaturePodIconButton::~FeaturePodIconButton() = default;
 
 void FeaturePodIconButton::SetToggled(bool toggled) {
+  if (!is_togglable_)
+    return;
+
   toggled_ = toggled;
   SchedulePaint();
 }
@@ -114,9 +118,13 @@ std::unique_ptr<views::InkDropMask> FeaturePodIconButton::CreateInkDropMask()
 void FeaturePodIconButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
   ImageButton::GetAccessibleNodeData(node_data);
   node_data->SetName(GetTooltipText(gfx::Point()));
-  node_data->role = ax::mojom::Role::kToggleButton;
-  node_data->SetCheckedState(toggled_ ? ax::mojom::CheckedState::kTrue
-                                      : ax::mojom::CheckedState::kFalse);
+  if (is_togglable_) {
+    node_data->role = ax::mojom::Role::kToggleButton;
+    node_data->SetCheckedState(toggled_ ? ax::mojom::CheckedState::kTrue
+                                        : ax::mojom::CheckedState::kFalse);
+  } else {
+    node_data->role = ax::mojom::Role::kButton;
+  }
 }
 
 const char* FeaturePodIconButton::GetClassName() const {
@@ -270,9 +278,10 @@ void FeaturePodLabelButton::LayoutInCenter(views::View* child, int y) {
       child_width, preferred_size.height());
 }
 
-FeaturePodButton::FeaturePodButton(FeaturePodControllerBase* controller)
+FeaturePodButton::FeaturePodButton(FeaturePodControllerBase* controller,
+                                   bool is_togglable)
     : controller_(controller),
-      icon_button_(new FeaturePodIconButton(this)),
+      icon_button_(new FeaturePodIconButton(this, is_togglable)),
       label_button_(new FeaturePodLabelButton(this)) {
   auto* layout = SetLayoutManager(std::make_unique<views::BoxLayout>(
       views::BoxLayout::Orientation::kVertical, gfx::Insets(),

ash/system/unified/feature_pod_button.h

@@ -23,7 +23,7 @@ class FeaturePodControllerBase;
 // ImageButon internally used in FeaturePodButton. Should not be used directly.
 class FeaturePodIconButton : public views::ImageButton {
  public:
-  explicit FeaturePodIconButton(views::ButtonListener* listener);
+  FeaturePodIconButton(views::ButtonListener* listener, bool is_togglable);
   ~FeaturePodIconButton() override;
 
   // Change the toggle state. See FeaturePodButton::SetToggled.
@@ -42,7 +42,10 @@ class FeaturePodIconButton : public views::ImageButton {
   bool toggled() const { return toggled_; }
 
  private:
-  // Ture if the button is currently toggled.
+  // True if this button is a togglable.
+  const bool is_togglable_;
+
+  // True if the button is currently toggled.
   bool toggled_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(FeaturePodIconButton);
@@ -95,13 +98,15 @@ class FeaturePodLabelButton : public views::Button {
 
 // A button in FeaturePodsView. These buttons are main entry points of features
 // in UnifiedSystemTray. Each button has its icon, label, and sub-label placed
-// vertically. They are also togglable and the background color indicates the
-// current state.
+// vertically. The button may be togglable and the background color indicates
+// the current state. Otherwise, the button is not a toggle button and just
+// navigates to the appropriate detailed view.
 // See the comment in FeaturePodsView for detail.
 class ASH_EXPORT FeaturePodButton : public views::View,
                                     public views::ButtonListener {
  public:
-  explicit FeaturePodButton(FeaturePodControllerBase* controller);
+  FeaturePodButton(FeaturePodControllerBase* controller,
+                   bool is_togglable = true);
   ~FeaturePodButton() override;
 
   // Set the vector icon shown in a circle.
@@ -130,7 +135,8 @@ class ASH_EXPORT FeaturePodButton : public views::View,
   void DisableLabelButtonFocus();
 
   // Change the toggled state. If toggled, the background color of the circle
-  // will change.
+  // will change. If the button is not togglable, then SetToggled() will do
+  // nothing and |IsToggled()| will always return false.
   void SetToggled(bool toggled);
   bool IsToggled() const { return icon_button_->toggled(); }
 
@@ -162,7 +168,6 @@ class ASH_EXPORT FeaturePodButton : public views::View,
 
   bool visible_preferred() const { return visible_preferred_; }
 
- protected:
   FeaturePodIconButton* icon_button() const { return icon_button_; }
 
  private:

ash/system/unified/feature_pods_container_view_unittest.cc

@@ -443,4 +443,18 @@ TEST_F(FeaturePodsContainerViewTest, PaginationMouseWheelHandling) {
   }
 }
 
+TEST_F(FeaturePodsContainerViewTest, NonTogglableButton) {
+  // Add one togglable and one non-tobblable button.
+  buttons_.push_back(new FeaturePodButton(this, /*is_togglable=*/false));
+  AddButtons(1);
+
+  // Non-togglable buttons should be labelled as a regular button for
+  // accessibility and vice versa.
+  ui::AXNodeData ax_node_data;
+  buttons_[0]->icon_button()->GetAccessibleNodeData(&ax_node_data);
+  EXPECT_EQ(ax_node_data.role, ax::mojom::Role::kButton);
+  buttons_[1]->icon_button()->GetAccessibleNodeData(&ax_node_data);
+  EXPECT_EQ(ax_node_data.role, ax::mojom::Role::kToggleButton);
+}
+
 }  // namespace ash