cros: Accessibility fixes for views-based lock.

- Allow ChromeVox to navigate to dropdown arrow using Search+Arrow keys.
- Fix accessibility label for dropdown arrow next to user label.
- Fire alert when showing bubble so ChromeVox reads the contents.

Bug: 757273
Change-Id: I7fc5cc8b730a2b3d796cf040679e0de549f8fc92
Reviewed-on: https://chromium-review.googlesource.com/822979Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Jacob Dufault <jdufault@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523613}

ash/ash_strings.grd

@@ -1134,6 +1134,10 @@ This file contains the strings for ash.
       <message name="IDS_ASH_LOGIN_POD_OWNER_USER" desc="Login/lock screen user pod menu title for a user who owns the device.">
         <ph name="USER_NAME">$1<ex>Ivan Arbuzov</ex></ph> (owner)
       </message>
+      <message name="IDS_ASH_LOGIN_POD_MENU_BUTTON_ACCESSIBLE_NAME" desc="Text to be spoken when the focus is set to the menu button of the user pod on the login screen.">
+        Options menu for <ph name="USER_EMAIL_ADDRESS">$1<ex>john.doe@example.com</ex></ph>
+      </message>
+
 
       <!-- Multi-profiles intro dialog -->
       <message name="IDS_ASH_MULTIPROFILES_INTRO_HEADLINE" desc="Describes which feature multi-profiles intro dialog presents.">

ash/login/ui/login_auth_user_view.cc

@@ -204,8 +204,7 @@ void LoginAuthUserView::SetAuthMethods(uint32_t auth_methods) {
   // case, then render the user view as if it was always focused, since clicking
   // on it will not do anything (such as swapping users).
   user_view_->SetForceOpaque(has_password);
-  user_view_->SetFocusBehavior(has_password ? FocusBehavior::NEVER
-                                            : FocusBehavior::ALWAYS);
+  user_view_->SetTapEnabled(!has_password);
 
   PreferredSizeChanged();
 }

ash/login/ui/login_bubble.cc

@@ -7,6 +7,7 @@
 #include "ash/login/ui/login_button.h"
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/shell.h"
+#include "ui/accessibility/ax_node_data.h"
 #include "ui/compositor/layer_animator.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/gfx/font.h"
@@ -76,6 +77,9 @@ class LoginErrorBubbleView : public LoginBaseBubbleView {
 
   // views::View:
   const char* GetClassName() const override { return "LoginErrorBubbleView"; }
+  void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
+    node_data->role = ui::AX_ROLE_TOOLTIP;
+  }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(LoginErrorBubbleView);
@@ -117,6 +121,11 @@ class LoginTooltipView : public LoginBaseBubbleView {
     AddChildView(text);
   }
 
+  // LoginBaseBubbleView:
+  void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
+    node_data->role = ui::AX_ROLE_TOOLTIP;
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(LoginTooltipView);
 };
