Defer menu start event until user moves into autofill menu

Defer the menu start event, so that it does not conflict with
form control focus announcements.

Bug: 990476
Change-Id: I3c2559080aa8ccde5e515977077ae8dd08ce251f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1758587Reviewed-by: Evan Stade <estade@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Akihiro Ota <akihiroota@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688679}

chrome/browser/ui/views/autofill/autofill_popup_base_view.cc

@@ -290,17 +290,6 @@ void AutofillPopupBaseView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
       l10n_util::GetStringUTF16(IDS_AUTOFILL_POPUP_ACCESSIBLE_NODE_DATA));
 }
 
-void AutofillPopupBaseView::VisibilityChanged(View* starting_from,
-                                              bool is_visible) {
-  // Fire these the first time a menu is visible. By firing these and the
-  // matching end events, we are telling screen readers that the focus
-  // is only changing temporarily, and the screen reader will restore the
-  // focus back to the appropriate textfield when the menu closes.
-  NotifyAccessibilityEvent(
-      is_visible ? ax::mojom::Event::kMenuStart : ax::mojom::Event::kMenuEnd,
-      true);
-}
-
 void AutofillPopupBaseView::SetSelection(const gfx::Point& point) {
   if (delegate_)
     delegate_->SetSelectionAtPoint(point);

chrome/browser/ui/views/autofill/autofill_popup_base_view.h

@@ -74,7 +74,6 @@ class AutofillPopupBaseView : public views::WidgetDelegateView,
   void OnMouseReleased(const ui::MouseEvent& event) override;
   void OnGestureEvent(ui::GestureEvent* event) override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
-  void VisibilityChanged(View* starting_from, bool is_visible) override;
 
   // views::WidgetFocusChangeListener implementation.
   void OnNativeFocusChanged(gfx::NativeView focused_now) override;

chrome/browser/ui/views/autofill/autofill_popup_view_native_views.cc

@@ -861,6 +861,19 @@ void AutofillPopupViewNativeViews::GetAccessibleNodeData(
       l10n_util::GetStringUTF16(IDS_AUTOFILL_POPUP_ACCESSIBLE_NODE_DATA));
 }
 
+void AutofillPopupViewNativeViews::VisibilityChanged(View* starting_from,
+                                                     bool is_visible) {
+  // Fire menu end event. The menu start event is delayed until the user
+  // navigates into the menu, otherwise some screen readers will ignore
+  // any focus events outside of the menu, including a focus event on
+  // the form control itself.
+  if (!is_visible) {
+    if (is_ax_menu_start_event_fired_)
+      NotifyAccessibilityEvent(ax::mojom::Event::kMenuEnd, true);
+    is_ax_menu_start_event_fired_ = false;
+  }
+}
+
 void AutofillPopupViewNativeViews::OnThemeChanged() {
   SetBackground(views::CreateSolidBackground(GetBackgroundColor()));
   // |body_container_| and |footer_container_| will be null if there is no body
@@ -896,6 +909,17 @@ void AutofillPopupViewNativeViews::OnSelectedRowChanged(
 
   if (current_row_selection)
     rows_[*current_row_selection]->SetSelected(true);
+
+  if (!is_ax_menu_start_event_fired_) {
+    // By firing these and the matching kMenuEnd events, we are telling screen
+    // readers that the focus is only changing temporarily, and the screen
+    // reader will restore the focus back to the appropriate textfield when the
+    // menu closes. Wait until item is selected before firing, otherwise if it's
+    // fired when the menu is first visible, then the focus on the form control
+    // is not read.
+    NotifyAccessibilityEvent(ax::mojom::Event::kMenuStart, true);
+    is_ax_menu_start_event_fired_ = true;
+  }
 }
 
 void AutofillPopupViewNativeViews::OnSuggestionsChanged() {

chrome/browser/ui/views/autofill/autofill_popup_view_native_views.h

@@ -76,6 +76,7 @@ class AutofillPopupViewNativeViews : public AutofillPopupBaseView,
   // views::View:
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   void OnThemeChanged() override;
+  void VisibilityChanged(View* starting_from, bool is_visible) override;
 
   // AutofillPopupView:
   void Show() override;
@@ -112,6 +113,8 @@ class AutofillPopupViewNativeViews : public AutofillPopupBaseView,
   views::View* body_container_ = nullptr;
   views::View* footer_container_ = nullptr;
 
+  bool is_ax_menu_start_event_fired_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(AutofillPopupViewNativeViews);
 };