coral: virtual cursor focus

This cl fixed two issues:
1. Virtual focus should reveal the close button and announce it.
2. Move the navigation order of the feedback dialog before the
   menu contents.

Test: manual test
Bug: b:396499004, b:396501813
Change-Id: I075723a46bbacb094f2567e5c650777e99ce5451
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6310875
Commit-Queue: Xiaodan Zhu <zxdan@chromium.org>
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1426075}

ash/ash_strings.grd

@@ -9004,7 +9004,7 @@ To shut down the device, press and hold the power button on the device again.
         Items in the suggested group
       </message>
       <message name="IDS_ASH_BIRCH_CORAL_SELECTOR_ITEM_CLOSE_BUTTON_TOOLTIP" desc="The tooltip of the close button on smart grouping expanded menu item">
-        Remove item from suggested group
+        Remove <ph name="ITEM_LABEL">$1<ex>YouTube</ex></ph> from suggested group
       </message>
       <message name="IDS_ASH_BIRCH_CORAL_ADDON_SELECTOR_SHOWN" desc="The accessible name for the birch coral button selector UI.">
         Hide items in <ph name="GROUP_NAME">$1<ex>Trip To Tokyo</ex></ph>
@@ -9012,6 +9012,9 @@ To shut down the device, press and hold the power button on the device again.
       <message name="IDS_ASH_BIRCH_CORAL_ADDON_SELECTOR_HIDDEN" desc="The accessible name for the birch coral button selector UI.">
         Show items in <ph name="GROUP_NAME">$1<ex>Trip To Tokyo</ex></ph>
       </message>
+      <message name="IDS_ASH_BIRCH_CORAL_ADDON_SELECTOR_ITEM_WITH_REMOVE_BUTTON" desc="The accessibile name of selector menu item with a remove button">
+        <ph name="ITEM_LABEL">$1<ex>YouTube</ex></ph> with remove button
+      </message>
       <message name="IDS_ASH_BIRCH_CORAL_SAVED_GROUPS_MAX_NUM_REACHED" desc="Message shown to users when they attempt to save a new coral group when the maximum number of coral saved groups has been reached.">
         Maximum number of saved groups reached.
       </message>

ash/ash_strings_grd/IDS_ASH_BIRCH_CORAL_ADDON_SELECTOR_ITEM_WITH_REMOVE_BUTTON.png.sha1

@@ -0,0 +1 @@
+10a307030674f176137c9866c3ab397ac1ab5c6f

ash/ash_strings_grd/IDS_ASH_BIRCH_CORAL_SELECTOR_ITEM_CLOSE_BUTTON_TOOLTIP.png.sha1

@@ -1 +1 @@
-fe70ea4754613d681b8f76a35a4b7a12f1e07fec
+ea0c6c1a2bd80167c078fedf0332812b077201a7

ash/wm/overview/birch/tab_app_selection_view.cc

@@ -21,6 +21,7 @@
 #include "base/task/cancelable_task_tracker.h"
 #include "chromeos/ash/services/coral/public/mojom/coral_service.mojom.h"
 #include "components/prefs/pref_service.h"
+#include "ui/accessibility/ax_action_data.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/metadata/metadata_impl_macros.h"
 #include "ui/compositor/layer.h"
@@ -145,11 +146,17 @@ class TabAppSelectionView::TabAppSelectionItemView
 
   explicit TabAppSelectionItemView(InitParams params)
       : type_(params.type),
