feat: add support for keyboard initialized menu popup (#38903)

* feat: add support for keyboard initialized menu popup

* Update docs/api/menu.md

Co-authored-by: Erick Zhao <erick@hotmail.ca>

* fix: add patch to chromium for keyboard accessibility menu behavior

* refactor: s/initiatedByKeyboard/sourceType

* fix: ignore initial mouse event to retain keyboard initiated focus

* Update docs/api/menu.md

Co-authored-by: Samuel Maddock <samuel.maddock@gmail.com>

---------

Co-authored-by: Erick Zhao <erick@hotmail.ca>
Co-authored-by: Samuel Maddock <samuel.maddock@gmail.com>

docs/api/menu.md

@@ -80,6 +80,10 @@ The `menu` object has the following instance methods:
   * `positioningItem` number (optional) _macOS_ - The index of the menu item to
     be positioned under the mouse cursor at the specified coordinates. Default
     is -1.
+  * `sourceType` string (optional) _Windows_ _Linux_ - This should map to the `menuSourceType`
+    provided by the `context-menu` event. It is not recommended to set this value manually,
+    only provide values you receive from other APIs or leave it `undefined`.
+    Can be `none`, `mouse`, `keyboard`, `touch`, `touchMenu`, `longPress`, `longTap`, `touchHandle`, `stylus`, `adjustSelection`, or `adjustSelectionReset`.
   * `callback` Function (optional) - Called when menu is closed.
 
 Pops up this menu as a context menu in the [`BrowserWindow`](browser-window.md).

lib/browser/api/menu.ts

@@ -69,7 +69,7 @@ Menu.prototype.popup = function (options = {}) {
   if (options == null || typeof options !== 'object') {
     throw new TypeError('Options must be an object');
   }
-  let { window, x, y, positioningItem, callback } = options;
+  let { window, x, y, positioningItem, sourceType, callback } = options;
 
   // no callback passed
   if (!callback || typeof callback !== 'function') callback = () => {};
@@ -78,6 +78,7 @@ Menu.prototype.popup = function (options = {}) {
   if (typeof x !== 'number') x = -1;
   if (typeof y !== 'number') y = -1;
   if (typeof positioningItem !== 'number') positioningItem = -1;
+  if (typeof sourceType !== 'string' || !sourceType) sourceType = 'mouse';
 
   // find which window to use
   const wins = BaseWindow.getAllWindows();
@@ -91,7 +92,7 @@ Menu.prototype.popup = function (options = {}) {
     }
   }
 
-  this.popupAt(window as unknown as BaseWindow, x, y, positioningItem, callback);
+  this.popupAt(window as unknown as BaseWindow, x, y, positioningItem, sourceType, callback);
   return { browserWindow: window, x, y, position: positioningItem };
 };
 

patches/chromium/.patches

@@ -128,3 +128,4 @@ fix_remove_profiles_from_spellcheck_service.patch
 chore_patch_out_profile_methods_in_chrome_browser_pdf.patch
 chore_patch_out_profile_methods_in_titlebar_config.patch
 fix_crash_on_nativetheme_change_during_context_menu_close.patch
+fix_select_the_first_menu_item_when_opened_via_keyboard.patch

patches/chromium/fix_select_the_first_menu_item_when_opened_via_keyboard.patch

@@ -0,0 +1,182 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Samuel Attard <marshallofsound@electronjs.org>
+Date: Mon, 26 Jun 2023 00:50:45 -0700
+Subject: fix: select the first menu item when opened via keyboard
+
+This fixes an accessibility issue where the root view is 'focused' to the screen reader instead of the first menu item as with all other native menus. This patch will be upstreamed.
+
+diff --git a/ui/views/controls/menu/menu_controller.cc b/ui/views/controls/menu/menu_controller.cc
+index f8b3ded1dba82259f7d536ed5bab1a972d182419..b095843c8b28783bf51c0bf7becb111010651486 100644
+--- a/ui/views/controls/menu/menu_controller.cc
++++ b/ui/views/controls/menu/menu_controller.cc
+@@ -563,6 +563,7 @@ void MenuController::Run(Widget* parent,
+                          MenuAnchorPosition position,
+                          bool context_menu,
+                          bool is_nested_drag,
++                         ui::MenuSourceType source_type,
+                          gfx::NativeView native_view_for_gestures) {
+   exit_type_ = ExitType::kNone;
+   possible_drag_ = false;
+@@ -627,6 +628,14 @@ void MenuController::Run(Widget* parent,
+   // Set the selection, which opens the initial menu.
+   SetSelection(root, SELECTION_OPEN_SUBMENU | SELECTION_UPDATE_IMMEDIATELY);
+ 
++  if (source_type == ui::MENU_SOURCE_KEYBOARD && context_menu && root->HasSubmenu()) {
++    // For context menus opened via the keyboard we select the first item by default
++    // to match accessibility expectations
++    MenuItemView* first_item = FindInitialSelectableMenuItem(root, INCREMENT_SELECTION_DOWN);
++    if (first_item)
++      SetSelection(first_item, SELECTION_UPDATE_IMMEDIATELY);
++  }
++
+   if (button_controller) {
+     pressed_lock_ = button_controller->TakeLock(
+         false, ui::LocatedEvent::FromIfValid(event));
+@@ -2262,19 +2271,15 @@ void MenuController::OpenMenuImpl(MenuItemView* item, bool show) {
+     }
+     item->GetSubmenu()->ShowAt(params);
+ 
+-    // Figure out if the mouse is under the menu; if so, remember the mouse
+-    // location so we can ignore the first mouse move event(s) with that
+-    // location. We do this after `ShowAt` because `ConvertFromScreen` doesn't
+-    // work correctly if the widget isn't shown.
++    // Remember the mouse location so we can ignore the first mouse move
++    // event(s) with that location. We do this after `ShowAt` because
++    // `ConvertFromScreen` doesn't work correctly if the widget isn't shown.
+     if (item->GetSubmenu()->GetWidget()) {
+       const gfx::Point mouse_pos = ConvertFromScreen(
+           *item->submenu_,
+           display::Screen::GetScreen()->GetCursorScreenPoint());
+-      MenuPart part_under_mouse = GetMenuPart(item->submenu_, mouse_pos);
+-      if (part_under_mouse.type != MenuPartType::kNone) {
+-        menu_open_mouse_loc_ =
+-            GetLocationInRootMenu(*item->submenu_, mouse_pos);
+-      }
++      menu_open_mouse_loc_ =
++          GetLocationInRootMenu(*item->submenu_, mouse_pos);
+     }
+ 
+     item->GetSubmenu()->GetWidget()->SetNativeWindowProperty(
+diff --git a/ui/views/controls/menu/menu_controller.h b/ui/views/controls/menu/menu_controller.h
+index e10b92be74908f8d96e421843cb2231f712a4895..82dc2f014d2e333057836290a00c7142b460bb57 100644
+--- a/ui/views/controls/menu/menu_controller.h
++++ b/ui/views/controls/menu/menu_controller.h
+@@ -134,6 +134,7 @@ class VIEWS_EXPORT MenuController
+            MenuAnchorPosition position,
+            bool context_menu,
+            bool is_nested_drag,
++           ui::MenuSourceType source_type,
+            gfx::NativeView native_view_for_gestures = gfx::NativeView());
+ 
+   bool for_drop() const { return for_drop_; }
+diff --git a/ui/views/controls/menu/menu_runner.cc b/ui/views/controls/menu/menu_runner.cc
+index adb22671b94fa16854773baad0e6bff1322c6646..fae32722f1209151fa1da59d0c7892aba8956108 100644
+--- a/ui/views/controls/menu/menu_runner.cc
++++ b/ui/views/controls/menu/menu_runner.cc
+@@ -82,7 +82,7 @@ void MenuRunner::RunMenuAt(Widget* parent,
+   }
+ 
+   impl_->RunMenuAt(parent, button_controller, bounds, anchor, run_types_,
+-                   native_view_for_gestures, corners);
++                   source_type, native_view_for_gestures, corners);
+ }
+ 
+ bool MenuRunner::IsRunning() const {
+diff --git a/ui/views/controls/menu/menu_runner_impl.cc b/ui/views/controls/menu/menu_runner_impl.cc
+index c2513d2889a2baabb1464a53b6b0a32176d52d0c..54f4f863d9ecdf22e3087944f773a866e6c50024 100644
+--- a/ui/views/controls/menu/menu_runner_impl.cc
++++ b/ui/views/controls/menu/menu_runner_impl.cc
+@@ -116,6 +116,7 @@ void MenuRunnerImpl::RunMenuAt(Widget* parent,
+                                const gfx::Rect& bounds,
+                                MenuAnchorPosition anchor,
+                                int32_t run_types,
++                               ui::MenuSourceType source_type,
+                                gfx::NativeView native_view_for_gestures,
+                                absl::optional<gfx::RoundedCornersF> corners) {
+   closing_event_time_ = base::TimeTicks();
+@@ -184,7 +185,7 @@ void MenuRunnerImpl::RunMenuAt(Widget* parent,
+   controller->Run(parent, button_controller, menu_, bounds, anchor,
+                   (run_types & MenuRunner::CONTEXT_MENU) != 0,
+                   (run_types & MenuRunner::NESTED_DRAG) != 0,
+-                  native_view_for_gestures);
++                  source_type, native_view_for_gestures);
+ }
+ 
+ void MenuRunnerImpl::Cancel() {
+diff --git a/ui/views/controls/menu/menu_runner_impl.h b/ui/views/controls/menu/menu_runner_impl.h
+index 4d2909b5094ab2a4af63504ac0b9f905b5b17759..c49038f592ab1f219ba8b902b69ec7b320e1f74d 100644
+--- a/ui/views/controls/menu/menu_runner_impl.h
++++ b/ui/views/controls/menu/menu_runner_impl.h
+@@ -52,6 +52,7 @@ class VIEWS_EXPORT MenuRunnerImpl : public MenuRunnerImplInterface,
+       const gfx::Rect& bounds,
+       MenuAnchorPosition anchor,
+       int32_t run_types,
++      ui::MenuSourceType source_type,
+       gfx::NativeView native_view_for_gestures,
+       absl::optional<gfx::RoundedCornersF> corners = absl::nullopt) override;
+   void Cancel() override;
+diff --git a/ui/views/controls/menu/menu_runner_impl_adapter.cc b/ui/views/controls/menu/menu_runner_impl_adapter.cc
+index b6c680063889bb997814cd1be45dfbe5f0989f74..dec5f4b2d609aa7b0cec0f16cc89e222bf9d7b85 100644
+--- a/ui/views/controls/menu/menu_runner_impl_adapter.cc
++++ b/ui/views/controls/menu/menu_runner_impl_adapter.cc
+@@ -33,10 +33,11 @@ void MenuRunnerImplAdapter::RunMenuAt(
+     const gfx::Rect& bounds,
+     MenuAnchorPosition anchor,
+     int32_t types,
++    ui::MenuSourceType source_type,
+     gfx::NativeView native_view_for_gestures,
+     absl::optional<gfx::RoundedCornersF> corners) {
+   impl_->RunMenuAt(parent, button_controller, bounds, anchor, types,
+-                   native_view_for_gestures);
++                   source_type, native_view_for_gestures);
+ }
+ 
+ void MenuRunnerImplAdapter::Cancel() {
+diff --git a/ui/views/controls/menu/menu_runner_impl_adapter.h b/ui/views/controls/menu/menu_runner_impl_adapter.h
+index e6587d2208a13576af1831b94724a6286f0e0607..91223ef3f099e20aee5cf1d685c45d2c7f53628e 100644
+--- a/ui/views/controls/menu/menu_runner_impl_adapter.h
++++ b/ui/views/controls/menu/menu_runner_impl_adapter.h
+@@ -43,6 +43,7 @@ class VIEWS_EXPORT MenuRunnerImplAdapter : public MenuRunnerImplInterface {
+       const gfx::Rect& bounds,
+       MenuAnchorPosition anchor,
+       int32_t types,
++      ui::MenuSourceType source_type,
+       gfx::NativeView native_view_for_gestures,
+       absl::optional<gfx::RoundedCornersF> corners = absl::nullopt) override;
+   void Cancel() override;
+diff --git a/ui/views/controls/menu/menu_runner_impl_cocoa.h b/ui/views/controls/menu/menu_runner_impl_cocoa.h
+index 50291eb07440a35173410336927490f44aae604b..febdb941ff72f7e8e9cc9b36e04c35b0f4c0227d 100644
+--- a/ui/views/controls/menu/menu_runner_impl_cocoa.h
++++ b/ui/views/controls/menu/menu_runner_impl_cocoa.h
+@@ -42,6 +42,7 @@ class VIEWS_EXPORT MenuRunnerImplCocoa : public MenuRunnerImplInterface {
+       const gfx::Rect& bounds,
+       MenuAnchorPosition anchor,
+       int32_t run_types,
++      ui::MenuSourceType source_type,
+       gfx::NativeView native_view_for_gestures,
+       absl::optional<gfx::RoundedCornersF> corners = absl::nullopt) override;
+   void Cancel() override;
+diff --git a/ui/views/controls/menu/menu_runner_impl_cocoa.mm b/ui/views/controls/menu/menu_runner_impl_cocoa.mm
+index 46a50f5a808c3bda24f44c1ce3a9944327f22fa4..0bfdf45ed43b0701d927940ca78be765b9184b4f 100644
+--- a/ui/views/controls/menu/menu_runner_impl_cocoa.mm
++++ b/ui/views/controls/menu/menu_runner_impl_cocoa.mm
+@@ -192,6 +192,7 @@
+     const gfx::Rect& bounds,
+     MenuAnchorPosition anchor,
+     int32_t run_types,
++    ui::MenuSourceType source_type,
+     gfx::NativeView native_view_for_gestures,
+     absl::optional<gfx::RoundedCornersF> corners) {
+   DCHECK(!IsRunning());
+diff --git a/ui/views/controls/menu/menu_runner_impl_interface.h b/ui/views/controls/menu/menu_runner_impl_interface.h
+index cf696fbcf071455007422b95608485c358d8a2b8..b18a6f9b1b4858af0fcb65d1a2e36ba2596e9726 100644
+--- a/ui/views/controls/menu/menu_runner_impl_interface.h
++++ b/ui/views/controls/menu/menu_runner_impl_interface.h
+@@ -45,6 +45,7 @@ class MenuRunnerImplInterface {
+       const gfx::Rect& bounds,
+       MenuAnchorPosition anchor,
+       int32_t run_types,
++      ui::MenuSourceType source_type,
+       gfx::NativeView native_view_for_gestures,
+       absl::optional<gfx::RoundedCornersF> corners = absl::nullopt) = 0;
+ 

shell/browser/api/electron_api_menu.cc

@@ -11,6 +11,7 @@
 #include "shell/browser/native_window.h"
 #include "shell/common/gin_converters/accelerator_converter.h"
 #include "shell/common/gin_converters/callback_converter.h"
+#include "shell/common/gin_converters/content_converter.h"
 #include "shell/common/gin_converters/file_path_converter.h"
 #include "shell/common/gin_converters/gurl_converter.h"
 #include "shell/common/gin_converters/image_converter.h"

shell/browser/api/electron_api_menu.h

@@ -78,6 +78,7 @@ class Menu : public gin::Wrappable<Menu>,
                        int x,
                        int y,
                        int positioning_item,
+                       ui::MenuSourceType source_type,
                        base::OnceClosure callback) = 0;
   virtual void ClosePopupAt(int32_t window_id) = 0;
   virtual std::u16string GetAcceleratorTextAtForTesting(int index) const;

shell/browser/api/electron_api_menu_mac.h

@@ -24,6 +24,7 @@ class MenuMac : public Menu {
                int x,
                int y,
                int positioning_item,
+               ui::MenuSourceType source_type,
                base::OnceClosure callback) override;
   void PopupOnUI(const base::WeakPtr<NativeWindow>& native_window,
                  int32_t window_id,

shell/browser/api/electron_api_menu_mac.mm

@@ -52,6 +52,7 @@ void MenuMac::PopupAt(BaseWindow* window,
                       int x,
                       int y,
                       int positioning_item,
+                      ui::MenuSourceType source_type,
                       base::OnceClosure callback) {
   NativeWindow* native_window = window->window();
   if (!native_window)

shell/browser/api/electron_api_menu_views.cc

@@ -22,6 +22,7 @@ void MenuViews::PopupAt(BaseWindow* window,
                         int x,
                         int y,
                         int positioning_item,
+                        ui::MenuSourceType source_type,
                         base::OnceClosure callback) {
   auto* native_window = static_cast<NativeWindowViews*>(window->window());
   if (!native_window)
@@ -55,7 +56,7 @@ void MenuViews::PopupAt(BaseWindow* window,
       std::make_unique<MenuRunner>(model(), flags, std::move(close_callback));
   menu_runners_[window_id]->RunMenuAt(
       native_window->widget(), nullptr, gfx::Rect(location, gfx::Size()),
-      views::MenuAnchorPosition::kTopLeft, ui::MENU_SOURCE_MOUSE);
+      views::MenuAnchorPosition::kTopLeft, source_type);
 }
 
 void MenuViews::ClosePopupAt(int32_t window_id) {

shell/browser/api/electron_api_menu_views.h

@@ -25,6 +25,7 @@ class MenuViews : public Menu {
                int x,
                int y,
                int positioning_item,
+               ui::MenuSourceType source_type,
                base::OnceClosure callback) override;
   void ClosePopupAt(int32_t window_id) override;
 

shell/common/gin_converters/content_converter.cc

@@ -24,36 +24,81 @@
 
 namespace gin {
 
-template <>
-struct Converter<ui::MenuSourceType> {
-  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
-                                   const ui::MenuSourceType& in) {
-    switch (in) {
-      case ui::MENU_SOURCE_MOUSE:
-        return StringToV8(isolate, "mouse");
-      case ui::MENU_SOURCE_KEYBOARD:
-        return StringToV8(isolate, "keyboard");
-      case ui::MENU_SOURCE_TOUCH:
-        return StringToV8(isolate, "touch");
-      case ui::MENU_SOURCE_TOUCH_EDIT_MENU:
-        return StringToV8(isolate, "touchMenu");
-      case ui::MENU_SOURCE_LONG_PRESS:
-        return StringToV8(isolate, "longPress");
-      case ui::MENU_SOURCE_LONG_TAP:
-        return StringToV8(isolate, "longTap");
-      case ui::MENU_SOURCE_TOUCH_HANDLE:
-        return StringToV8(isolate, "touchHandle");
-      case ui::MENU_SOURCE_STYLUS:
-        return StringToV8(isolate, "stylus");
-      case ui::MENU_SOURCE_ADJUST_SELECTION:
-        return StringToV8(isolate, "adjustSelection");
-      case ui::MENU_SOURCE_ADJUST_SELECTION_RESET:
-        return StringToV8(isolate, "adjustSelectionReset");
-      default:
-        return StringToV8(isolate, "none");
-    }
+// static
+v8::Local<v8::Value> Converter<ui::MenuSourceType>::ToV8(
+    v8::Isolate* isolate,
+    const ui::MenuSourceType& in) {
+  switch (in) {
+    case ui::MENU_SOURCE_MOUSE:
+      return StringToV8(isolate, "mouse");
+    case ui::MENU_SOURCE_KEYBOARD:
+      return StringToV8(isolate, "keyboard");
+    case ui::MENU_SOURCE_TOUCH:
+      return StringToV8(isolate, "touch");
+    case ui::MENU_SOURCE_TOUCH_EDIT_MENU:
+      return StringToV8(isolate, "touchMenu");
+    case ui::MENU_SOURCE_LONG_PRESS:
+      return StringToV8(isolate, "longPress");
+    case ui::MENU_SOURCE_LONG_TAP:
+      return StringToV8(isolate, "longTap");
+    case ui::MENU_SOURCE_TOUCH_HANDLE:
+      return StringToV8(isolate, "touchHandle");
+    case ui::MENU_SOURCE_STYLUS:
+      return StringToV8(isolate, "stylus");
+    case ui::MENU_SOURCE_ADJUST_SELECTION:
+      return StringToV8(isolate, "adjustSelection");
+    case ui::MENU_SOURCE_ADJUST_SELECTION_RESET:
+      return StringToV8(isolate, "adjustSelectionReset");
+    case ui::MENU_SOURCE_NONE:
+      return StringToV8(isolate, "none");
   }
-};
+}
+
+// static
+bool Converter<ui::MenuSourceType>::FromV8(v8::Isolate* isolate,
+                                           v8::Local<v8::Value> val,
+                                           ui::MenuSourceType* out) {
+  std::string type;
+  if (!ConvertFromV8(isolate, val, &type))
+    return false;
+
+  if (type == "mouse") {
+    *out = ui::MENU_SOURCE_MOUSE;
+    return true;
+  } else if (type == "keyboard") {
+    *out = ui::MENU_SOURCE_KEYBOARD;
+    return true;
+  } else if (type == "touch") {
+    *out = ui::MENU_SOURCE_TOUCH;
+    return true;
+  } else if (type == "touchMenu") {
+    *out = ui::MENU_SOURCE_TOUCH_EDIT_MENU;
+    return true;
+  } else if (type == "longPress") {
+    *out = ui::MENU_SOURCE_LONG_PRESS;
+    return true;
+  } else if (type == "longTap") {
+    *out = ui::MENU_SOURCE_LONG_TAP;
+    return true;
+  } else if (type == "touchHandle") {
+    *out = ui::MENU_SOURCE_TOUCH_HANDLE;
+    return true;
+  } else if (type == "stylus") {
+    *out = ui::MENU_SOURCE_STYLUS;
+    return true;
+  } else if (type == "adjustSelection") {
+    *out = ui::MENU_SOURCE_ADJUST_SELECTION;
+    return true;
+  } else if (type == "adjustSelectionReset") {
+    *out = ui::MENU_SOURCE_ADJUST_SELECTION_RESET;
+    return true;
+  } else if (type == "none") {
+    *out = ui::MENU_SOURCE_NONE;
+    return true;
+  }
+
+  return false;
+}
 
 // static
 v8::Local<v8::Value> Converter<blink::mojom::MenuItem::Type>::ToV8(

shell/common/gin_converters/content_converter.h

@@ -13,6 +13,7 @@
 #include "third_party/blink/public/common/permissions/permission_utils.h"
 #include "third_party/blink/public/mojom/choosers/popup_menu.mojom.h"
 #include "third_party/blink/public/mojom/permissions/permission_status.mojom.h"
+#include "ui/base/ui_base_types.h"
 
 namespace content {
 struct ContextMenuParams;
@@ -39,6 +40,15 @@ struct Converter<ContextMenuParamsWithRenderFrameHost> {
       const ContextMenuParamsWithRenderFrameHost& val);
 };
 
+template <>
+struct Converter<ui::MenuSourceType> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const ui::MenuSourceType& val);
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     ui::MenuSourceType* out);
+};
+
 template <>
 struct Converter<blink::mojom::PermissionStatus> {
   static bool FromV8(v8::Isolate* isolate,

typings/internal-electron.d.ts

@@ -115,7 +115,7 @@ declare namespace Electron {
     commandsMap: Record<string, MenuItem>;
     groupsMap: Record<string, MenuItem[]>;
     getItemCount(): number;
-    popupAt(window: BaseWindow, x: number, y: number, positioning: number, callback: () => void): void;
+    popupAt(window: BaseWindow, x: number, y: number, positioning: number, sourceType: Required<Electron.PopupOptions>['sourceType'], callback: () => void): void;
     closePopupAt(id: number): void;
     setSublabel(index: number, label: string): void;
     setToolTip(index: number, tooltip: string): void;