From cc7822e16a68c04a38cd9939234b9655d2cd24b9 Mon Sep 17 00:00:00 2001
From: Tom Schuster <tschuster@mozilla.com>
Date: Fri, 14 Jun 2024 21:08:04 +0000
Subject: [PATCH] Bug 1893068 - Remove popup inline event handlers from the
 menubar. r=Gijs,devtools-reviewers,nchevobbe

Differential Revision: https://phabricator.services.mozilla.com/D212247
---
 browser/base/content/browser-menubar.inc      | 49 +++-------
 browser/base/content/browser-menubar.js       | 93 +++++++++++++++++++
 .../test/menubar/browser_file_close_tabs.js   |  8 +-
 .../test/menubar/browser_file_share.js        |  8 +-
 4 files changed, 114 insertions(+), 44 deletions(-)

diff --git a/browser/base/content/browser-menubar.inc b/browser/base/content/browser-menubar.inc
index f7a006892c51f..7b8a78623e0dd 100644
--- a/browser/base/content/browser-menubar.inc
+++ b/browser/base/content/browser-menubar.inc
@@ -3,26 +3,16 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-       <menubar id="main-menubar"
-# On macOS, we don't track whether activation of the native menubar happened
-# with the keyboard.
-#ifndef XP_MACOSX
-                onpopupshowing="if (event.target.parentNode.parentNode == this)
-                                  this.setAttribute('openedwithkey',
-                                                    event.target.parentNode.openedWithKey);"
-#endif
-                >
-            <script src="chrome://browser/content/browser-menubar.js" />
-
+       <menubar id="main-menubar">
+          <script src="chrome://browser/content/browser-menubar.js" />
             <menu id="file-menu" data-l10n-id="menu-file">
-              <menupopup id="menu_FilePopup"
-                         onpopupshowing="gFileMenu.onPopupShowing(event);">
+              <menupopup id="menu_FilePopup">
                 <menuitem id="menu_newNavigatorTab"
                           command="cmd_newNavigatorTab"
                           key="key_newNavigatorTab" data-l10n-id="menu-file-new-tab"/>
                 <menu id="menu_newUserContext"
                       hidden="true" data-l10n-id="menu-file-new-container-tab">
-                  <menupopup onpopupshowing="return createUserContextMenu(event);" />
+                  <menupopup id="menu_newUserContextPopup" />
                 </menu>
                 <menuitem id="menu_newNavigator"
                           key="key_newNavigator"
@@ -81,9 +71,7 @@
             </menu>
 
             <menu id="edit-menu" data-l10n-id="menu-edit">
-              <menupopup id="menu_EditPopup"
-                         onpopupshowing="updateEditUIVisibility()"
-                         onpopuphidden="updateEditUIVisibility()">
+              <menupopup id="menu_EditPopup">
                 <menuitem id="menu_undo"
                           key="key_undo"
                           command="cmd_undo" data-l10n-id="text-action-undo"/>
@@ -134,7 +122,7 @@
             <menu id="view-menu" data-l10n-id="menu-view">
               <menupopup id="menu_viewPopup">
                 <menu id="viewToolbarsMenu" data-l10n-id="menu-view-toolbars-menu">
-                  <menupopup id="view-menu-popup" onpopupshowing="onViewToolbarsPopupShowing(event);">
+                  <menupopup id="view-menu-popup">
                     <menuseparator/>
                     <menuitem id="menu_customizeToolbars"
                               command="cmd_CustomizeToolbars" data-l10n-id="menu-view-customize-toolbar2"/>
@@ -164,7 +152,7 @@
                   </menupopup>
                 </menu>
                 <menu id="pageStyleMenu" data-l10n-id="menu-view-page-style-menu">
-                  <menupopup onpopupshowing="gPageStyleMenu.fillPopup(this);">
+                  <menupopup id="pageStyleMenuPopup">
                     <menuitem id="menu_pageStyleNoStyle"
                               type="radio" data-l10n-id="menu-view-page-style-no-style"/>
                     <menuitem id="menu_pageStylePersistentOnly"
@@ -215,8 +203,6 @@
                          placespopup="true"
                          is="places-popup"
 #endif
-                         onpopupshowing="if (!this.parentNode._placesView)
-                                           new HistoryMenu(event);"
                          tooltip="bhTooltip"
                          popupsinherittooltip="true">
                 <menuitem id="menu_showAllHistory"
@@ -242,7 +228,7 @@
                              placespopup="true"
                              is="places-popup"
 #endif
-                             onpopupshowing="document.getElementById('history-menu')._placesView.populateUndoSubmenu();"/>
+                  />
                 </menu>
                 <menu id="historyUndoWindowMenu"
                       disabled="true" data-l10n-id="menu-history-undo-window-menu">
