views: ax-ignore MenuScrollViewContainer

Right now, MenuScrollViewContainer claims to be an AXMenuBar, which
produces the following puzzling AX tree:

  AXGroup (menu root)
    AXMenuBar (scroll container)
      AXMenu (actual menu view)
        AXMenuItem ...

An AXMenuBar is something like the top system menu bar, at least on
macOS, not the "contents of a menu" or anything similar to that. After
this change, the AX tree is instead:

  AXGroup (menu root)
    AXMenu (actual menu view)
      AXMenuItem ...

which makes much more sense.

Bug: 954701
Change-Id: I00215874d75d9ee4870acd0e2101ef280dc2a199
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1962587
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742311}

ui/views/controls/menu/menu_scroll_view_container.cc

@@ -11,6 +11,7 @@
 #include "ui/accessibility/ax_node_data.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/color_palette.h"
+#include "ui/views/accessibility/view_accessibility.h"
 #include "ui/views/border.h"
 #include "ui/views/bubble/bubble_border.h"
 #include "ui/views/controls/menu/menu_config.h"
@@ -192,6 +193,11 @@ MenuScrollViewContainer::MenuScrollViewContainer(SubmenuView* content_view)
   arrow_ = BubbleBorderTypeFromAnchor(
       content_view_->GetMenuItem()->GetMenuController()->GetAnchorPosition());
 
+  // The MenuScrollViewContainer is a purely presentational container so it
+  // should not appear in the AX tree. Note that its subviews still appear in
+  // the tree!
+  GetViewAccessibility().OverrideRole(ax::mojom::Role::kIgnored);
+
   CreateBorder();
 }
 
@@ -237,12 +243,6 @@ void MenuScrollViewContainer::OnPaintBackground(gfx::Canvas* canvas) {
       NativeTheme::kMenuPopupBackground, NativeTheme::kNormal, bounds, extra);
 }
 
-void MenuScrollViewContainer::GetAccessibleNodeData(ui::AXNodeData* node_data) {
-  // Get the name from the submenu view.
-  content_view_->GetAccessibleNodeData(node_data);
-  node_data->role = ax::mojom::Role::kMenuBar;
-}
-
 void MenuScrollViewContainer::OnBoundsChanged(
     const gfx::Rect& previous_bounds) {
   const bool scroll_buttons_visible =

ui/views/controls/menu/menu_scroll_view_container.h

@@ -36,7 +36,6 @@ class MenuScrollViewContainer : public View {
   // View overrides.
   gfx::Size CalculatePreferredSize() const override;
   void OnPaintBackground(gfx::Canvas* canvas) override;
-  void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   void OnThemeChanged() override;
 
  protected:

ui/views/controls/menu/submenu_view.cc

@@ -395,8 +395,8 @@ void SubmenuView::ShowAt(Widget* parent,
     host_->InitMenuHost(parent, bounds, scroll_view_container_, do_capture);
   }
 
-  GetScrollViewContainer()->NotifyAccessibilityEvent(
-      ax::mojom::Event::kMenuStart, true);
+  host_->GetRootView()->NotifyAccessibilityEvent(ax::mojom::Event::kMenuStart,
+                                                 true);
   NotifyAccessibilityEvent(ax::mojom::Event::kMenuPopupStart, true);
 }
 
@@ -407,12 +407,12 @@ void SubmenuView::Reposition(const gfx::Rect& bounds) {
 
 void SubmenuView::Close() {
   if (host_) {
-    // We send the event to the ScrollViewContainer first because the View
+    // We send the event to the MenuHostRootView first because the View
     // accessibility delegate sets up a focus override when receiving the
     // kMenuStart event that we want to be disabled when we send the
     // kMenuPopupEnd event in order to access the previously focused node.
-    GetScrollViewContainer()->NotifyAccessibilityEvent(
-        ax::mojom::Event::kMenuEnd, true);
+    host_->GetRootView()->NotifyAccessibilityEvent(ax::mojom::Event::kMenuEnd,
+                                                   true);
     NotifyAccessibilityEvent(ax::mojom::Event::kMenuPopupEnd, true);
 
     host_->DestroyMenuHost();