+        title_(base::UTF8ToUTF16(params.title)),
         identifier_(params.identifier),
         owner_(params.owner) {
     views::Builder<views::BoxLayoutView>(this)
         .SetAccessibleRole(ax::mojom::Role::kMenuItem)
-        .SetAccessibleName(base::UTF8ToUTF16(params.title))
+        .SetAccessibleName(
+            params.show_close_button
+                ? l10n_util::GetStringFUTF16(
+                      IDS_ASH_BIRCH_CORAL_ADDON_SELECTOR_ITEM_WITH_REMOVE_BUTTON,
+                      title_)
+                : title_)
         .SetBetweenChildSpacing(kItemChildSpacing)
         .SetCrossAxisAlignment(views::LayoutAlignment::kCenter)
         .SetFocusBehavior(views::View::FocusBehavior::ACCESSIBLE_ONLY)
@@ -171,6 +178,7 @@ class TabAppSelectionView::TabAppSelectionItemView
                 .CustomConfigure(base::BindOnce([](views::Label* label) {
                   TypographyProvider::Get()->StyleLabel(
                       TypographyToken::kCrosButton2, *label);
+                  label->GetViewAccessibility().SetIsIgnored(true);
                 })))
         .BuildChildren();
 
@@ -189,8 +197,8 @@ class TabAppSelectionView::TabAppSelectionItemView
       close_button_->layer()->SetOpacity(0.f);
       close_button_->SetEnabled(false);
       close_button_->SetID(TabAppSelectionView::kCloseButtonID);