@@ -223,6 +232,9 @@ void LoginBubble::Show() {
   bubble_view_->GetWidget()->AddObserver(this);
 
   ScheduleAnimation(true /*visible*/);
+
+  // Fire an alert so ChromeVox will read the contents of the bubble.
+  bubble_view_->NotifyAccessibilityEvent(ui::AX_EVENT_ALERT, true);
 }
 
 void LoginBubble::CloseImmediately() {

ash/login/ui/login_user_view.cc

@@ -29,6 +29,7 @@
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/layout/fill_layout.h"
+#include "ui/views/layout/grid_layout.h"
 #include "ui/views/painter.h"
 
 namespace ash {
@@ -80,12 +81,6 @@ int GetImageSize(LoginDisplayStyle style) {
   return kLargeUserImageSizeDp;
 }
 
-views::View* MakePreferredSizeView(gfx::Size size) {
-  auto* view = new NonAccessibleView();
-  view->SetPreferredSize(size);
-  return view;
-}
-
 }  // namespace
 
 // Renders a user's profile icon.
@@ -193,6 +188,32 @@ class LoginUserView::UserLabel : public NonAccessibleView {
   DISALLOW_COPY_AND_ASSIGN(UserLabel);
 };
 
+// A button embedded inside of LoginUserView, which is activated whenever the
+// user taps anywhere in the LoginUserView. Previously, LoginUserView was a
+// views::Button, but this breaks ChromeVox as it does not expect buttons to
+// have any children (ie, the dropdown button).
+class LoginUserView::TapButton : public views::Button {
+ public:
+  explicit TapButton(LoginUserView* parent)
+      : views::Button(parent), parent_(parent) {}
+  ~TapButton() override = default;
+
+  // views::Button:
+  void OnFocus() override {
+    views::Button::OnFocus();
+    parent_->UpdateOpacity();
+  }
+  void OnBlur() override {
+    views::Button::OnBlur();
+    parent_->UpdateOpacity();
+  }
+
+ private:
+  LoginUserView* const parent_;
+
+  DISALLOW_COPY_AND_ASSIGN(TapButton);
+};
+
 // LoginUserView is defined after LoginUserView::UserLabel so it can access the
 // class members.
 
@@ -212,6 +233,10 @@ views::View* LoginUserView::TestApi::user_label() const {
   return view_->user_label_;
 }
 
+views::View* LoginUserView::TestApi::tap_button() const {
+  return view_->tap_button_;
+}
+
 bool LoginUserView::TestApi::is_opaque() const {
   return view_->is_opaque_;
 }
@@ -234,7 +259,7 @@ int LoginUserView::WidthForLayoutStyle(LoginDisplayStyle style) {
 LoginUserView::LoginUserView(LoginDisplayStyle style,
                              bool show_dropdown,
                              const OnTap& on_tap)
-    : views::Button(this), on_tap_(on_tap), display_style_(style) {
+    : on_tap_(on_tap), display_style_(style) {
   // show_dropdown can only be true when the user view is rendering in large
   // mode.
   DCHECK(!show_dropdown || style == LoginDisplayStyle::kLarge);
@@ -251,6 +276,8 @@ LoginUserView::LoginUserView(LoginDisplayStyle style,
         gfx::CreateVectorIcon(kLockScreenDropdownIcon, SK_ColorWHITE));
     user_dropdown_->SetFocusBehavior(FocusBehavior::ALWAYS);
   }
+  tap_button_ = new TapButton(this);
+  SetTapEnabled(true);
 
   switch (style) {
     case LoginDisplayStyle::kLarge:
@@ -277,8 +304,6 @@ LoginUserView::LoginUserView(LoginDisplayStyle style,
   if (user_dropdown_)
     setup_layer(user_dropdown_);
 
-  SetFocusBehavior(FocusBehavior::ALWAYS);
-
   hover_notifier_ = std::make_unique<HoverNotifier>(
       this, base::Bind(&LoginUserView::OnHover, base::Unretained(this)));
   user_menu_ = std::make_unique<LoginBubble>();
@@ -345,6 +370,11 @@ void LoginUserView::SetForceOpaque(bool force_opaque) {
   UpdateOpacity();
 }
 
+void LoginUserView::SetTapEnabled(bool enabled) {
+  tap_button_->SetFocusBehavior(enabled ? FocusBehavior::ALWAYS
+                                        : FocusBehavior::NEVER);
+}
+
 const char* LoginUserView::GetClassName() const {
   return kUserViewClassName;
 }
@@ -363,17 +393,13 @@ gfx::Size LoginUserView::CalculatePreferredSize() const {
   return gfx::Size();
 }
 
-void LoginUserView::OnFocus() {
-  views::Button::OnFocus();
-  UpdateOpacity();
+void LoginUserView::Layout() {
+  views::View::Layout();
+  tap_button_->SetBoundsRect(GetLocalBounds());
 }
 
-void LoginUserView::OnBlur() {
-  views::Button::OnBlur();
-  UpdateOpacity();
-}
-
-void LoginUserView::ButtonPressed(Button* sender, const ui::Event& event) {
+void LoginUserView::ButtonPressed(views::Button* sender,
+                                  const ui::Event& event) {
   // Handle click on the dropdown arrow.
   if (sender == user_dropdown_) {
     DCHECK(user_dropdown_);
@@ -405,8 +431,13 @@ void LoginUserView::OnHover(bool has_hover) {
 }
 
 void LoginUserView::UpdateCurrentUserState() {
-  SetAccessibleName(
-      base::UTF8ToUTF16(current_user_->basic_user_info->display_email));
+  auto email = base::UTF8ToUTF16(current_user_->basic_user_info->display_email);
+  tap_button_->SetAccessibleName(email);
+  if (user_dropdown_) {
+    user_dropdown_->SetAccessibleName(l10n_util::GetStringFUTF16(
+        IDS_ASH_LOGIN_POD_MENU_BUTTON_ACCESSIBLE_NAME, email));
+  }
+
   user_image_->UpdateForUser(current_user_);
   user_label_->UpdateForUser(current_user_);
   Layout();
@@ -414,8 +445,8 @@ void LoginUserView::UpdateCurrentUserState() {
 
 void LoginUserView::UpdateOpacity() {
   bool was_opaque = is_opaque_;
-  is_opaque_ = force_opaque_ || IsMouseHovered() || HasFocus();
-
+  is_opaque_ =
+      force_opaque_ || tap_button_->IsMouseHovered() || tap_button_->HasFocus();
   if (was_opaque == is_opaque_)
     return;
 
@@ -444,52 +475,68 @@ void LoginUserView::UpdateOpacity() {
 }
 
 void LoginUserView::SetLargeLayout() {
-  auto* root_layout =
-      new views::BoxLayout(views::BoxLayout::kVertical, gfx::Insets(),
-                           kVerticalSpacingBetweenEntriesDp);
-  SetLayoutManager(root_layout);
+  // Add views in tabbing order; they are rendered in a different order below.
+  AddChildView(user_image_);
+  AddChildView(user_label_);
+  AddChildView(tap_button_);
+  if (user_dropdown_)
+    AddChildView(user_dropdown_);
 
-  // Space between top of user view and user icon.
-  AddChildView(MakePreferredSizeView(
-      gfx::Size(0, kDistanceFromTopOfBigUserViewToUserIconDp -
-                       kVerticalSpacingBetweenEntriesDp)));
+  // Use views::GridLayout instead of views::BoxLayout because views::BoxLayout
+  // lays out children according to the view->children order.
+  views::GridLayout* layout = views::GridLayout::CreateAndInstall(this);
 
-  // Centered user image
+  constexpr int kImageColumnId = 0;
+  constexpr int kLabelDropdownColumnId = 1;
   {
-    auto* row = new NonAccessibleView();
-    AddChildView(row);
-
-    auto* layout = new views::BoxLayout(views::BoxLayout::kHorizontal);
-    layout->set_main_axis_alignment(
-        views::BoxLayout::MAIN_AXIS_ALIGNMENT_CENTER);
-    row->SetLayoutManager(layout);
-
-    row->AddChildView(user_image_);
+    views::ColumnSet* image = layout->AddColumnSet(kImageColumnId);
+    image->AddColumn(views::GridLayout::CENTER, views::GridLayout::CENTER,
+                     1 /*resize_percent*/, views::GridLayout::USE_PREF,
+                     0 /*fixed_width*/, 0 /*min_width*/);
   }
 
-  // User name, menu dropdown
   {
-    auto* row = new NonAccessibleView();
-    AddChildView(row);
-
-    auto* layout =
-        new views::BoxLayout(views::BoxLayout::kHorizontal, gfx::Insets(),
-                             kDistanceBetweenUsernameAndDropdownDp);
-    layout->set_main_axis_alignment(
-        views::BoxLayout::MAIN_AXIS_ALIGNMENT_CENTER);
-    row->SetLayoutManager(layout);
-
-    // Add an empty view that has the same size as the dropdown so we center on
-    // |user_label_|. This is simpler than doing manual size calculation to take
-    // into account the extra offset.
+    views::ColumnSet* label_dropdown =
+        layout->AddColumnSet(kLabelDropdownColumnId);
+    label_dropdown->AddPaddingColumn(1.0f /*resize_percent*/, 0 /*width*/);
     if (user_dropdown_) {
-      row->AddChildView(
-          MakePreferredSizeView(user_dropdown_->GetPreferredSize()));
+      label_dropdown->AddPaddingColumn(
+          0 /*resize_percent*/, user_dropdown_->GetPreferredSize().width() +
+                                    kDistanceBetweenUsernameAndDropdownDp);
     }
-    row->AddChildView(user_label_);
-    if (user_dropdown_)
-      row->AddChildView(user_dropdown_);
+    label_dropdown->AddColumn(views::GridLayout::CENTER,
+                              views::GridLayout::CENTER, 0 /*resize_percent*/,
+                              views::GridLayout::USE_PREF, 0 /*fixed_width*/,
+                              0 /*min_width*/);
+    if (user_dropdown_) {
+      label_dropdown->AddPaddingColumn(0 /*resize_percent*/,
+                                       kDistanceBetweenUsernameAndDropdownDp);
+      label_dropdown->AddColumn(views::GridLayout::CENTER,
+                                views::GridLayout::CENTER, 0 /*resize_percent*/,
+                                views::GridLayout::USE_PREF, 0 /*fixed_width*/,
+                                0 /*min_width*/);
+    }
+    label_dropdown->AddPaddingColumn(1.0f /*resize_percent*/, 0 /*width*/);
   }
+
+  auto add_padding = [&](int amount) {
+    layout->AddPaddingRow(0 /*vertical_resize*/, amount /*size*/);
+  };
+
+  // Add views in rendering order.
+  add_padding(kDistanceFromTopOfBigUserViewToUserIconDp);
+
+  // Image
+  layout->StartRow(0 /*vertical_resize*/, kImageColumnId);
+  layout->AddView(user_image_);
+
+  add_padding(kVerticalSpacingBetweenEntriesDp);
+
+  // Label/dropdown.
+  layout->StartRow(0 /*vertical_resize*/, kLabelDropdownColumnId);
+  layout->AddView(user_label_);
+  if (user_dropdown_)
+    layout->AddView(user_dropdown_);
 }
 
 void LoginUserView::SetSmallishLayout() {
@@ -500,6 +547,7 @@ void LoginUserView::SetSmallishLayout() {
 
   AddChildView(user_image_);
   AddChildView(user_label_);
+  AddChildView(tap_button_);
 }
 
 }  // namespace ash

ash/login/ui/login_user_view.h

@@ -20,7 +20,7 @@ class LoginButton;
 
 // Display the user's profile icon, name, and a menu icon in various layout
 // styles.
-class ASH_EXPORT LoginUserView : public views::Button,
+class ASH_EXPORT LoginUserView : public views::View,
                                  public views::ButtonListener {
  public:
   // TestApi is used for tests to get internal implementation details.
@@ -34,6 +34,7 @@ class ASH_EXPORT LoginUserView : public views::Button,
     const base::string16& displayed_name() const;
 
     views::View* user_label() const;
+    views::View* tap_button() const;
 
     bool is_opaque() const;
 
@@ -57,18 +58,24 @@ class ASH_EXPORT LoginUserView : public views::Button,
   // Set if the view must be opaque.
   void SetForceOpaque(bool force_opaque);
 
+  // Enables or disables tapping the view.
+  void SetTapEnabled(bool enabled);
+
   const mojom::LoginUserInfoPtr& current_user() const { return current_user_; }
 
-  // views::Button:
+  // views::View:
   const char* GetClassName() const override;
   gfx::Size CalculatePreferredSize() const override;
-  void OnFocus() override;
-  void OnBlur() override;
+  void Layout() override;
 
   // views::ButtonListener:
-  void ButtonPressed(Button* sender, const ui::Event& event) override;
+  void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
  private:
+  class UserImage;
+  class UserLabel;
+  class TapButton;
+
   // Called when hover state changes.
   void OnHover(bool has_hover);
 
@@ -80,9 +87,6 @@ class ASH_EXPORT LoginUserView : public views::Button,
   void SetLargeLayout();
   void SetSmallishLayout();
 
-  class UserImage;
-  class UserLabel;
-
   // Executed when the user view is pressed.
   OnTap on_tap_;
 
@@ -97,6 +101,7 @@ class ASH_EXPORT LoginUserView : public views::Button,
   UserImage* user_image_ = nullptr;
   UserLabel* user_label_ = nullptr;
   LoginButton* user_dropdown_ = nullptr;
+  TapButton* tap_button_ = nullptr;
   std::unique_ptr<LoginBubble> user_menu_;
 
   // True iff the view is currently opaque (ie, opacity = 1).

ash/login/ui/login_user_view_unittest.cc

@@ -159,10 +159,10 @@ TEST_F(LoginUserViewUnittest, FocusHoverOpaqueInteractions) {
   EXPECT_FALSE(two_test.is_opaque());
 
   // Only the focused element is opaque.
-  one->RequestFocus();
+  one_test.tap_button()->RequestFocus();
   EXPECT_TRUE(one_test.is_opaque());
   EXPECT_FALSE(two_test.is_opaque());
-  two->RequestFocus();
+  two_test.tap_button()->RequestFocus();
   EXPECT_FALSE(one_test.is_opaque());
   EXPECT_TRUE(two_test.is_opaque());
 
@@ -182,7 +182,7 @@ TEST_F(LoginUserViewUnittest, FocusHoverOpaqueInteractions) {
   EXPECT_TRUE(two_test.is_opaque());
 
   // Losing focus (after a mouse hover) makes the element transparent.
-  one->RequestFocus();
+  one_test.tap_button()->RequestFocus();
   EXPECT_TRUE(one_test.is_opaque());
   EXPECT_FALSE(two_test.is_opaque());
 }
@@ -206,21 +206,21 @@ TEST_F(LoginUserViewUnittest, ForcedOpaque) {
   EXPECT_FALSE(two_test.is_opaque());
 
   // Forced opaque stays opaque when gaining or losing focus.
-  one->RequestFocus();
+  one_test.tap_button()->RequestFocus();
   EXPECT_TRUE(one_test.is_opaque());
   EXPECT_FALSE(two_test.is_opaque());
-  two->RequestFocus();
+  two_test.tap_button()->RequestFocus();
   EXPECT_TRUE(one_test.is_opaque());
   EXPECT_TRUE(two_test.is_opaque());
 
   // An element can become transparent when losing forced opaque.
-  EXPECT_TRUE(two->HasFocus());
+  EXPECT_TRUE(two_test.tap_button()->HasFocus());
   one->SetForceOpaque(false);
   EXPECT_FALSE(one_test.is_opaque());
   EXPECT_TRUE(two_test.is_opaque());
 
   // An element can stay opaque when losing forced opaque.
-  EXPECT_TRUE(two->HasFocus());
+  EXPECT_TRUE(two_test.tap_button()->HasFocus());
   two->SetForceOpaque(true);
   EXPECT_FALSE(one_test.is_opaque());
   EXPECT_TRUE(two_test.is_opaque());