feat: add registerAccelerator flag to allow menu items to optionally skip accelerator registration (backport: 4-0-x) (#15840)

* feat: add registerAccelerator flag to allow menu items to skip registration

* docs: add docs for registerAccelerator

* docs: re-add accidentally removed line

atom/browser/api/atom_api_menu.cc

@@ -41,6 +41,8 @@ void Menu::AfterInit(v8::Isolate* isolate) {
   delegate.Get("isCommandIdEnabled", &is_enabled_);
   delegate.Get("isCommandIdVisible", &is_visible_);
   delegate.Get("getAcceleratorForCommandId", &get_accelerator_);
+  delegate.Get("shouldRegisterAcceleratorForCommandId",
+               &should_register_accelerator_);
   delegate.Get("executeCommand", &execute_command_);
   delegate.Get("menuWillShow", &menu_will_show_);
 }
@@ -74,6 +76,12 @@ bool Menu::GetAcceleratorForCommandIdWithParams(
   return mate::ConvertFromV8(isolate(), val, accelerator);
 }
 
+bool Menu::ShouldRegisterAcceleratorForCommandId(int command_id) const {
+  v8::Locker locker(isolate());
+  v8::HandleScope handle_scope(isolate());
+  return should_register_accelerator_.Run(GetWrapper(), command_id);
+}
+
 void Menu::ExecuteCommand(int command_id, int flags) {
   v8::Locker locker(isolate());
   v8::HandleScope handle_scope(isolate());

atom/browser/api/atom_api_menu.h

@@ -51,6 +51,7 @@ class Menu : public mate::TrackableObject<Menu>,
       int command_id,
       bool use_default_accelerator,
       ui::Accelerator* accelerator) const override;
+  bool ShouldRegisterAcceleratorForCommandId(int command_id) const override;
   void ExecuteCommand(int command_id, int event_flags) override;
   void MenuWillShow(ui::SimpleMenuModel* source) override;
 
@@ -102,6 +103,7 @@ class Menu : public mate::TrackableObject<Menu>,
   base::Callback<bool(v8::Local<v8::Value>, int)> is_visible_;
   base::Callback<v8::Local<v8::Value>(v8::Local<v8::Value>, int, bool)>
       get_accelerator_;
+  base::Callback<bool(v8::Local<v8::Value>, int)> should_register_accelerator_;
   base::Callback<void(v8::Local<v8::Value>, v8::Local<v8::Value>, int)>
       execute_command_;
   base::Callback<void(v8::Local<v8::Value>)> menu_will_show_;

atom/browser/ui/accelerator_util.cc

@@ -77,9 +77,11 @@ void GenerateAcceleratorTable(AcceleratorTable* table,
       GenerateAcceleratorTable(table, submodel);
     } else {
       ui::Accelerator accelerator;
-      if (model->GetAcceleratorAtWithParams(i, true, &accelerator)) {
-        MenuItem item = {i, model};
-        (*table)[accelerator] = item;
+      if (model->ShouldRegisterAcceleratorAt(i)) {
+        if (model->GetAcceleratorAtWithParams(i, true, &accelerator)) {
+          MenuItem item = {i, model};
+          (*table)[accelerator] = item;
+        }
       }
     }
   }

atom/browser/ui/atom_menu_model.cc

@@ -43,6 +43,14 @@ bool AtomMenuModel::GetAcceleratorAtWithParams(
   return false;
 }
 
+bool AtomMenuModel::ShouldRegisterAcceleratorAt(int index) const {
+  if (delegate_) {
+    return delegate_->ShouldRegisterAcceleratorForCommandId(
+        GetCommandIdAt(index));
+  }
+  return true;
+}
+
 void AtomMenuModel::MenuWillClose() {
   ui::SimpleMenuModel::MenuWillClose();
   for (Observer& observer : observers_) {

atom/browser/ui/atom_menu_model.h

@@ -23,6 +23,9 @@ class AtomMenuModel : public ui::SimpleMenuModel {
         bool use_default_accelerator,
         ui::Accelerator* accelerator) const = 0;
 
+    virtual bool ShouldRegisterAcceleratorForCommandId(
+        int command_id) const = 0;
+
    private:
     // ui::SimpleMenuModel::Delegate:
     bool GetAcceleratorForCommandId(
@@ -52,6 +55,7 @@ class AtomMenuModel : public ui::SimpleMenuModel {
   bool GetAcceleratorAtWithParams(int index,
                                   bool use_default_accelerator,
                                   ui::Accelerator* accelerator) const;
+  bool ShouldRegisterAcceleratorAt(int index) const;
 
   // ui::SimpleMenuModel:
   void MenuWillClose() override;

docs/api/menu-item.md

@@ -27,6 +27,8 @@ See [`Menu`](menu.md) for examples.
   * `visible` Boolean (optional) - If false, the menu item will be entirely hidden.
   * `checked` Boolean (optional) - Should only be specified for `checkbox` or `radio` type
     menu items.
+  * `registerAccelerator` Boolean (optional) - If false, the accelerator won't be registered
+    with the system, but it will still be displayed. Defaults to true.
   * `submenu` (MenuItemConstructorOptions[] | [Menu](menu.md)) (optional) - Should be specified
     for `submenu` type menu items. If `submenu` is specified, the `type: 'submenu'` can be omitted.
     If the value is not a [`Menu`](menu.md) then it will be automatically converted to one using

lib/browser/api/menu-item-roles.js

@@ -16,12 +16,14 @@ const roles = {
   copy: {
     label: 'Copy',
     accelerator: 'CommandOrControl+C',
-    webContentsMethod: 'copy'
+    webContentsMethod: 'copy',
+    registerAccelerator: false
   },
   cut: {
     label: 'Cut',
     accelerator: 'CommandOrControl+X',
-    webContentsMethod: 'cut'
+    webContentsMethod: 'cut',
+    registerAccelerator: false
   },
   delete: {
     label: 'Delete',
@@ -59,12 +61,14 @@ const roles = {
   paste: {
     label: 'Paste',
     accelerator: 'CommandOrControl+V',
-    webContentsMethod: 'paste'
+    webContentsMethod: 'paste',
+    registerAccelerator: false
   },
   pasteandmatchstyle: {
     label: 'Paste and Match Style',
     accelerator: 'Shift+CommandOrControl+V',
-    webContentsMethod: 'pasteAndMatchStyle'
+    webContentsMethod: 'pasteAndMatchStyle',
+    registerAccelerator: false
   },
   quit: {
     get label () {
@@ -243,6 +247,10 @@ exports.getDefaultAccelerator = (role) => {
   if (roles.hasOwnProperty(role)) return roles[role].accelerator
 }
 
+exports.shouldRegisterAccelerator = (role) => {
+  return roles.hasOwnProperty(role) ? roles[role].registerAccelerator : true
+}
+
 exports.getDefaultSubmenu = (role) => {
   if (!roles.hasOwnProperty(role)) return
 

lib/browser/api/menu-item.js

@@ -36,6 +36,7 @@ const MenuItem = function (options) {
   this.overrideProperty('enabled', true)
   this.overrideProperty('visible', true)
   this.overrideProperty('checked', false)
+  this.overrideProperty('registerAccelerator', roles.shouldRegisterAccelerator(this.role))
 
   if (!MenuItem.types.includes(this.type)) {
     throw new Error(`Unknown menu item type: ${this.type}`)

lib/browser/api/menu.js

@@ -24,6 +24,7 @@ const delegate = {
     if (command.accelerator != null) return command.accelerator
     if (useDefaultAccelerator) return command.getDefaultRoleAccelerator()
   },
+  shouldRegisterAcceleratorForCommandId: (menu, id) => menu.commandsMap[id] ? menu.commandsMap[id].registerAccelerator : undefined,
   executeCommand: (menu, event, id) => {
     const command = menu.commandsMap[id]
     if (!command) return