-      close_button_->SetTooltipText(l10n_util::GetStringUTF16(
-          IDS_ASH_BIRCH_CORAL_SELECTOR_ITEM_CLOSE_BUTTON_TOOLTIP));
+      close_button_->SetTooltipText(l10n_util::GetStringFUTF16(
+          IDS_ASH_BIRCH_CORAL_SELECTOR_ITEM_CLOSE_BUTTON_TOOLTIP, title_));
       views::FocusRing::Get(close_button_)
           ->SetHasFocusPredicate(base::BindRepeating(
               [](const TabAppSelectionView* owner, const views::View* view) {
@@ -261,6 +269,7 @@ class TabAppSelectionView::TabAppSelectionItemView
       return;
     }
     RemoveChildViewT(std::exchange(close_button_, nullptr));
+    SetAccessibleName(title_);
   }
 
   // views::BoxLayoutView:
@@ -278,6 +287,15 @@ class TabAppSelectionView::TabAppSelectionItemView
   void OnFocus() override { owner_->MoveFocus(this); }
   void OnBlur() override { owner_->MoveFocus(nullptr); }
 
+  bool HandleAccessibleAction(const ui::AXActionData& action_data) override {
+    // Override accessibility focus behavior.
+    if (action_data.action == ax::mojom::Action::kFocus) {
+      owner_->MoveFocus(this);
+      return true;
+    }
+    return views::BoxLayoutView::HandleAccessibleAction(action_data);
+  }
+
  private:
   void OnCloseButtonPressed() {
     // `this` will be destroyed.
@@ -285,6 +303,7 @@ class TabAppSelectionView::TabAppSelectionItemView
   }
 
   const InitParams::Type type_;
+  const std::u16string title_;
   const std::string identifier_;
 
   // True when the mouse is hovered over this view. The background is painted
@@ -656,6 +675,10 @@ std::optional<size_t> TabAppSelectionView::GetSelectedIndex() const {
 
 std::vector<views::View*> TabAppSelectionView::BuildFocusList() {
   std::vector<views::View*> focus_list;
+  focus_list.emplace_back(GetViewByID(ViewID::kUserFeedbackID));
+  focus_list.emplace_back(GetViewByID(ViewID::kThumbsUpID));
+  focus_list.emplace_back(GetViewByID(ViewID::kThumbsDownID));
+
   std::optional<size_t> selected_index = GetSelectedIndex();
   const raw_ptr<TabAppSelectionItemView>& item_view =
       selected_index ? item_views_[*selected_index] : item_views_[0];
@@ -663,9 +686,6 @@ std::vector<views::View*> TabAppSelectionView::BuildFocusList() {
   if (item_view->close_button()) {
     focus_list.emplace_back(item_view->close_button());
   }
-  focus_list.emplace_back(GetViewByID(ViewID::kUserFeedbackID));
-  focus_list.emplace_back(GetViewByID(ViewID::kThumbsUpID));
-  focus_list.emplace_back(GetViewByID(ViewID::kThumbsDownID));
 
   return focus_list;
 }

ash/wm/overview/birch/tab_app_selection_view_unittest.cc

@@ -234,15 +234,9 @@ class TabAppKeyboardNavigationTest : public TabAppSelectionViewTest {
 
 // Tests the forward Tab navigation works as expected.
 TEST_F(TabAppKeyboardNavigationTest, TabForward) {
-  // The tab navigation should go through the first item, first item's close
-  // button, user feedback, thumbs up button, thumbs down button, coral
-  // chip, and coral chip add-on button in forward order.
-  PressAndReleaseKey(ui::VKEY_TAB);
-  EXPECT_TRUE(IsViewFocusedOnSelector(GetItemViewAt(0)));
-
-  PressAndReleaseKey(ui::VKEY_TAB);
-  EXPECT_TRUE(IsViewFocusedOnSelector(GetItemCloseButtonAt(0)));
-
+  // The tab navigation should go through the user feedback, thumbs up button,
+  // thumbs down button, first item, first item's close button, coral chip, and
+  // coral chip add-on button in forward order.
   PressAndReleaseKey(ui::VKEY_TAB);
   EXPECT_TRUE(IsViewFocusedOnSelector(
       selector_view_->GetViewByID(TabAppSelectionView::kUserFeedbackID)));
@@ -254,6 +248,11 @@ TEST_F(TabAppKeyboardNavigationTest, TabForward) {
   PressAndReleaseKey(ui::VKEY_TAB);
   EXPECT_TRUE(IsViewFocusedOnSelector(
       selector_view_->GetViewByID(TabAppSelectionView::kThumbsDownID)));
+  PressAndReleaseKey(ui::VKEY_TAB);
+  EXPECT_TRUE(IsViewFocusedOnSelector(GetItemViewAt(0)));
+
+  PressAndReleaseKey(ui::VKEY_TAB);
+  EXPECT_TRUE(IsViewFocusedOnSelector(GetItemCloseButtonAt(0)));
 
   PressAndReleaseKey(ui::VKEY_TAB);
   EXPECT_TRUE(IsViewFocusedOnChip(GetFirstCoralButton()));
@@ -264,6 +263,12 @@ TEST_F(TabAppKeyboardNavigationTest, TabForward) {
 
 // Tests the reverse Tab navigation works as expected.
 TEST_F(TabAppKeyboardNavigationTest, TabBackward) {
+  PressAndReleaseKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN);
+  EXPECT_TRUE(IsViewFocusedOnSelector(GetItemCloseButtonAt(0)));
+
+  PressAndReleaseKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN);
+  EXPECT_TRUE(IsViewFocusedOnSelector(GetItemViewAt(0)));
+
   PressAndReleaseKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN);
   EXPECT_TRUE(IsViewFocusedOnSelector(
       selector_view_->GetViewByID(TabAppSelectionView::kThumbsDownID)));
@@ -276,12 +281,6 @@ TEST_F(TabAppKeyboardNavigationTest, TabBackward) {
   EXPECT_TRUE(IsViewFocusedOnSelector(
       selector_view_->GetViewByID(TabAppSelectionView::kUserFeedbackID)));
 
-  PressAndReleaseKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN);
-  EXPECT_TRUE(IsViewFocusedOnSelector(GetItemCloseButtonAt(0)));
-
-  PressAndReleaseKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN);
-  EXPECT_TRUE(IsViewFocusedOnSelector(GetItemViewAt(0)));
-
   PressAndReleaseKey(ui::VKEY_TAB, ui::EF_SHIFT_DOWN);
   EXPECT_TRUE(IsViewFocusedOnChip(GetFirstCoralButton()->addon_view()));