From ac238219982d84546ed2c304d602a67acab33444 Mon Sep 17 00:00:00 2001 From: Stephen A Pohl Date: Sat, 2 Nov 2024 02:13:05 +0000 Subject: [PATCH] Bug 1855346: Ensure that menu items added by macOS also don't execute commands when we expect all commands to be suppressed. Inspired by research by :chesterbr and :bintoro, and a patch by :chesterbr. r=mstange a=RyanVM --- toolkit/xre/MacApplicationDelegate.mm | 4 +- widget/cocoa/nsCocoaUtils.mm | 2 +- widget/cocoa/nsMenuBarX.h | 36 ++--- widget/cocoa/nsMenuBarX.mm | 191 +++++++++++++------------- widget/cocoa/nsMenuItemX.mm | 6 +- widget/cocoa/nsMenuUtilsX.mm | 51 +++---- widget/cocoa/nsMenuX.mm | 12 +- 7 files changed, 150 insertions(+), 152 deletions(-) diff --git a/toolkit/xre/MacApplicationDelegate.mm b/toolkit/xre/MacApplicationDelegate.mm index ca99497624e9..b7db9890d967 100644 --- a/toolkit/xre/MacApplicationDelegate.mm +++ b/toolkit/xre/MacApplicationDelegate.mm @@ -165,7 +165,7 @@ nsTArray TakeStartupURLs() { return std::move(StartupURLs()); } if (![NSApp windowsMenu]) { // If the application has a windows menu, it will keep it up to date and // prepend the window list to the Dock menu automatically. - NSMenu* windowsMenu = [[NSMenu alloc] initWithTitle:@"Window"]; + NSMenu* windowsMenu = [[GeckoNSMenu alloc] initWithTitle:@"Window"]; [NSApp setWindowsMenu:windowsMenu]; [windowsMenu release]; } @@ -209,7 +209,7 @@ nsTArray TakeStartupURLs() { return std::move(StartupURLs()); } NS_OBJC_BEGIN_TRY_BLOCK_RETURN; // Create the NSMenu that will contain the dock menu items. - NSMenu* menu = [[[NSMenu alloc] initWithTitle:@""] autorelease]; + NSMenu* menu = [[[GeckoNSMenu alloc] initWithTitle:@""] autorelease]; [menu setAutoenablesItems:NO]; // Add application-specific dock menu items. On error, do not insert the diff --git a/widget/cocoa/nsCocoaUtils.mm b/widget/cocoa/nsCocoaUtils.mm index 5bb487f7f725..8dd8f56ddc0d 100644 --- a/widget/cocoa/nsCocoaUtils.mm +++ b/widget/cocoa/nsCocoaUtils.mm @@ -347,7 +347,7 @@ void nsCocoaUtils::PrepareForNativeAppModalDialog() { "Main menu does not have any items, something is terribly wrong!"); // Create new menu bar for use with modal dialog - NSMenu* newMenuBar = [[NSMenu alloc] initWithTitle:@""]; + NSMenu* newMenuBar = [[GeckoNSMenu alloc] initWithTitle:@""]; // Swap in our app menu. Note that the event target is whatever window is up // when the app modal dialog goes up. diff --git a/widget/cocoa/nsMenuBarX.h b/widget/cocoa/nsMenuBarX.h index bcf124a7acc5..172e9f965f5c 100644 --- a/widget/cocoa/nsMenuBarX.h +++ b/widget/cocoa/nsMenuBarX.h @@ -37,34 +37,22 @@ class Element; - (id)initWithApplicationMenu:(nsMenuBarX*)aApplicationMenu; @end -// Objective-C class used to allow us to intervene with keyboard event handling. -// We allow mouse actions to work normally. -@interface GeckoNSMenu : NSMenu { -} -- (BOOL)performSuperKeyEquivalent:(NSEvent*)aEvent; -@end - -// Objective-C class used as action target for menu items -@interface NativeMenuItemTarget : NSObject { -} -- (IBAction)menuItemHit:(id)aSender; -@end - -// Objective-C class used for menu items on the Services menu to allow Gecko -// to override their standard behavior in order to stop key equivalents from -// firing in certain instances. -@interface GeckoServicesNSMenuItem : NSMenuItem { +// Objective-C class used for menu items to allow Gecko to override their +// standard behavior in order to stop key equivalents from firing in certain +// instances. +@interface GeckoNSMenuItem : NSMenuItem { } - (id)target; - (SEL)action; - (void)_doNothing:(id)aSender; @end -// Objective-C class used as the Services menu so that Gecko can override the -// standard behavior of the Services menu in order to stop key equivalents -// from firing in certain instances. -@interface GeckoServicesNSMenu : NSMenu { +// Objective-C class used to allow us to intervene with keyboard event handling, +// particularly to stop key equivalents from firing in certain instances. We +// allow mouse actions to work normally. +@interface GeckoNSMenu : NSMenu { } +- (BOOL)performSuperKeyEquivalent:(NSEvent*)aEvent; - (void)addItem:(NSMenuItem*)aNewItem; - (NSMenuItem*)addItemWithTitle:(NSString*)aString action:(SEL)aSelector @@ -77,6 +65,12 @@ class Element; - (void)_overrideClassOfMenuItem:(NSMenuItem*)aMenuItem; @end +// Objective-C class used as action target for menu items. +@interface NativeMenuItemTarget : NSObject { +} +- (IBAction)menuItemHit:(id)aSender; +@end + // Once instantiated, this object lives until its DOM node or its parent window // is destroyed. Do not hold references to this, they can become invalid any // time the DOM node can be destroyed. diff --git a/widget/cocoa/nsMenuBarX.mm b/widget/cocoa/nsMenuBarX.mm index 9865fc196dca..bf83057ebf0a 100644 --- a/widget/cocoa/nsMenuBarX.mm +++ b/widget/cocoa/nsMenuBarX.mm @@ -5,12 +5,13 @@ #include +#include "nsChildView.h" +#include "nsCocoaUtils.h" +#include "nsCocoaWindow.h" #include "nsMenuBarX.h" -#include "nsMenuX.h" #include "nsMenuItemX.h" #include "nsMenuUtilsX.h" -#include "nsCocoaUtils.h" -#include "nsChildView.h" +#include "nsMenuX.h" #include "nsCOMPtr.h" #include "nsString.h" @@ -37,6 +38,11 @@ NSMenu* sApplicationMenu = nil; BOOL sApplicationMenuIsFallback = NO; BOOL gSomeMenuBarPainted = NO; +// Controls whether or not native menu items should invoke their commands. See +// class comments for `GeckoNSMenuItem` and `GeckoNSMenu` below for an +// explanation of why this switch is necessary. +static BOOL gMenuItemsExecuteCommands = YES; + // defined in nsCocoaWindow.mm. extern BOOL sTouchBarIsInitialized; @@ -191,9 +197,9 @@ void nsMenuBarX::ConstructFallbackNativeMenus() { } sApplicationMenu.delegate = mApplicationMenuDelegate; NSMenuItem* quitMenuItem = - [[[NSMenuItem alloc] initWithTitle:labelStr - action:@selector(menuItemHit:) - keyEquivalent:keyStr] autorelease]; + [[[GeckoNSMenuItem alloc] initWithTitle:labelStr + action:@selector(menuItemHit:) + keyEquivalent:keyStr] autorelease]; quitMenuItem.target = nsMenuBarX::sNativeEventTarget; quitMenuItem.tag = eCommand_ID_Quit; [sApplicationMenu addItem:quitMenuItem]; @@ -681,9 +687,9 @@ NSMenuItem* nsMenuBarX::CreateNativeAppMenuItem(nsMenuX* aMenu, } // put together the actual NSMenuItem - NSMenuItem* newMenuItem = [[NSMenuItem alloc] initWithTitle:labelString - action:aAction - keyEquivalent:keyEquiv]; + NSMenuItem* newMenuItem = [[GeckoNSMenuItem alloc] initWithTitle:labelString + action:aAction + keyEquivalent:keyEquiv]; newMenuItem.tag = aTag; newMenuItem.target = aTarget; @@ -806,7 +812,7 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { [sApplicationMenu addItem:itemBeingAdded]; // set this menu item up as the Mac OS X Services menu - NSMenu* servicesMenu = [[GeckoServicesNSMenu alloc] initWithTitle:@""]; + NSMenu* servicesMenu = [[GeckoNSMenu alloc] initWithTitle:@""]; itemBeingAdded.submenu = servicesMenu; NSApp.servicesMenu = servicesMenu; @@ -896,9 +902,9 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { // the current application does not have a DOM node for "Quit". Add one // anyway, in English. NSMenuItem* defaultQuitItem = - [[[NSMenuItem alloc] initWithTitle:@"Quit" - action:@selector(menuItemHit:) - keyEquivalent:@"q"] autorelease]; + [[[GeckoNSMenuItem alloc] initWithTitle:@"Quit" + action:@selector(menuItemHit:) + keyEquivalent:@"q"] autorelease]; defaultQuitItem.target = nsMenuBarX::sNativeEventTarget; defaultQuitItem.tag = eCommand_ID_Quit; [sApplicationMenu addItem:defaultQuitItem]; @@ -908,15 +914,38 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { NS_OBJC_END_TRY_ABORT_BLOCK; } +// Objective-C class used for menu items to allow Gecko to override their +// standard behavior in order to stop key equivalents from firing in certain +// instances. When gMenuItemsExecuteCommands is NO, we return a dummy target and +// action instead of the actual target and action. +@implementation GeckoNSMenuItem + +- (id)target { + id realTarget = super.target; + if (gMenuItemsExecuteCommands) { + return realTarget; + } + return realTarget ? self : nil; +} + +- (SEL)action { + SEL realAction = super.action; + if (gMenuItemsExecuteCommands) { + return realAction; + } + return realAction ? @selector(_doNothing:) : nullptr; +} + +- (void)_doNothing:(id)aSender { +} + +@end + // // Objective-C class used to allow us to have keyboard commands // look like they are doing something but actually do nothing. // We allow mouse actions to work normally. // - -// Controls whether or not native menu items should invoke their commands. -static BOOL gMenuItemsExecuteCommands = YES; - @implementation GeckoNSMenu // Keyboard commands should not cause menu items to invoke their @@ -945,8 +974,14 @@ static BOOL gMenuItemsExecuteCommands = YES; NSResponder* firstResponder = keyWindow.firstResponder; - gMenuItemsExecuteCommands = NO; + if ([keyWindow isKindOfClass:[BaseWindow class]]) { + gMenuItemsExecuteCommands = NO; + } + + NS_OBJC_BEGIN_TRY_IGNORE_BLOCK [super performKeyEquivalent:aEvent]; + NS_OBJC_END_TRY_IGNORE_BLOCK + gMenuItemsExecuteCommands = YES; // return to default // Return YES if we invoked a command and there is now no key window or we @@ -964,6 +999,46 @@ static BOOL gMenuItemsExecuteCommands = YES; return [super performKeyEquivalent:aEvent]; } +- (void)addItem:(NSMenuItem*)aNewItem { + [self _overrideClassOfMenuItem:aNewItem]; + [super addItem:aNewItem]; +} + +- (NSMenuItem*)addItemWithTitle:(NSString*)aString + action:(SEL)aSelector + keyEquivalent:(NSString*)aKeyEquiv { + NSMenuItem* newItem = [super addItemWithTitle:aString + action:aSelector + keyEquivalent:aKeyEquiv]; + [self _overrideClassOfMenuItem:newItem]; + return newItem; +} + +- (void)insertItem:(NSMenuItem*)aNewItem atIndex:(NSInteger)aIndex { + [self _overrideClassOfMenuItem:aNewItem]; + [super insertItem:aNewItem atIndex:aIndex]; +} + +- (NSMenuItem*)insertItemWithTitle:(NSString*)aString + action:(SEL)aSelector + keyEquivalent:(NSString*)aKeyEquiv + atIndex:(NSInteger)aIndex { + NSMenuItem* newItem = [super insertItemWithTitle:aString + action:aSelector + keyEquivalent:aKeyEquiv + atIndex:aIndex]; + [self _overrideClassOfMenuItem:newItem]; + return newItem; +} + +- (void)_overrideClassOfMenuItem:(NSMenuItem*)aMenuItem { + if ([aMenuItem class] == [NSMenuItem class]) { + // See class comment for `GeckoNSMenuItem` above for an explanation of why + // we do this. + object_setClass(aMenuItem, [GeckoNSMenuItem class]); + } +} + @end // @@ -974,9 +1049,9 @@ static BOOL gMenuItemsExecuteCommands = YES; // called when some menu item in this menu gets hit - (IBAction)menuItemHit:(id)aSender { - if (!gMenuItemsExecuteCommands) { - return; - } + // We should never get here when we do not want menu items to execute their + // commands. + MOZ_RELEASE_ASSERT(gMenuItemsExecuteCommands); if (![aSender isKindOfClass:[NSMenuItem class]]) { return; @@ -1093,77 +1168,3 @@ static BOOL gMenuItemsExecuteCommands = YES; } @end - -// Objective-C class used for menu items on the Services menu to allow Gecko -// to override their standard behavior in order to stop key equivalents from -// firing in certain instances. When gMenuItemsExecuteCommands is NO, we return -// a dummy target and action instead of the actual target and action. - -@implementation GeckoServicesNSMenuItem - -- (id)target { - id realTarget = super.target; - if (gMenuItemsExecuteCommands) { - return realTarget; - } - return realTarget ? self : nil; -} - -- (SEL)action { - SEL realAction = super.action; - if (gMenuItemsExecuteCommands) { - return realAction; - } - return realAction ? @selector(_doNothing:) : nullptr; -} - -- (void)_doNothing:(id)aSender { -} - -@end - -// Objective-C class used as the Services menu so that Gecko can override the -// standard behavior of the Services menu in order to stop key equivalents -// from firing in certain instances. - -@implementation GeckoServicesNSMenu - -- (void)addItem:(NSMenuItem*)aNewItem { - [self _overrideClassOfMenuItem:aNewItem]; - [super addItem:aNewItem]; -} - -- (NSMenuItem*)addItemWithTitle:(NSString*)aString - action:(SEL)aSelector - keyEquivalent:(NSString*)aKeyEquiv { - NSMenuItem* newItem = [super addItemWithTitle:aString - action:aSelector - keyEquivalent:aKeyEquiv]; - [self _overrideClassOfMenuItem:newItem]; - return newItem; -} - -- (void)insertItem:(NSMenuItem*)aNewItem atIndex:(NSInteger)aIndex { - [self _overrideClassOfMenuItem:aNewItem]; - [super insertItem:aNewItem atIndex:aIndex]; -} - -- (NSMenuItem*)insertItemWithTitle:(NSString*)aString - action:(SEL)aSelector - keyEquivalent:(NSString*)aKeyEquiv - atIndex:(NSInteger)aIndex { - NSMenuItem* newItem = [super insertItemWithTitle:aString - action:aSelector - keyEquivalent:aKeyEquiv - atIndex:aIndex]; - [self _overrideClassOfMenuItem:newItem]; - return newItem; -} - -- (void)_overrideClassOfMenuItem:(NSMenuItem*)aMenuItem { - if ([aMenuItem class] == [NSMenuItem class]) { - object_setClass(aMenuItem, [GeckoServicesNSMenuItem class]); - } -} - -@end diff --git a/widget/cocoa/nsMenuItemX.mm b/widget/cocoa/nsMenuItemX.mm index 26241217dc25..bb936c3ef00f 100644 --- a/widget/cocoa/nsMenuItemX.mm +++ b/widget/cocoa/nsMenuItemX.mm @@ -79,9 +79,9 @@ nsMenuItemX::nsMenuItemX(nsMenuX* aParent, const nsString& aLabel, } else { NSString* newCocoaLabelString = nsMenuUtilsX::GetTruncatedCocoaLabel(aLabel); - mNativeMenuItem = [[NSMenuItem alloc] initWithTitle:newCocoaLabelString - action:nil - keyEquivalent:@""]; + mNativeMenuItem = [[GeckoNSMenuItem alloc] initWithTitle:newCocoaLabelString + action:nil + keyEquivalent:@""]; mIsChecked = mContent->AsElement()->AttrValueIs( kNameSpaceID_None, nsGkAtoms::checked, nsGkAtoms::_true, eCaseMatters); diff --git a/widget/cocoa/nsMenuUtilsX.mm b/widget/cocoa/nsMenuUtilsX.mm index 9a950221cd9f..7378ef21bcce 100644 --- a/widget/cocoa/nsMenuUtilsX.mm +++ b/widget/cocoa/nsMenuUtilsX.mm @@ -133,23 +133,24 @@ NSMenuItem* nsMenuUtilsX::GetStandardEditMenuItem() { // we return here isn't always released before it needs to be added to // another menu. See bmo bug 468393. NSMenuItem* standardEditMenuItem = - [[[NSMenuItem alloc] initWithTitle:@"Edit" action:nil - keyEquivalent:@""] autorelease]; - NSMenu* standardEditMenu = [[NSMenu alloc] initWithTitle:@"Edit"]; + [[[GeckoNSMenuItem alloc] initWithTitle:@"Edit" + action:nil + keyEquivalent:@""] autorelease]; + NSMenu* standardEditMenu = [[GeckoNSMenu alloc] initWithTitle:@"Edit"]; standardEditMenuItem.submenu = standardEditMenu; [standardEditMenu release]; // Add Undo - NSMenuItem* undoItem = [[NSMenuItem alloc] initWithTitle:@"Undo" - action:@selector(undo:) - keyEquivalent:@"z"]; + NSMenuItem* undoItem = [[GeckoNSMenuItem alloc] initWithTitle:@"Undo" + action:@selector(undo:) + keyEquivalent:@"z"]; [standardEditMenu addItem:undoItem]; [undoItem release]; // Add Redo - NSMenuItem* redoItem = [[NSMenuItem alloc] initWithTitle:@"Redo" - action:@selector(redo:) - keyEquivalent:@"Z"]; + NSMenuItem* redoItem = [[GeckoNSMenuItem alloc] initWithTitle:@"Redo" + action:@selector(redo:) + keyEquivalent:@"Z"]; [standardEditMenu addItem:redoItem]; [redoItem release]; @@ -157,38 +158,40 @@ NSMenuItem* nsMenuUtilsX::GetStandardEditMenuItem() { [standardEditMenu addItem:[NSMenuItem separatorItem]]; // Add Cut - NSMenuItem* cutItem = [[NSMenuItem alloc] initWithTitle:@"Cut" - action:@selector(cut:) - keyEquivalent:@"x"]; + NSMenuItem* cutItem = [[GeckoNSMenuItem alloc] initWithTitle:@"Cut" + action:@selector(cut:) + keyEquivalent:@"x"]; [standardEditMenu addItem:cutItem]; [cutItem release]; // Add Copy - NSMenuItem* copyItem = [[NSMenuItem alloc] initWithTitle:@"Copy" - action:@selector(copy:) - keyEquivalent:@"c"]; + NSMenuItem* copyItem = [[GeckoNSMenuItem alloc] initWithTitle:@"Copy" + action:@selector(copy:) + keyEquivalent:@"c"]; [standardEditMenu addItem:copyItem]; [copyItem release]; // Add Paste - NSMenuItem* pasteItem = [[NSMenuItem alloc] initWithTitle:@"Paste" - action:@selector(paste:) - keyEquivalent:@"v"]; + NSMenuItem* pasteItem = + [[GeckoNSMenuItem alloc] initWithTitle:@"Paste" + action:@selector(paste:) + keyEquivalent:@"v"]; [standardEditMenu addItem:pasteItem]; [pasteItem release]; // Add Delete - NSMenuItem* deleteItem = [[NSMenuItem alloc] initWithTitle:@"Delete" - action:@selector(delete:) - keyEquivalent:@""]; + NSMenuItem* deleteItem = + [[GeckoNSMenuItem alloc] initWithTitle:@"Delete" + action:@selector(delete:) + keyEquivalent:@""]; [standardEditMenu addItem:deleteItem]; [deleteItem release]; // Add Select All NSMenuItem* selectAllItem = - [[NSMenuItem alloc] initWithTitle:@"Select All" - action:@selector(selectAll:) - keyEquivalent:@"a"]; + [[GeckoNSMenuItem alloc] initWithTitle:@"Select All" + action:@selector(selectAll:) + keyEquivalent:@"a"]; [standardEditMenu addItem:selectAllItem]; [selectAllItem release]; diff --git a/widget/cocoa/nsMenuX.mm b/widget/cocoa/nsMenuX.mm index d6003c4faf46..3eb56b71d22f 100644 --- a/widget/cocoa/nsMenuX.mm +++ b/widget/cocoa/nsMenuX.mm @@ -116,9 +116,9 @@ nsMenuX::nsMenuX(nsMenuParentX* aParent, nsMenuGroupOwnerX* aMenuGroupOwner, mVisible = !nsMenuUtilsX::NodeIsHiddenOrCollapsed(mContent); NSString* newCocoaLabelString = nsMenuUtilsX::GetTruncatedCocoaLabel(mLabel); - mNativeMenuItem = [[NSMenuItem alloc] initWithTitle:newCocoaLabelString - action:nil - keyEquivalent:@""]; + mNativeMenuItem = [[GeckoNSMenuItem alloc] initWithTitle:newCocoaLabelString + action:nil + keyEquivalent:@""]; mNativeMenuItem.submenu = mNativeMenu; SetEnabled(!mContent->IsElement() || @@ -794,9 +794,9 @@ void nsMenuX::InsertPlaceholderIfNeeded() { if ([mNativeMenu numberOfItems] == 0) { MOZ_RELEASE_ASSERT(mVisibleItemsCount == 0); - NSMenuItem* item = [[NSMenuItem alloc] initWithTitle:@"" - action:nil - keyEquivalent:@""]; + NSMenuItem* item = [[GeckoNSMenuItem alloc] initWithTitle:@"" + action:nil + keyEquivalent:@""]; item.enabled = NO; item.view = [[[NSView alloc] initWithFrame:NSMakeRect(0, 0, 150, 1)] autorelease];