Skip to content

Commit

Permalink
Bug 1893068 - Remove popup inline event handlers from the menubar. r=…
Browse files Browse the repository at this point in the history 
…Gijs,devtools-reviewers,nchevobbe

Differential Revision: https://phabricator.services.mozilla.com/D212247
  • Loading branch information
Tom Schuster committed Jun 14, 2024
1 parent 2aeaaec commit cc7822e
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 44 deletions.
49 changes: 13 additions & 36 deletions browser/base/content/browser-menubar.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"/>
Expand Down Expand Up @@ -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"/>
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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">
Expand All @@ -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.
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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 -->
Expand Down Expand Up @@ -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"
Expand Down
93 changes: 93 additions & 0 deletions browser/base/content/browser-menubar.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
);
8 changes: 4 additions & 4 deletions browser/base/content/test/menubar/browser_file_close_tabs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));
});
}
8 changes: 4 additions & 4 deletions browser/base/content/test/menubar/browser_file_share.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));
});
}

0 comments on commit cc7822e

Please sign in to comment.