@@ -251,7 +237,7 @@
                              placespopup="true"
                              is="places-popup"
 #endif
-                             onpopupshowing="document.getElementById('history-menu')._placesView.populateUndoWindowSubmenu();">
+                  >
 #ifdef HIDDEN_WINDOW
 # This entry is never visible. It's here to make the cmd-shift-n
 # shortcut work in the hidden window when the last window is closed.
@@ -284,9 +270,6 @@
                onmouseup="BookmarksEventHandler.onMouseUp(event);"
                oncommand="BookmarksEventHandler.onCommand(event);"
                onclick="BookmarksEventHandler.onClick(event, this.parentNode._placesView);"
-               onpopupshowing="BookmarkingUI.onMainMenuPopupShowing(event);
-                               if (!this.parentNode._placesView)
-                                 new PlacesMenu(event, `place:parent=${PlacesUtils.bookmarks.menuGuid}`);"
                tooltip="bhTooltip" popupsinherittooltip="true">
       <menuitem id="bookmarksShowAll"
                 command="Browser:ShowAllBookmarks"
@@ -315,9 +298,7 @@
                    placespopup="true"
                    is="places-popup"
 #endif
-                   context="placesContext"
-                   onpopupshowing="if (!this.parentNode._placesView)
-                                     new PlacesMenu(event, `place:parent=${PlacesUtils.bookmarks.toolbarGuid}`);"/>
+                   context="placesContext"/>
       </menu>
       <menu id="menu_unsortedBookmarks"
             class="menu-iconic bookmark-item"
@@ -328,9 +309,7 @@
                    placespopup="true"
                    is="places-popup"
 #endif
-                   context="placesContext"
-                   onpopupshowing="if (!this.parentNode._placesView)
-                                     new PlacesMenu(event, `place:parent=${PlacesUtils.bookmarks.unfiledGuid}`);"/>
+                   context="placesContext"/>
       </menu>
       <menu id="menu_mobileBookmarks"
             class="menu-iconic bookmark-item"
@@ -342,9 +321,7 @@
                    placespopup="true"
                    is="places-popup"
 #endif
-                   context="placesContext"
-                   onpopupshowing="if (!this.parentNode._placesView)
-                                     new PlacesMenu(event, `place:parent=${PlacesUtils.bookmarks.mobileGuid}`);"/>
+                   context="placesContext"/>
       </menu>
       <menuseparator id="bookmarksMenuItemsSeparator"/>
       <!-- Bookmarks menu items -->
@@ -439,7 +416,7 @@
             </menu>
 #endif
             <menu id="helpMenu" data-l10n-id="menu-help">
-              <menupopup id="menu_HelpPopup" onpopupshowing="buildHelpMenu();">
+              <menupopup id="menu_HelpPopup">
 <!-- Note: Items under here are cloned to the AppMenu Help submenu. The cloned items
      have their strings defined by appmenu-data-l10n-id. -->
                 <menuitem id="menu_openHelp"
diff --git a/browser/base/content/browser-menubar.js b/browser/base/content/browser-menubar.js
index 00e34eafa2f40..d537a2f035780 100644
--- a/browser/base/content/browser-menubar.js
+++ b/browser/base/content/browser-menubar.js
@@ -114,6 +114,99 @@ document.addEventListener(
         let historyMenu = document.getElementById("history-menu");
         historyMenu._placesView._onCommand(event);
       });
+
+    mainMenuBar.addEventListener("popupshowing", event => {
+      // On macOS, we don't track whether activation of the native menubar happened
+      // with the keyboard.
+      if (AppConstants.platform != "macosx") {
+        // We only set the "openedwithkey" if a specific menu like "Edit" was opened
+        // instead of the general menu bar. (e.g. Alt+E instead of just Alt)
+        if (event.target.parentNode.parentNode == this) {
+          this.setAttribute(
+            "openedwithkey",
+            event.target.parentNode.openedWithKey
+          );
+        }
+      }
+
+      switch (event.target.id) {
+        case "menu_FilePopup":
+          gFileMenu.onPopupShowing(event);
+          break;
+        case "menu_newUserContextPopup":
+          createUserContextMenu(event);
+          break;
+        case "menu_EditPopup":
+          updateEditUIVisibility();
+          break;
+        case "view-menu-popup":
+          onViewToolbarsPopupShowing(event);
+          break;
+        case "viewSidebarMenu":
+          SidebarController.setMegalistMenubarVisibility(event);
+          break;
+        case "pageStyleMenuPopup":
+          gPageStyleMenu.fillPopup(event.target);
+          break;
+        case "historyMenuPopup":
+          if (!event.target.parentNode._placesView) {
+            new HistoryMenu(event);
+          }
+          break;
+        case "historyUndoPopup":
+          document
+            .getElementById("history-menu")
+            ._placesView.populateUndoSubmenu();
+          break;
+        case "historyUndoWindowPopup":
+          document
+            .getElementById("history-menu")
+            ._placesView.populateUndoWindowSubmenu();
+          break;
+        case "bookmarksMenuPopup":
+          BookmarkingUI.onMainMenuPopupShowing(event);
+          if (!event.target.parentNode._placesView) {
+            new PlacesMenu(
+              event,
+              `place:parent=${PlacesUtils.bookmarks.menuGuid}`
+            );
+          }
+          break;
+        case "bookmarksToolbarFolderPopup":
+          if (!event.target.parentNode._placesView) {
+            new PlacesMenu(
+              event,
+              `place:parent=${PlacesUtils.bookmarks.toolbarGuid}`
+            );
+          }
+          break;
+        case "otherBookmarksFolderPopup":
+          if (!event.target.parentNode._placesView) {
+            new PlacesMenu(
+              event,
+              `place:parent=${PlacesUtils.bookmarks.unfiledGuid}`
+            );
+          }
+          break;
+        case "mobileBookmarksFolderPopup":
+          if (!event.target.parentNode._placesView) {
+            new PlacesMenu(
+              event,
+              `place:parent=${PlacesUtils.bookmarks.mobileGuid}`
+            );
+          }
+          break;
+        case "menu_HelpPopup":
+          buildHelpMenu();
+          break;
+      }
+    });
+
+    document
+      .getElementById("menu_EditPopup")
+      .addEventListener("popuphidden", () => {
+        updateEditUIVisibility();
+      });
   },
   { once: true }
 );
diff --git a/browser/base/content/test/menubar/browser_file_close_tabs.js b/browser/base/content/test/menubar/browser_file_close_tabs.js
index 15abd92bba748..fc5fb6ef55611 100644
--- a/browser/base/content/test/menubar/browser_file_close_tabs.js
+++ b/browser/base/content/test/menubar/browser_file_close_tabs.js
@@ -46,15 +46,15 @@ add_task(async function test_menu_close_tab_count() {
 async function simulateMenuOpen(menu) {
   return new Promise(resolve => {
     menu.addEventListener("popupshown", resolve, { once: true });
-    menu.dispatchEvent(new MouseEvent("popupshowing"));
-    menu.dispatchEvent(new MouseEvent("popupshown"));
+    menu.dispatchEvent(new MouseEvent("popupshowing", { bubbles: true }));
+    menu.dispatchEvent(new MouseEvent("popupshown", { bubbles: true }));
   });
 }
 
 async function simulateMenuClosed(menu) {
   return new Promise(resolve => {
     menu.addEventListener("popuphidden", resolve, { once: true });
-    menu.dispatchEvent(new MouseEvent("popuphiding"));
-    menu.dispatchEvent(new MouseEvent("popuphidden"));
+    menu.dispatchEvent(new MouseEvent("popuphiding", { bubbles: true }));
+    menu.dispatchEvent(new MouseEvent("popuphidden", { bubbles: true }));
   });
 }
diff --git a/browser/base/content/test/menubar/browser_file_share.js b/browser/base/content/test/menubar/browser_file_share.js
index bd6a4c3f60bff..f2ba9eaa8c2e7 100644
--- a/browser/base/content/test/menubar/browser_file_share.js
+++ b/browser/base/content/test/menubar/browser_file_share.js
@@ -122,15 +122,15 @@ add_task(async function test_file_menu_share() {
 async function simulateMenuOpen(menu) {
   return new Promise(resolve => {
     menu.addEventListener("popupshown", resolve, { once: true });
-    menu.dispatchEvent(new MouseEvent("popupshowing"));
-    menu.dispatchEvent(new MouseEvent("popupshown"));
+    menu.dispatchEvent(new MouseEvent("popupshowing", { bubbles: true }));
+    menu.dispatchEvent(new MouseEvent("popupshown", { bubbles: true }));
   });
 }
 
 async function simulateMenuClosed(menu) {
   return new Promise(resolve => {
     menu.addEventListener("popuphidden", resolve, { once: true });
-    menu.dispatchEvent(new MouseEvent("popuphiding"));
-    menu.dispatchEvent(new MouseEvent("popuphidden"));
+    menu.dispatchEvent(new MouseEvent("popuphiding", { bubbles: true }));
+    menu.dispatchEvent(new MouseEvent("popuphidden", { bubbles: true }));
   });
 }