diff --git a/widget/cocoa/NativeMenuSupport.mm b/widget/cocoa/NativeMenuSupport.mm index 7542dc9fbd37..54047feda344 100644 --- a/widget/cocoa/NativeMenuSupport.mm +++ b/widget/cocoa/NativeMenuSupport.mm @@ -6,7 +6,6 @@ #include "mozilla/widget/NativeMenuSupport.h" #include "MainThreadUtils.h" -#include "nsCocoaWindow.h" #include "nsMenuBarX.h" namespace mozilla::widget { @@ -16,11 +15,8 @@ void NativeMenuSupport::CreateNativeMenuBar(nsIWidget* aParent, dom::Element* aM RefPtr mb = new nsMenuBarX(); - nsresult rv = mb->Create(aMenuBarElement); + nsresult rv = mb->Create(aParent, aMenuBarElement); MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv)); - - // Give the menubar to the parent window. The parent takes ownership. - static_cast(aParent)->SetMenuBar(std::move(mb)); } } // namespace mozilla::widget diff --git a/widget/cocoa/nsCocoaWindow.h b/widget/cocoa/nsCocoaWindow.h index a922551a6f71..bd341bd2f0f8 100644 --- a/widget/cocoa/nsCocoaWindow.h +++ b/widget/cocoa/nsCocoaWindow.h @@ -327,7 +327,7 @@ class nsCocoaWindow final : public nsBaseWidget, public nsPIWidgetCocoa { bool HasModalDescendents() { return mNumModalDescendents > 0; } NSWindow* GetCocoaWindow() { return mWindow; } - void SetMenuBar(RefPtr&& aMenuBar); + void SetMenuBar(nsMenuBarX* aMenuBar); nsMenuBarX* GetMenuBar(); virtual void SetInputContext(const InputContext& aContext, diff --git a/widget/cocoa/nsCocoaWindow.mm b/widget/cocoa/nsCocoaWindow.mm index d68414316565..65f1f0dce2c4 100644 --- a/widget/cocoa/nsCocoaWindow.mm +++ b/widget/cocoa/nsCocoaWindow.mm @@ -2130,12 +2130,13 @@ void nsCocoaWindow::ResumeCompositor() { Unused << bc->SetExplicitActive(ExplicitActiveStatus::Active); } -void nsCocoaWindow::SetMenuBar(RefPtr&& aMenuBar) { +void nsCocoaWindow::SetMenuBar(nsMenuBarX* aMenuBar) { + if (mMenuBar) mMenuBar->SetParent(nullptr); if (!mWindow) { mMenuBar = nullptr; return; } - mMenuBar = std::move(aMenuBar); + mMenuBar = aMenuBar; // Only paint for active windows, or paint the hidden window menu bar if no // other menu bar has been painted yet so that some reasonable menu bar is diff --git a/widget/cocoa/nsMenuBarX.h b/widget/cocoa/nsMenuBarX.h index 73e3cb9bd414..f46c8fb8844c 100644 --- a/widget/cocoa/nsMenuBarX.h +++ b/widget/cocoa/nsMenuBarX.h @@ -37,13 +37,13 @@ class Element; // We allow mouse actions to work normally. @interface GeckoNSMenu : NSMenu { } -- (BOOL)performSuperKeyEquivalent:(NSEvent*)aEvent; +- (BOOL)performSuperKeyEquivalent:(NSEvent*)theEvent; @end // Objective-C class used as action target for menu items @interface NativeMenuItemTarget : NSObject { } -- (IBAction)menuItemHit:(id)aSender; +- (IBAction)menuItemHit:(id)sender; @end // Objective-C class used for menu items on the Services menu to allow Gecko @@ -53,7 +53,7 @@ class Element; } - (id)target; - (SEL)action; -- (void)_doNothing:(id)aSender; +- (void)_doNothing:(id)sender; @end // Objective-C class used as the Services menu so that Gecko can override the @@ -61,16 +61,16 @@ class Element; // from firing in certain instances. @interface GeckoServicesNSMenu : NSMenu { } -- (void)addItem:(NSMenuItem*)aNewItem; +- (void)addItem:(NSMenuItem*)newItem; - (NSMenuItem*)addItemWithTitle:(NSString*)aString action:(SEL)aSelector - keyEquivalent:(NSString*)aKeyEquiv; -- (void)insertItem:(NSMenuItem*)aNewItem atIndex:(NSInteger)aIndex; + keyEquivalent:(NSString*)keyEquiv; +- (void)insertItem:(NSMenuItem*)newItem atIndex:(NSInteger)index; - (NSMenuItem*)insertItemWithTitle:(NSString*)aString action:(SEL)aSelector - keyEquivalent:(NSString*)aKeyEquiv - atIndex:(NSInteger)aIndex; -- (void)_overrideClassOfMenuItem:(NSMenuItem*)aMenuItem; + keyEquivalent:(NSString*)keyEquiv + atIndex:(NSInteger)index; +- (void)_overrideClassOfMenuItem:(NSMenuItem*)menuItem; @end // Once instantiated, this object lives until its DOM node or its parent window is destroyed. @@ -97,33 +97,34 @@ class nsMenuBarX : public nsMenuGroupOwnerX, public nsChangeObserver { nsMenuObjectTypeX MenuObjectType() override { return eMenuBarObjectType; } // nsMenuBarX - nsresult Create(mozilla::dom::Element* aElement); + nsresult Create(nsIWidget* aParent, mozilla::dom::Element* aElement); + void SetParent(nsIWidget* aParent); uint32_t GetMenuCount(); bool MenuContainsAppMenu(); nsMenuX* GetMenuAt(uint32_t aIndex); nsMenuX* GetXULHelpMenu(); void SetSystemHelpMenu(); nsresult Paint(); - void ForceUpdateNativeMenuAt(const nsAString& aIndexString); + void ForceUpdateNativeMenuAt(const nsAString& indexString); void ForceNativeMenuReload(); // used for testing static void ResetNativeApplicationMenu(); void SetNeedsRebuild(); void ApplicationMenuOpened(); - bool PerformKeyEquivalent(NSEvent* aEvent); + bool PerformKeyEquivalent(NSEvent* theEvent); protected: void ConstructNativeMenus(); void ConstructFallbackNativeMenus(); - void InsertMenuAtIndex(mozilla::UniquePtr&& aMenu, uint32_t aIndex); + nsresult InsertMenuAtIndex(nsMenuX* aMenu, uint32_t aIndex); void RemoveMenuAtIndex(uint32_t aIndex); - already_AddRefed HideItem(mozilla::dom::Document* aDocument, - const nsAString& aID); + void HideItem(mozilla::dom::Document* inDoc, const nsAString& inID, nsIContent** outHiddenNode); void AquifyMenuBar(); - NSMenuItem* CreateNativeAppMenuItem(nsMenuX* aMenu, const nsAString& aNodeID, SEL aAction, - int aTag, NativeMenuItemTarget* aTarget); - void CreateApplicationMenu(nsMenuX* aMenu); + NSMenuItem* CreateNativeAppMenuItem(nsMenuX* inMenu, const nsAString& nodeID, SEL action, int tag, + NativeMenuItemTarget* target); + nsresult CreateApplicationMenu(nsMenuX* inMenu); nsTArray> mMenuArray; + nsIWidget* mParentWindow; // [weak] GeckoNSMenu* mNativeMenu; // root menu, representing entire menu bar bool mNeedsRebuild; ApplicationMenuDelegate* mApplicationMenuDelegate; diff --git a/widget/cocoa/nsMenuBarX.mm b/widget/cocoa/nsMenuBarX.mm index 4feea6282287..0e0216629f05 100644 --- a/widget/cocoa/nsMenuBarX.mm +++ b/widget/cocoa/nsMenuBarX.mm @@ -6,11 +6,11 @@ #include #include "nsMenuBarX.h" -#include "nsMenuBaseX.h" #include "nsMenuX.h" #include "nsMenuItemX.h" #include "nsMenuUtilsX.h" #include "nsCocoaUtils.h" +#include "nsCocoaWindow.h" #include "nsChildView.h" #include "nsCOMPtr.h" @@ -30,9 +30,6 @@ #include "mozilla/Components.h" #include "mozilla/dom/Element.h" -using namespace mozilla; -using mozilla::dom::Element; - NativeMenuItemTarget* nsMenuBarX::sNativeEventTarget = nil; nsMenuBarX* nsMenuBarX::sLastGeckoMenuBarPainted = nullptr; NSMenu* sApplicationMenu = nil; @@ -73,7 +70,10 @@ static nsIContent* sQuitItemContent = nullptr; @end nsMenuBarX::nsMenuBarX() - : nsMenuGroupOwnerX(), mNeedsRebuild(false), mApplicationMenuDelegate(nil) { + : nsMenuGroupOwnerX(), + mParentWindow(nullptr), + mNeedsRebuild(false), + mApplicationMenuDelegate(nil) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; mNativeMenu = [[GeckoNSMenu alloc] initWithTitle:@"MainMenuBar"]; @@ -120,9 +120,14 @@ nsMenuBarX::~nsMenuBarX() { NS_OBJC_END_TRY_ABORT_BLOCK; } -nsresult nsMenuBarX::Create(Element* aContent) { +nsresult nsMenuBarX::Create(nsIWidget* aParent, Element* aContent) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; + if (!aParent) { + return NS_ERROR_INVALID_ARG; + } + + mParentWindow = aParent; mContent = aContent; if (mContent) { @@ -139,6 +144,9 @@ nsresult nsMenuBarX::Create(Element* aContent) { ConstructFallbackNativeMenus(); } + // Give this to the parent window. The parent takes ownership. + static_cast(mParentWindow)->SetMenuBar(this); + return NS_OK; NS_OBJC_END_TRY_ABORT_BLOCK; @@ -148,7 +156,15 @@ void nsMenuBarX::ConstructNativeMenus() { for (nsIContent* menuContent = mContent->GetFirstChild(); menuContent; menuContent = menuContent->GetNextSibling()) { if (menuContent->IsXULElement(nsGkAtoms::menu)) { - InsertMenuAtIndex(MakeUnique(this, this, menuContent->AsElement()), GetMenuCount()); + nsMenuX* newMenu = new nsMenuX(); + if (newMenu) { + nsresult rv = newMenu->Create(this, this, menuContent->AsElement()); + if (NS_SUCCEEDED(rv)) { + InsertMenuAtIndex(newMenu, GetMenuCount()); + } else { + delete newMenu; + } + } } } } @@ -191,12 +207,12 @@ void nsMenuBarX::ConstructFallbackNativeMenus() { if (!mApplicationMenuDelegate) { mApplicationMenuDelegate = [[ApplicationMenuDelegate alloc] initWithApplicationMenu:this]; } - sApplicationMenu.delegate = mApplicationMenuDelegate; + [sApplicationMenu setDelegate:mApplicationMenuDelegate]; NSMenuItem* quitMenuItem = [[[NSMenuItem alloc] initWithTitle:labelStr action:@selector(menuItemHit:) keyEquivalent:keyStr] autorelease]; - quitMenuItem.target = nsMenuBarX::sNativeEventTarget; - quitMenuItem.tag = eCommand_ID_Quit; + [quitMenuItem setTarget:nsMenuBarX::sNativeEventTarget]; + [quitMenuItem setTag:eCommand_ID_Quit]; [sApplicationMenu addItem:quitMenuItem]; sApplicationMenuIsFallback = YES; @@ -208,12 +224,13 @@ uint32_t nsMenuBarX::GetMenuCount() { return mMenuArray.Length(); } bool nsMenuBarX::MenuContainsAppMenu() { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; - return (mNativeMenu.numberOfItems > 0 && [mNativeMenu itemAtIndex:0].submenu == sApplicationMenu); + return ([mNativeMenu numberOfItems] > 0 && + [[mNativeMenu itemAtIndex:0] submenu] == sApplicationMenu); NS_OBJC_END_TRY_ABORT_BLOCK; } -void nsMenuBarX::InsertMenuAtIndex(UniquePtr&& aMenu, uint32_t aIndex) { +nsresult nsMenuBarX::InsertMenuAtIndex(nsMenuX* aMenu, uint32_t aIndex) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; // If we've only yet created a fallback global Application menu (using @@ -223,29 +240,32 @@ void nsMenuBarX::InsertMenuAtIndex(UniquePtr&& aMenu, uint32_t aIndex) } // If we haven't created a global Application menu yet, do it. if (!sApplicationMenu) { - CreateApplicationMenu(aMenu.get()); + nsresult rv = NS_OK; // avoid warning about rv being unused + rv = CreateApplicationMenu(aMenu); + NS_ASSERTION(NS_SUCCEEDED(rv), "Can't create Application menu"); // Hook the new Application menu up to the menu bar. - NSMenu* mainMenu = NSApp.mainMenu; - NS_ASSERTION(mainMenu.numberOfItems > 0, + NSMenu* mainMenu = [NSApp mainMenu]; + NS_ASSERTION([mainMenu numberOfItems] > 0, "Main menu does not have any items, something is terribly wrong!"); - [mainMenu itemAtIndex:0].submenu = sApplicationMenu; + [[mainMenu itemAtIndex:0] setSubmenu:sApplicationMenu]; } // add menu to array that owns our menus - nsMenuX* menu = aMenu.get(); - mMenuArray.InsertElementAt(aIndex, std::move(aMenu)); + mMenuArray.InsertElementAt(aIndex, aMenu); // hook up submenus - nsIContent* menuContent = menu->Content(); + nsIContent* menuContent = aMenu->Content(); if (menuContent->GetChildCount() > 0 && !nsMenuUtilsX::NodeIsHiddenOrCollapsed(menuContent)) { - int insertionIndex = nsMenuUtilsX::CalculateNativeInsertionPoint(this, menu); + int insertionIndex = nsMenuUtilsX::CalculateNativeInsertionPoint(this, aMenu); if (MenuContainsAppMenu()) { insertionIndex++; } - [mNativeMenu insertItem:menu->NativeMenuItem() atIndex:insertionIndex]; + [mNativeMenu insertItem:aMenu->NativeMenuItem() atIndex:insertionIndex]; } + return NS_OK; + NS_OBJC_END_TRY_ABORT_BLOCK; } @@ -284,17 +304,25 @@ void nsMenuBarX::ObserveContentRemoved(mozilla::dom::Document* aDocument, nsICon void nsMenuBarX::ObserveContentInserted(mozilla::dom::Document* aDocument, nsIContent* aContainer, nsIContent* aChild) { - InsertMenuAtIndex(MakeUnique(this, this, aChild), aContainer->ComputeIndexOf(aChild)); + nsMenuX* newMenu = new nsMenuX(); + if (newMenu) { + nsresult rv = newMenu->Create(this, this, aChild); + if (NS_SUCCEEDED(rv)) { + InsertMenuAtIndex(newMenu, aContainer->ComputeIndexOf(aChild)); + } else { + delete newMenu; + } + } } -void nsMenuBarX::ForceUpdateNativeMenuAt(const nsAString& aIndexString) { +void nsMenuBarX::ForceUpdateNativeMenuAt(const nsAString& indexString) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; NSString* locationString = - [NSString stringWithCharacters:reinterpret_cast(aIndexString.BeginReading()) - length:aIndexString.Length()]; + [NSString stringWithCharacters:reinterpret_cast(indexString.BeginReading()) + length:indexString.Length()]; NSArray* indexes = [locationString componentsSeparatedByString:@"|"]; - unsigned int indexCount = indexes.count; + unsigned int indexCount = [indexes count]; if (indexCount == 0) { return; } @@ -333,12 +361,7 @@ void nsMenuBarX::ForceUpdateNativeMenuAt(const nsAString& aIndexString) { if (!targetMenu) { return; } - MOZ_RELEASE_ASSERT(targetMenu->MenuObjectType() == eSubmenuObjectType || - targetMenu->MenuObjectType() == eMenuItemObjectType); - RefPtr content = targetMenu->MenuObjectType() == eSubmenuObjectType - ? static_cast(targetMenu)->Content() - : static_cast(targetMenu)->Content(); - if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(content)) { + if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(targetMenu->Content())) { visible++; if (targetMenu->MenuObjectType() == eSubmenuObjectType && visible == (targetIndex + 1)) { currentMenu = static_cast(targetMenu); @@ -399,7 +422,7 @@ void nsMenuBarX::SetSystemHelpMenu() { if (xulHelpMenu) { NSMenu* helpMenu = (NSMenu*)xulHelpMenu->NativeData(); if (helpMenu) { - NSApp.helpMenu = helpMenu; + [NSApp setHelpMenu:helpMenu]; } } @@ -415,8 +438,8 @@ nsresult nsMenuBarX::Paint() { // We have to keep the same menu item for the Application menu so we keep // passing it along. - NSMenu* outgoingMenu = NSApp.mainMenu; - NS_ASSERTION(outgoingMenu.numberOfItems > 0, + NSMenu* outgoingMenu = [NSApp mainMenu]; + NS_ASSERTION([outgoingMenu numberOfItems] > 0, "Main menu does not have any items, something is terribly wrong!"); NSMenuItem* appMenuItem = [[outgoingMenu itemAtIndex:0] retain]; @@ -425,7 +448,7 @@ nsresult nsMenuBarX::Paint() { [appMenuItem release]; // Set menu bar and event target. - NSApp.mainMenu = mNativeMenu; + [NSApp setMainMenu:mNativeMenu]; SetSystemHelpMenu(); nsMenuBarX::sLastGeckoMenuBarPainted = this; @@ -460,19 +483,23 @@ void nsMenuBarX::ApplicationMenuOpened() { } } -bool nsMenuBarX::PerformKeyEquivalent(NSEvent* aEvent) { - return [mNativeMenu performSuperKeyEquivalent:aEvent]; +bool nsMenuBarX::PerformKeyEquivalent(NSEvent* theEvent) { + return [mNativeMenu performSuperKeyEquivalent:theEvent]; } -// Hide the item in the menu by setting the 'hidden' attribute. Returns it so -// the caller can hang onto it if they so choose. -already_AddRefed nsMenuBarX::HideItem(mozilla::dom::Document* aDocument, - const nsAString& aID) { - nsCOMPtr menuElement = aDocument->GetElementById(aID); +// Hide the item in the menu by setting the 'hidden' attribute. Returns it in |outHiddenNode| so +// the caller can hang onto it if they so choose. It is acceptable to pass nsull +// for |outHiddenNode| if the caller doesn't care about the hidden node. +void nsMenuBarX::HideItem(mozilla::dom::Document* inDoc, const nsAString& inID, + nsIContent** outHiddenNode) { + nsCOMPtr menuElement = inDoc->GetElementById(inID); if (menuElement) { menuElement->SetAttr(kNameSpaceID_None, nsGkAtoms::hidden, u"true"_ns, false); + if (outHiddenNode) { + *outHiddenNode = menuElement.get(); + NS_IF_ADDREF(*outHiddenNode); + } } - return menuElement.forget(); } // Do what is necessary to conform to the Aqua guidelines for menus. @@ -480,48 +507,47 @@ void nsMenuBarX::AquifyMenuBar() { RefPtr domDoc = mContent->GetComposedDoc(); if (domDoc) { // remove the "About..." item and its separator - HideItem(domDoc, u"aboutSeparator"_ns); - mAboutItemContent = HideItem(domDoc, u"aboutName"_ns); + HideItem(domDoc, u"aboutSeparator"_ns, nullptr); + HideItem(domDoc, u"aboutName"_ns, getter_AddRefs(mAboutItemContent)); if (!sAboutItemContent) { sAboutItemContent = mAboutItemContent; } // remove quit item and its separator - HideItem(domDoc, u"menu_FileQuitSeparator"_ns); - mQuitItemContent = HideItem(domDoc, u"menu_FileQuitItem"_ns); + HideItem(domDoc, u"menu_FileQuitSeparator"_ns, nullptr); + HideItem(domDoc, u"menu_FileQuitItem"_ns, getter_AddRefs(mQuitItemContent)); if (!sQuitItemContent) { sQuitItemContent = mQuitItemContent; } // remove prefs item and its separator, but save off the pref content node // so we can invoke its command later. - HideItem(domDoc, u"menu_PrefsSeparator"_ns); - mPrefItemContent = HideItem(domDoc, u"menu_preferences"_ns); + HideItem(domDoc, u"menu_PrefsSeparator"_ns, nullptr); + HideItem(domDoc, u"menu_preferences"_ns, getter_AddRefs(mPrefItemContent)); if (!sPrefItemContent) { sPrefItemContent = mPrefItemContent; } // hide items that we use for the Application menu - HideItem(domDoc, u"menu_mac_services"_ns); - HideItem(domDoc, u"menu_mac_hide_app"_ns); - HideItem(domDoc, u"menu_mac_hide_others"_ns); - HideItem(domDoc, u"menu_mac_show_all"_ns); - HideItem(domDoc, u"menu_mac_touch_bar"_ns); + HideItem(domDoc, u"menu_mac_services"_ns, nullptr); + HideItem(domDoc, u"menu_mac_hide_app"_ns, nullptr); + HideItem(domDoc, u"menu_mac_hide_others"_ns, nullptr); + HideItem(domDoc, u"menu_mac_show_all"_ns, nullptr); + HideItem(domDoc, u"menu_mac_touch_bar"_ns, nullptr); } } // for creating menu items destined for the Application menu -NSMenuItem* nsMenuBarX::CreateNativeAppMenuItem(nsMenuX* aMenu, const nsAString& aNodeID, - SEL aAction, int aTag, - NativeMenuItemTarget* aTarget) { +NSMenuItem* nsMenuBarX::CreateNativeAppMenuItem(nsMenuX* inMenu, const nsAString& nodeID, + SEL action, int tag, NativeMenuItemTarget* target) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; - RefPtr doc = aMenu->Content()->GetUncomposedDoc(); + RefPtr doc = inMenu->Content()->GetUncomposedDoc(); if (!doc) { return nil; } - RefPtr menuItem = doc->GetElementById(aNodeID); + RefPtr menuItem = doc->GetElementById(nodeID); if (!menuItem) { return nil; } @@ -576,15 +602,15 @@ NSMenuItem* nsMenuBarX::CreateNativeAppMenuItem(nsMenuX* aMenu, const nsAString& // put together the actual NSMenuItem NSMenuItem* newMenuItem = [[NSMenuItem alloc] initWithTitle:labelString - action:aAction + action:action keyEquivalent:keyEquiv]; - newMenuItem.tag = aTag; - newMenuItem.target = aTarget; - newMenuItem.keyEquivalentModifierMask = macKeyModifiers; + [newMenuItem setTag:tag]; + [newMenuItem setTarget:target]; + [newMenuItem setKeyEquivalentModifierMask:macKeyModifiers]; MenuItemInfo* info = [[MenuItemInfo alloc] initWithMenuGroupOwner:this]; - newMenuItem.representedObject = info; + [newMenuItem setRepresentedObject:info]; [info release]; return newMenuItem; @@ -593,13 +619,13 @@ NSMenuItem* nsMenuBarX::CreateNativeAppMenuItem(nsMenuX* aMenu, const nsAString& } // build the Application menu shared by all menu bars -void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { +nsresult nsMenuBarX::CreateApplicationMenu(nsMenuX* inMenu) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; // At this point, the application menu is the application menu from // the nib in cocoa widgets. We do not have a way to create an application // menu manually, so we grab the one from the nib and use that. - sApplicationMenu = [[NSApp.mainMenu itemAtIndex:0].submenu retain]; + sApplicationMenu = [[[[NSApp mainMenu] itemAtIndex:0] submenu] retain]; /* We support the following menu items here: @@ -640,7 +666,7 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { if (!mApplicationMenuDelegate) { mApplicationMenuDelegate = [[ApplicationMenuDelegate alloc] initWithApplicationMenu:this]; } - sApplicationMenu.delegate = mApplicationMenuDelegate; + [sApplicationMenu setDelegate:mApplicationMenuDelegate]; // This code reads attributes we are going to care about from the DOM elements @@ -648,7 +674,7 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { BOOL addAboutSeparator = FALSE; // Add the About menu item - itemBeingAdded = CreateNativeAppMenuItem(aMenu, u"aboutName"_ns, @selector(menuItemHit:), + itemBeingAdded = CreateNativeAppMenuItem(inMenu, u"aboutName"_ns, @selector(menuItemHit:), eCommand_ID_About, nsMenuBarX::sNativeEventTarget); if (itemBeingAdded) { [sApplicationMenu addItem:itemBeingAdded]; @@ -664,8 +690,9 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { } // Add the Preferences menu item - itemBeingAdded = CreateNativeAppMenuItem(aMenu, u"menu_preferences"_ns, @selector(menuItemHit:), - eCommand_ID_Prefs, nsMenuBarX::sNativeEventTarget); + itemBeingAdded = + CreateNativeAppMenuItem(inMenu, u"menu_preferences"_ns, @selector(menuItemHit:), + eCommand_ID_Prefs, nsMenuBarX::sNativeEventTarget); if (itemBeingAdded) { [sApplicationMenu addItem:itemBeingAdded]; [itemBeingAdded release]; @@ -676,14 +703,14 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { } // Add Services menu item - itemBeingAdded = CreateNativeAppMenuItem(aMenu, u"menu_mac_services"_ns, nil, 0, nil); + itemBeingAdded = CreateNativeAppMenuItem(inMenu, u"menu_mac_services"_ns, nil, 0, nil); if (itemBeingAdded) { [sApplicationMenu addItem:itemBeingAdded]; // set this menu item up as the Mac OS X Services menu NSMenu* servicesMenu = [[GeckoServicesNSMenu alloc] initWithTitle:@""]; - itemBeingAdded.submenu = servicesMenu; - NSApp.servicesMenu = servicesMenu; + [itemBeingAdded setSubmenu:servicesMenu]; + [NSApp setServicesMenu:servicesMenu]; [itemBeingAdded release]; itemBeingAdded = nil; @@ -696,7 +723,7 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { // Add menu item to hide this application itemBeingAdded = - CreateNativeAppMenuItem(aMenu, u"menu_mac_hide_app"_ns, @selector(menuItemHit:), + CreateNativeAppMenuItem(inMenu, u"menu_mac_hide_app"_ns, @selector(menuItemHit:), eCommand_ID_HideApp, nsMenuBarX::sNativeEventTarget); if (itemBeingAdded) { [sApplicationMenu addItem:itemBeingAdded]; @@ -708,7 +735,7 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { // Add menu item to hide other applications itemBeingAdded = - CreateNativeAppMenuItem(aMenu, u"menu_mac_hide_others"_ns, @selector(menuItemHit:), + CreateNativeAppMenuItem(inMenu, u"menu_mac_hide_others"_ns, @selector(menuItemHit:), eCommand_ID_HideOthers, nsMenuBarX::sNativeEventTarget); if (itemBeingAdded) { [sApplicationMenu addItem:itemBeingAdded]; @@ -720,7 +747,7 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { // Add menu item to show all applications itemBeingAdded = - CreateNativeAppMenuItem(aMenu, u"menu_mac_show_all"_ns, @selector(menuItemHit:), + CreateNativeAppMenuItem(inMenu, u"menu_mac_show_all"_ns, @selector(menuItemHit:), eCommand_ID_ShowAll, nsMenuBarX::sNativeEventTarget); if (itemBeingAdded) { [sApplicationMenu addItem:itemBeingAdded]; @@ -739,7 +766,7 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { // Add Touch Bar customization menu item. itemBeingAdded = - CreateNativeAppMenuItem(aMenu, u"menu_mac_touch_bar"_ns, @selector(menuItemHit:), + CreateNativeAppMenuItem(inMenu, u"menu_mac_touch_bar"_ns, @selector(menuItemHit:), eCommand_ID_TouchBar, nsMenuBarX::sNativeEventTarget); if (itemBeingAdded) { @@ -761,7 +788,7 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { // Add quit menu item itemBeingAdded = - CreateNativeAppMenuItem(aMenu, u"menu_FileQuitItem"_ns, @selector(menuItemHit:), + CreateNativeAppMenuItem(inMenu, u"menu_FileQuitItem"_ns, @selector(menuItemHit:), eCommand_ID_Quit, nsMenuBarX::sNativeEventTarget); if (itemBeingAdded) { [sApplicationMenu addItem:itemBeingAdded]; @@ -773,15 +800,19 @@ void nsMenuBarX::CreateApplicationMenu(nsMenuX* aMenu) { NSMenuItem* defaultQuitItem = [[[NSMenuItem alloc] initWithTitle:@"Quit" action:@selector(menuItemHit:) keyEquivalent:@"q"] autorelease]; - defaultQuitItem.target = nsMenuBarX::sNativeEventTarget; - defaultQuitItem.tag = eCommand_ID_Quit; + [defaultQuitItem setTarget:nsMenuBarX::sNativeEventTarget]; + [defaultQuitItem setTag:eCommand_ID_Quit]; [sApplicationMenu addItem:defaultQuitItem]; } } + return (sApplicationMenu) ? NS_OK : NS_ERROR_FAILURE; + NS_OBJC_END_TRY_ABORT_BLOCK; } +void nsMenuBarX::SetParent(nsIWidget* aParent) { mParentWindow = aParent; } + // // Objective-C class used to allow us to have keyboard commands // look like they are doing something but actually do nothing. @@ -798,35 +829,35 @@ static BOOL gMenuItemsExecuteCommands = YES; // the keyboard command to the window. We still have the menus // go through the mechanics so they'll give the proper visual // feedback. -- (BOOL)performKeyEquivalent:(NSEvent*)aEvent { +- (BOOL)performKeyEquivalent:(NSEvent*)theEvent { // We've noticed that Mac OS X expects this check in subclasses before // calling NSMenu's "performKeyEquivalent:". // // There is no case in which we'd need to do anything or return YES // when we have no items so we can just do this check first. - if (self.numberOfItems <= 0) { + if ([self numberOfItems] <= 0) { return NO; } - NSWindow* keyWindow = NSApp.keyWindow; + NSWindow* keyWindow = [NSApp keyWindow]; // If there is no key window then just behave normally. This // probably means that this menu is associated with Gecko's // hidden window. if (!keyWindow) { - return [super performKeyEquivalent:aEvent]; + return [super performKeyEquivalent:theEvent]; } - NSResponder* firstResponder = keyWindow.firstResponder; + NSResponder* firstResponder = [keyWindow firstResponder]; gMenuItemsExecuteCommands = NO; - [super performKeyEquivalent:aEvent]; + [super performKeyEquivalent:theEvent]; gMenuItemsExecuteCommands = YES; // return to default // Return YES if we invoked a command and there is now no key window or we changed // the first responder. In this case we do not want to propagate the event because // we don't want it handled again. - if (!NSApp.keyWindow || NSApp.keyWindow.firstResponder != firstResponder) { + if (![NSApp keyWindow] || [[NSApp keyWindow] firstResponder] != firstResponder) { return YES; } @@ -834,8 +865,8 @@ static BOOL gMenuItemsExecuteCommands = YES; return NO; } -- (BOOL)performSuperKeyEquivalent:(NSEvent*)aEvent { - return [super performKeyEquivalent:aEvent]; +- (BOOL)performSuperKeyEquivalent:(NSEvent*)theEvent { + return [super performKeyEquivalent:theEvent]; } @end @@ -847,19 +878,19 @@ static BOOL gMenuItemsExecuteCommands = YES; @implementation NativeMenuItemTarget // called when some menu item in this menu gets hit -- (IBAction)menuItemHit:(id)aSender { +- (IBAction)menuItemHit:(id)sender { if (!gMenuItemsExecuteCommands) { return; } - int tag = [aSender tag]; + int tag = [sender tag]; nsMenuGroupOwnerX* menuGroupOwner = nullptr; nsMenuBarX* menuBar = nullptr; - MenuItemInfo* info = [aSender representedObject]; + MenuItemInfo* info = [sender representedObject]; if (info) { - menuGroupOwner = info.menuGroupOwner; + menuGroupOwner = [info menuGroupOwner]; if (!menuGroupOwner) { return; } @@ -886,19 +917,19 @@ static BOOL gMenuItemsExecuteCommands = YES; return; } if (tag == eCommand_ID_HideApp) { - [NSApp hide:aSender]; + [NSApp hide:sender]; return; } if (tag == eCommand_ID_HideOthers) { - [NSApp hideOtherApplications:aSender]; + [NSApp hideOtherApplications:sender]; return; } if (tag == eCommand_ID_ShowAll) { - [NSApp unhideAllApplications:aSender]; + [NSApp unhideAllApplications:sender]; return; } if (tag == eCommand_ID_TouchBar) { - [NSApp toggleTouchBarCustomizationPalette:aSender]; + [NSApp toggleTouchBarCustomizationPalette:sender]; return; } if (tag == eCommand_ID_Quit) { @@ -941,7 +972,7 @@ static BOOL gMenuItemsExecuteCommands = YES; @implementation GeckoServicesNSMenuItem - (id)target { - id realTarget = super.target; + id realTarget = [super target]; if (gMenuItemsExecuteCommands) { return realTarget; } @@ -949,14 +980,14 @@ static BOOL gMenuItemsExecuteCommands = YES; } - (SEL)action { - SEL realAction = super.action; + SEL realAction = [super action]; if (gMenuItemsExecuteCommands) { return realAction; } return realAction ? @selector(_doNothing:) : nullptr; } -- (void)_doNothing:(id)aSender { +- (void)_doNothing:(id)sender { } @end @@ -967,39 +998,39 @@ static BOOL gMenuItemsExecuteCommands = YES; @implementation GeckoServicesNSMenu -- (void)addItem:(NSMenuItem*)aNewItem { - [self _overrideClassOfMenuItem:aNewItem]; - [super addItem:aNewItem]; +- (void)addItem:(NSMenuItem*)newItem { + [self _overrideClassOfMenuItem:newItem]; + [super addItem:newItem]; } - (NSMenuItem*)addItemWithTitle:(NSString*)aString action:(SEL)aSelector - keyEquivalent:(NSString*)aKeyEquiv { - NSMenuItem* newItem = [super addItemWithTitle:aString action:aSelector keyEquivalent:aKeyEquiv]; + keyEquivalent:(NSString*)keyEquiv { + NSMenuItem* newItem = [super addItemWithTitle:aString action:aSelector keyEquivalent:keyEquiv]; [self _overrideClassOfMenuItem:newItem]; return newItem; } -- (void)insertItem:(NSMenuItem*)aNewItem atIndex:(NSInteger)aIndex { - [self _overrideClassOfMenuItem:aNewItem]; - [super insertItem:aNewItem atIndex:aIndex]; +- (void)insertItem:(NSMenuItem*)newItem atIndex:(NSInteger)index { + [self _overrideClassOfMenuItem:newItem]; + [super insertItem:newItem atIndex:index]; } - (NSMenuItem*)insertItemWithTitle:(NSString*)aString action:(SEL)aSelector - keyEquivalent:(NSString*)aKeyEquiv - atIndex:(NSInteger)aIndex { + keyEquivalent:(NSString*)keyEquiv + atIndex:(NSInteger)index { NSMenuItem* newItem = [super insertItemWithTitle:aString action:aSelector - keyEquivalent:aKeyEquiv - atIndex:aIndex]; + keyEquivalent:keyEquiv + atIndex:index]; [self _overrideClassOfMenuItem:newItem]; return newItem; } -- (void)_overrideClassOfMenuItem:(NSMenuItem*)aMenuItem { - if ([aMenuItem class] == [NSMenuItem class]) { - object_setClass(aMenuItem, [GeckoServicesNSMenuItem class]); +- (void)_overrideClassOfMenuItem:(NSMenuItem*)menuItem { + if ([menuItem class] == [NSMenuItem class]) { + object_setClass(menuItem, [GeckoServicesNSMenuItem class]); } } diff --git a/widget/cocoa/nsMenuBaseX.h b/widget/cocoa/nsMenuBaseX.h index b09e813c8299..ee8798ecc163 100644 --- a/widget/cocoa/nsMenuBaseX.h +++ b/widget/cocoa/nsMenuBaseX.h @@ -28,6 +28,7 @@ class nsMenuObjectX { virtual ~nsMenuObjectX() {} virtual nsMenuObjectTypeX MenuObjectType() = 0; virtual void* NativeData() = 0; + nsIContent* Content() { return mContent; } /** * Called when an icon of a menu item somewhere in this menu has updated. @@ -35,6 +36,9 @@ class nsMenuObjectX { * parent. */ virtual void IconUpdated() {} + + protected: + nsCOMPtr mContent; }; // diff --git a/widget/cocoa/nsMenuGroupOwnerX.h b/widget/cocoa/nsMenuGroupOwnerX.h index 6a87b7d09bc6..627833e0dff3 100644 --- a/widget/cocoa/nsMenuGroupOwnerX.h +++ b/widget/cocoa/nsMenuGroupOwnerX.h @@ -40,8 +40,6 @@ class nsMenuGroupOwnerX : public nsMenuObjectX, public nsIMutationObserver { nsChangeObserver* LookupContentChangeObserver(nsIContent* aContent); - RefPtr mContent; - uint32_t mCurrentCommandID; // unique command id (per menu-bar) to // give to next item that asks diff --git a/widget/cocoa/nsMenuItemIconX.h b/widget/cocoa/nsMenuItemIconX.h index d561173a7794..ea0c0a225be6 100644 --- a/widget/cocoa/nsMenuItemIconX.h +++ b/widget/cocoa/nsMenuItemIconX.h @@ -21,11 +21,15 @@ class nsMenuObjectX; #import -class nsMenuItemIconX final : public mozilla::widget::IconLoader::Listener { +class nsMenuItemIconX : public mozilla::widget::IconLoader::Listener { public: nsMenuItemIconX(nsMenuObjectX* aMenuItem, nsIContent* aContent, NSMenuItem* aNativeMenuItem); - ~nsMenuItemIconX(); + + NS_INLINE_DECL_REFCOUNTING(nsMenuItemIconX) + + private: + virtual ~nsMenuItemIconX(); public: // SetupIcon succeeds if it was able to set up the icon, or if there should @@ -35,6 +39,12 @@ class nsMenuItemIconX final : public mozilla::widget::IconLoader::Listener { // GetIconURI fails if the item should not have any icon. nsresult GetIconURI(nsIURI** aIconURI); + // Unless we take precautions, we may outlive the object that created us + // (mMenuObject, which owns our native menu item (mNativeMenuItem)). + // Destroy() should be called from mMenuObject's destructor to prevent + // this from happening. See bug 499600. + void Destroy(); + // Implements this method for mozilla::widget::IconLoader::Listener. // Called once the icon load is complete. nsresult OnComplete(imgIContainer* aImage) override; diff --git a/widget/cocoa/nsMenuItemIconX.mm b/widget/cocoa/nsMenuItemIconX.mm index 6d71f0882f75..3bf09002b540 100644 --- a/widget/cocoa/nsMenuItemIconX.mm +++ b/widget/cocoa/nsMenuItemIconX.mm @@ -51,10 +51,20 @@ nsMenuItemIconX::nsMenuItemIconX(nsMenuObjectX* aMenuItem, nsIContent* aContent, } nsMenuItemIconX::~nsMenuItemIconX() { + Destroy(); + MOZ_COUNT_DTOR(nsMenuItemIconX); +} + +// Called from mMenuObjectX's destructor, to prevent us from outliving it +// (as might otherwise happen if calls to our imgINotificationObserver methods +// are still outstanding). mMenuObjectX owns our mNativeMenuItem. +void nsMenuItemIconX::Destroy() { if (mIconLoader) { mIconLoader->Destroy(); + mIconLoader = nullptr; } - MOZ_COUNT_DTOR(nsMenuItemIconX); + mMenuObject = nullptr; + mNativeMenuItem = nil; } nsresult nsMenuItemIconX::SetupIcon() { @@ -71,7 +81,7 @@ nsresult nsMenuItemIconX::SetupIcon() { if (NS_FAILED(rv)) { // There is no icon for this menu item. An icon might have been set // earlier. Clear it. - mNativeMenuItem.image = nil; + [mNativeMenuItem setImage:nil]; return NS_OK; } @@ -82,7 +92,7 @@ nsresult nsMenuItemIconX::SetupIcon() { if (!mSetIcon) { // Load placeholder icon. NSSize iconSize = NSMakeSize(kIconSize, kIconSize); - mNativeMenuItem.image = [MOZIconHelper placeholderIconWithSize:iconSize]; + [mNativeMenuItem setImage:[MOZIconHelper placeholderIconWithSize:iconSize]]; } rv = mIconLoader->LoadIcon(iconURI, mContent); @@ -90,7 +100,7 @@ nsresult nsMenuItemIconX::SetupIcon() { // There is no icon for this menu item, as an error occurred while loading it. // An icon might have been set earlier or the place holder icon may have // been set. Clear it. - mNativeMenuItem.image = nil; + [mNativeMenuItem setImage:nil]; } mSetIcon = true; @@ -201,7 +211,7 @@ nsresult nsMenuItemIconX::OnComplete(imgIContainer* aImage) { withSize:NSMakeSize(kIconSize, kIconSize) subrect:mImageRegionRect scaleFactor:0.0f]; - mNativeMenuItem.image = image; + [mNativeMenuItem setImage:image]; if (mMenuObject) { mMenuObject->IconUpdated(); } diff --git a/widget/cocoa/nsMenuItemX.h b/widget/cocoa/nsMenuItemX.h index 14f6162c02fc..ca102a426cfe 100644 --- a/widget/cocoa/nsMenuItemX.h +++ b/widget/cocoa/nsMenuItemX.h @@ -41,10 +41,9 @@ enum EMenuItemType { // 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. -class nsMenuItemX final : public nsMenuObjectX, public nsChangeObserver { +class nsMenuItemX : public nsMenuObjectX, public nsChangeObserver { public: - nsMenuItemX(nsMenuX* aParent, const nsString& aLabel, EMenuItemType aItemType, - nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aNode); + nsMenuItemX(); virtual ~nsMenuItemX(); NS_DECL_CHANGEOBSERVER @@ -54,29 +53,30 @@ class nsMenuItemX final : public nsMenuObjectX, public nsChangeObserver { nsMenuObjectTypeX MenuObjectType() override { return eMenuItemObjectType; } // nsMenuItemX + nsresult Create(nsMenuX* aParent, const nsString& aLabel, + EMenuItemType aItemType, nsMenuGroupOwnerX* aMenuGroupOwner, + nsIContent* aNode); nsresult SetChecked(bool aIsChecked); EMenuItemType GetMenuItemType(); void DoCommand(); nsresult DispatchDOMEvent(const nsString& eventName, bool* preventDefaultCalled); void SetupIcon(); - nsIContent* Content() { return mContent; } protected: - void UncheckRadioSiblings(nsIContent* aCheckedElement); + void UncheckRadioSiblings(nsIContent* inCheckedElement); void SetKeyEquiv(); - nsCOMPtr mContent; // XUL or - EMenuItemType mType; // nsMenuItemX objects should always have a valid native menu item. - NSMenuItem* mNativeMenuItem = nil; // [strong] - nsMenuX* mMenuParent = nullptr; // [weak] - nsMenuGroupOwnerX* mMenuGroupOwner = nullptr; // [weak] + NSMenuItem* mNativeMenuItem; // [strong] + nsMenuX* mMenuParent; // [weak] + nsMenuGroupOwnerX* mMenuGroupOwner; // [weak] RefPtr mCommandElement; - mozilla::UniquePtr mIcon; // always non-null - bool mIsChecked = false; + // The icon object should never outlive its creating nsMenuItemX object. + RefPtr mIcon; + bool mIsChecked; }; #endif // nsMenuItemX_h_ diff --git a/widget/cocoa/nsMenuItemX.mm b/widget/cocoa/nsMenuItemX.mm index 26e7e0df9a8f..f99b394c7079 100644 --- a/widget/cocoa/nsMenuItemX.mm +++ b/widget/cocoa/nsMenuItemX.mm @@ -26,13 +26,49 @@ using namespace mozilla; using mozilla::dom::Event; using mozilla::dom::CallerType; -nsMenuItemX::nsMenuItemX(nsMenuX* aParent, const nsString& aLabel, EMenuItemType aItemType, - nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aNode) - : mContent(aNode), mType(aItemType), mMenuParent(aParent), mMenuGroupOwner(aMenuGroupOwner) { - NS_OBJC_BEGIN_TRY_ABORT_BLOCK; +nsMenuItemX::nsMenuItemX() { + mType = eRegularMenuItemType; + mNativeMenuItem = nil; + mMenuParent = nullptr; + mMenuGroupOwner = nullptr; + mIsChecked = false; MOZ_COUNT_CTOR(nsMenuItemX); +} +nsMenuItemX::~nsMenuItemX() { + NS_OBJC_BEGIN_TRY_ABORT_BLOCK; + + // Prevent the icon object from outliving us. + if (mIcon) { + mIcon->Destroy(); + } + + // autorelease the native menu item so that anything else happening to this + // object happens before the native menu item actually dies + [mNativeMenuItem autorelease]; + + if (mContent) { + mMenuGroupOwner->UnregisterForContentChanges(mContent); + } + if (mCommandElement) { + mMenuGroupOwner->UnregisterForContentChanges(mCommandElement); + } + + MOZ_COUNT_DTOR(nsMenuItemX); + + NS_OBJC_END_TRY_ABORT_BLOCK; +} + +nsresult nsMenuItemX::Create(nsMenuX* aParent, const nsString& aLabel, EMenuItemType aItemType, + nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aNode) { + NS_OBJC_BEGIN_TRY_ABORT_BLOCK; + + mType = aItemType; + mMenuParent = aParent; + mContent = aNode; + + mMenuGroupOwner = aMenuGroupOwner; NS_ASSERTION(mMenuGroupOwner, "No menu owner given, must have one!"); mMenuGroupOwner->RegisterForContentChanges(mContent, this); @@ -79,7 +115,7 @@ nsMenuItemX::nsMenuItemX(nsMenuX* aParent, const nsString& aLabel, EMenuItemType action:nil keyEquivalent:@""]; - mNativeMenuItem.enabled = isEnabled; + [mNativeMenuItem setEnabled:(BOOL)isEnabled]; SetChecked(mContent->IsElement() && mContent->AsElement()->AttrValueIs(kNameSpaceID_None, nsGkAtoms::checked, @@ -87,26 +123,12 @@ nsMenuItemX::nsMenuItemX(nsMenuX* aParent, const nsString& aLabel, EMenuItemType SetKeyEquiv(); } - mIcon = MakeUnique(this, mContent, mNativeMenuItem); - - NS_OBJC_END_TRY_ABORT_BLOCK; -} - -nsMenuItemX::~nsMenuItemX() { - NS_OBJC_BEGIN_TRY_ABORT_BLOCK; - - // autorelease the native menu item so that anything else happening to this - // object happens before the native menu item actually dies - [mNativeMenuItem autorelease]; - - if (mContent) { - mMenuGroupOwner->UnregisterForContentChanges(mContent); - } - if (mCommandElement) { - mMenuGroupOwner->UnregisterForContentChanges(mCommandElement); + mIcon = new nsMenuItemIconX(this, mContent, mNativeMenuItem); + if (!mIcon) { + return NS_ERROR_OUT_OF_MEMORY; } - MOZ_COUNT_DTOR(nsMenuItemX); + return NS_OK; NS_OBJC_END_TRY_ABORT_BLOCK; } @@ -122,7 +144,11 @@ nsresult nsMenuItemX::SetChecked(bool aIsChecked) { mIsChecked ? u"true"_ns : u"false"_ns, true); // update native menu item - mNativeMenuItem.state = mIsChecked ? NSOnState : NSOffState; + if (mIsChecked) { + [mNativeMenuItem setState:NSOnState]; + } else { + [mNativeMenuItem setState:NSOffState]; + } return NS_OK; @@ -179,16 +205,16 @@ nsresult nsMenuItemX::DispatchDOMEvent(const nsString& eventName, bool* preventD // Walk the sibling list looking for nodes with the same name and // uncheck them all. -void nsMenuItemX::UncheckRadioSiblings(nsIContent* aCheckedContent) { +void nsMenuItemX::UncheckRadioSiblings(nsIContent* inCheckedContent) { nsAutoString myGroupName; - if (aCheckedContent->IsElement()) { - aCheckedContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::name, myGroupName); + if (inCheckedContent->IsElement()) { + inCheckedContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::name, myGroupName); } if (!myGroupName.Length()) { // no groupname, nothing to do return; } - nsCOMPtr parent = aCheckedContent->GetParent(); + nsCOMPtr parent = inCheckedContent->GetParent(); if (!parent) { return; } @@ -196,7 +222,7 @@ void nsMenuItemX::UncheckRadioSiblings(nsIContent* aCheckedContent) { // loop over siblings for (nsIContent* sibling = parent->GetFirstChild(); sibling; sibling = sibling->GetNextSibling()) { - if (sibling != aCheckedContent && sibling->IsElement()) { // skip this node + if (sibling != inCheckedContent && sibling->IsElement()) { // skip this node // if the current sibling is in the same group, clear it if (sibling->AsElement()->AttrValueIs(kNameSpaceID_None, nsGkAtoms::name, myGroupName, eCaseMatters)) { @@ -237,14 +263,14 @@ void nsMenuItemX::SetKeyEquiv() { uint8_t modifiers = nsMenuUtilsX::GeckoModifiersForNodeAttribute(modifiersStr); unsigned int macModifiers = nsMenuUtilsX::MacModifiersForGeckoModifiers(modifiers); - mNativeMenuItem.keyEquivalentModifierMask = macModifiers; + [mNativeMenuItem setKeyEquivalentModifierMask:macModifiers]; NSString* keyEquivalent = [[NSString stringWithCharacters:(unichar*)keyChar.get() length:keyChar.Length()] lowercaseString]; if ([keyEquivalent isEqualToString:@" "]) { - mNativeMenuItem.keyEquivalent = @""; + [mNativeMenuItem setKeyEquivalent:@""]; } else { - mNativeMenuItem.keyEquivalent = keyEquivalent; + [mNativeMenuItem setKeyEquivalent:keyEquivalent]; } return; @@ -252,7 +278,7 @@ void nsMenuItemX::SetKeyEquiv() { } // if the key was removed, clear the key - mNativeMenuItem.keyEquivalent = @""; + [mNativeMenuItem setKeyEquivalent:@""]; NS_OBJC_END_TRY_ABORT_BLOCK; } @@ -287,8 +313,12 @@ void nsMenuItemX::ObserveAttributeChanged(dom::Document* aDocument, nsIContent* } else if (aAttribute == nsGkAtoms::image) { SetupIcon(); } else if (aAttribute == nsGkAtoms::disabled) { - mNativeMenuItem.enabled = !aContent->AsElement()->AttrValueIs( - kNameSpaceID_None, nsGkAtoms::disabled, nsGkAtoms::_true, eCaseMatters); + if (aContent->AsElement()->AttrValueIs(kNameSpaceID_None, nsGkAtoms::disabled, + nsGkAtoms::_true, eCaseMatters)) { + [mNativeMenuItem setEnabled:NO]; + } else { + [mNativeMenuItem setEnabled:YES]; + } } } else if (aContent == mCommandElement) { // the only thing that really matters when the menu isn't showing is the @@ -309,8 +339,12 @@ void nsMenuItemX::ObserveAttributeChanged(dom::Document* aDocument, nsIContent* } } // now we sync our native menu item with the command DOM node - mNativeMenuItem.enabled = !aContent->AsElement()->AttrValueIs( - kNameSpaceID_None, nsGkAtoms::disabled, nsGkAtoms::_true, eCaseMatters); + if (aContent->AsElement()->AttrValueIs(kNameSpaceID_None, nsGkAtoms::disabled, + nsGkAtoms::_true, eCaseMatters)) { + [mNativeMenuItem setEnabled:NO]; + } else { + [mNativeMenuItem setEnabled:YES]; + } } } @@ -343,4 +377,8 @@ void nsMenuItemX::ObserveContentInserted(dom::Document* aDocument, nsIContent* a } } -void nsMenuItemX::SetupIcon() { mIcon->SetupIcon(); } +void nsMenuItemX::SetupIcon() { + if (mIcon) { + mIcon->SetupIcon(); + } +} diff --git a/widget/cocoa/nsMenuUtilsX.h b/widget/cocoa/nsMenuUtilsX.h index c4f8fc480297..deee16718260 100644 --- a/widget/cocoa/nsMenuUtilsX.h +++ b/widget/cocoa/nsMenuUtilsX.h @@ -23,7 +23,7 @@ uint8_t GeckoModifiersForNodeAttribute(const nsString& modifiersAttribute); unsigned int MacModifiersForGeckoModifiers(uint8_t geckoModifiers); nsMenuBarX* GetHiddenWindowMenuBar(); // returned object is not retained NSMenuItem* GetStandardEditMenuItem(); // returned object is not retained -bool NodeIsHiddenOrCollapsed(nsIContent* aContent); +bool NodeIsHiddenOrCollapsed(nsIContent* inContent); int CalculateNativeInsertionPoint(nsMenuObjectX* aParent, nsMenuObjectX* aChild); // Find the menu item by following the path aLocationString from aRootMenu. diff --git a/widget/cocoa/nsMenuUtilsX.mm b/widget/cocoa/nsMenuUtilsX.mm index c59c808c9b60..28367cc0dad2 100644 --- a/widget/cocoa/nsMenuUtilsX.mm +++ b/widget/cocoa/nsMenuUtilsX.mm @@ -119,7 +119,7 @@ NSMenuItem* nsMenuUtilsX::GetStandardEditMenuItem() { action:nil keyEquivalent:@""] autorelease]; NSMenu* standardEditMenu = [[NSMenu alloc] initWithTitle:@"Edit"]; - standardEditMenuItem.submenu = standardEditMenu; + [standardEditMenuItem setSubmenu:standardEditMenu]; [standardEditMenu release]; // Add Undo @@ -179,12 +179,12 @@ NSMenuItem* nsMenuUtilsX::GetStandardEditMenuItem() { NS_OBJC_END_TRY_ABORT_BLOCK; } -bool nsMenuUtilsX::NodeIsHiddenOrCollapsed(nsIContent* aContent) { - return aContent->IsElement() && - (aContent->AsElement()->AttrValueIs(kNameSpaceID_None, nsGkAtoms::hidden, nsGkAtoms::_true, - eCaseMatters) || - aContent->AsElement()->AttrValueIs(kNameSpaceID_None, nsGkAtoms::collapsed, - nsGkAtoms::_true, eCaseMatters)); +bool nsMenuUtilsX::NodeIsHiddenOrCollapsed(nsIContent* inContent) { + return inContent->IsElement() && + (inContent->AsElement()->AttrValueIs(kNameSpaceID_None, nsGkAtoms::hidden, + nsGkAtoms::_true, eCaseMatters) || + inContent->AsElement()->AttrValueIs(kNameSpaceID_None, nsGkAtoms::collapsed, + nsGkAtoms::_true, eCaseMatters)); } // Determines how many items are visible among the siblings in a menu that are @@ -200,7 +200,7 @@ int nsMenuUtilsX::CalculateNativeInsertionPoint(nsMenuObjectX* aParent, nsMenuOb if (currMenu == aChild) { return insertionPoint; // we found ourselves, break out } - if (currMenu && currMenu->NativeMenuItem().menu) { + if (currMenu && [currMenu->NativeMenuItem() menu]) { insertionPoint++; } } @@ -226,7 +226,7 @@ int nsMenuUtilsX::CalculateNativeInsertionPoint(nsMenuObjectX* aParent, nsMenuOb } else { nativeItem = (NSMenuItem*)(currItem->NativeData()); } - if (nativeItem.menu) { + if ([nativeItem menu]) { insertionPoint++; } } @@ -236,20 +236,20 @@ int nsMenuUtilsX::CalculateNativeInsertionPoint(nsMenuObjectX* aParent, nsMenuOb NSMenuItem* nsMenuUtilsX::NativeMenuItemWithLocation(NSMenu* aRootMenu, NSString* aLocationString, bool aIsMenuBar) { - NSArray* indexes = [aLocationString componentsSeparatedByString:@"|"]; - unsigned int pathLength = indexes.count; + NSArray* indexes = [aLocationString componentsSeparatedByString:@"|"]; + unsigned int pathLength = [indexes count]; if (pathLength == 0) { return nil; } NSMenu* currentSubmenu = aRootMenu; for (unsigned int depth = 0; depth < pathLength; depth++) { - NSInteger targetIndex = [indexes objectAtIndex:depth].integerValue; + NSInteger targetIndex = [[indexes objectAtIndex:depth] integerValue]; if (aIsMenuBar && depth == 0) { // We remove the application menu from consideration for the top-level menu. targetIndex++; } - int itemCount = currentSubmenu.numberOfItems; + int itemCount = [currentSubmenu numberOfItems]; if (targetIndex < itemCount) { NSMenuItem* menuItem = [currentSubmenu itemAtIndex:targetIndex]; // if this is the last index just return the menu item @@ -257,8 +257,8 @@ NSMenuItem* nsMenuUtilsX::NativeMenuItemWithLocation(NSMenu* aRootMenu, NSString return menuItem; } // if this is not the last index find the submenu and keep going - if (menuItem.hasSubmenu) { - currentSubmenu = menuItem.submenu; + if ([menuItem hasSubmenu]) { + currentSubmenu = [menuItem submenu]; } else { return nil; } diff --git a/widget/cocoa/nsMenuX.h b/widget/cocoa/nsMenuX.h index aeb0f7906aef..494f00d08083 100644 --- a/widget/cocoa/nsMenuX.h +++ b/widget/cocoa/nsMenuX.h @@ -32,9 +32,9 @@ class nsIWidget; // 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. -class nsMenuX final : public nsMenuObjectX, public nsChangeObserver { +class nsMenuX : public nsMenuObjectX, public nsChangeObserver { public: - nsMenuX(nsMenuObjectX* aParent, nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aContent); + nsMenuX(); virtual ~nsMenuX(); // If > 0, the OS is indexing all the app's menus (triggered by opening @@ -51,21 +51,15 @@ class nsMenuX final : public nsMenuObjectX, public nsChangeObserver { // nsMenuX nsresult Create(nsMenuObjectX* aParent, nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aNode); - - // The returned object is an nsMenuX or an nsMenuItemX object - nsMenuObjectX* GetItemAt(uint32_t aPos); uint32_t GetItemCount(); - - // The returned object is an nsMenuX or an nsMenuItemX object - nsMenuObjectX* GetVisibleItemAt(uint32_t aPos); + nsMenuObjectX* GetItemAt(uint32_t aPos); nsresult GetVisibleItemCount(uint32_t& aCount); - + nsMenuObjectX* GetVisibleItemAt(uint32_t aPos); nsEventStatus MenuOpened(); void MenuClosed(); void SetRebuild(bool aMenuEvent); NSMenuItem* NativeMenuItem(); nsresult SetupIcon(); - nsIContent* Content() { return mContent; } static bool IsXULHelpMenu(nsIContent* aMenuContent); @@ -74,35 +68,31 @@ class nsMenuX final : public nsMenuObjectX, public nsChangeObserver { nsresult RemoveAll(); nsresult SetEnabled(bool aIsEnabled); nsresult GetEnabled(bool* aIsEnabled); - already_AddRefed GetMenuPopupContent(); + void GetMenuPopupContent(nsIContent** aResult); bool OnOpen(); bool OnClose(); - void AddMenuItem(mozilla::UniquePtr&& aMenuItem); - void AddMenu(mozilla::UniquePtr&& aMenu); - void LoadMenuItem(nsIContent* aMenuItemContent); - void LoadSubMenu(nsIContent* aMenuContent); - GeckoNSMenu* CreateMenuWithGeckoString(nsString& aMenuTitle); + nsresult AddMenuItem(nsMenuItemX* aMenuItem); + nsMenuX* AddMenu(mozilla::UniquePtr aMenu); + void LoadMenuItem(nsIContent* inMenuItemContent); + void LoadSubMenu(nsIContent* inMenuContent); + GeckoNSMenu* CreateMenuWithGeckoString(nsString& menuTitle); - nsCOMPtr mContent; // XUL or - - // Contains nsMenuX and nsMenuItemX objects nsTArray> mMenuObjectsArray; - nsString mLabel; - uint32_t mVisibleItemsCount = 0; // cache - nsMenuObjectX* mParent = nullptr; // [weak] - nsMenuGroupOwnerX* mMenuGroupOwner = nullptr; // [weak] - mozilla::UniquePtr mIcon; - GeckoNSMenu* mNativeMenu = nil; // [strong] - MenuDelegate* mMenuDelegate = nil; // [strong] + uint32_t mVisibleItemsCount; // cache + nsMenuObjectX* mParent; // [weak] + nsMenuGroupOwnerX* mMenuGroupOwner; // [weak] + // The icon object should never outlive its creating nsMenuX object. + RefPtr mIcon; // [strong] + GeckoNSMenu* mNativeMenu; // [strong] + MenuDelegate* mMenuDelegate; // [strong] // nsMenuX objects should always have a valid native menu item. - NSMenuItem* mNativeMenuItem = nil; // [strong] - bool mIsEnabled = true; - bool mDidFirePopupHiding = false; - bool mDidFirePopupHidden = false; - bool mNeedsRebuild = true; - bool mConstructed = false; - bool mVisible = true; + NSMenuItem* mNativeMenuItem; // [strong] + bool mIsEnabled; + bool mDestroyHandlerCalled; + bool mNeedsRebuild; + bool mConstructed; + bool mVisible; }; #endif // nsMenuX_h_ diff --git a/widget/cocoa/nsMenuX.mm b/widget/cocoa/nsMenuX.mm index 53eba6879a34..b81108acc4d0 100644 --- a/widget/cocoa/nsMenuX.mm +++ b/widget/cocoa/nsMenuX.mm @@ -77,39 +77,39 @@ int32_t nsMenuX::sIndexingMenuLevel = 0; @end -// TODO: It is unclear whether this is still needed. -static void SwizzleDynamicIndexingMethods() { - if (gMenuMethodsSwizzled) { - return; - } - - nsToolkit::SwizzleMethods([NSMenu class], @selector(_addItem:toTable:), - @selector(nsMenuX_NSMenu_addItem:toTable:), true); - nsToolkit::SwizzleMethods([NSMenu class], @selector(_removeItem:fromTable:), - @selector(nsMenuX_NSMenu_removeItem:fromTable:), true); - // On SnowLeopard the Shortcut framework (which contains the - // SCTGRLIndex class) is loaded on demand, whenever the user first opens - // a menu (which normally hasn't happened yet). So we need to load it - // here explicitly. - dlopen("/System/Library/PrivateFrameworks/Shortcut.framework/Shortcut", RTLD_LAZY); - Class SCTGRLIndexClass = ::NSClassFromString(@"SCTGRLIndex"); - nsToolkit::SwizzleMethods(SCTGRLIndexClass, @selector(indexMenuBarDynamically), - @selector(nsMenuX_SCTGRLIndex_indexMenuBarDynamically)); - - gMenuMethodsSwizzled = true; -} - // // nsMenuX // -nsMenuX::nsMenuX(nsMenuObjectX* aParent, nsMenuGroupOwnerX* aMenuGroupOwner, nsIContent* aContent) - : mContent(aContent), mParent(aParent), mMenuGroupOwner(aMenuGroupOwner) { +nsMenuX::nsMenuX() + : mVisibleItemsCount(0), + mParent(nullptr), + mMenuGroupOwner(nullptr), + mNativeMenu(nil), + mNativeMenuItem(nil), + mIsEnabled(true), + mDestroyHandlerCalled(false), + mNeedsRebuild(true), + mConstructed(false), + mVisible(true) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; - MOZ_COUNT_CTOR(nsMenuX); + if (!gMenuMethodsSwizzled) { + nsToolkit::SwizzleMethods([NSMenu class], @selector(_addItem:toTable:), + @selector(nsMenuX_NSMenu_addItem:toTable:), true); + nsToolkit::SwizzleMethods([NSMenu class], @selector(_removeItem:fromTable:), + @selector(nsMenuX_NSMenu_removeItem:fromTable:), true); + // On SnowLeopard the Shortcut framework (which contains the + // SCTGRLIndex class) is loaded on demand, whenever the user first opens + // a menu (which normally hasn't happened yet). So we need to load it + // here explicitly. + dlopen("/System/Library/PrivateFrameworks/Shortcut.framework/Shortcut", RTLD_LAZY); + Class SCTGRLIndexClass = ::NSClassFromString(@"SCTGRLIndex"); + nsToolkit::SwizzleMethods(SCTGRLIndexClass, @selector(indexMenuBarDynamically), + @selector(nsMenuX_SCTGRLIndex_indexMenuBarDynamically)); - SwizzleDynamicIndexingMethods(); + gMenuMethodsSwizzled = true; + } mMenuDelegate = [[MenuDelegate alloc] initWithGeckoMenu:this]; @@ -117,15 +117,54 @@ nsMenuX::nsMenuX(nsMenuObjectX* aParent, nsMenuGroupOwnerX* aMenuGroupOwner, nsI nsMenuBarX::sNativeEventTarget = [[NativeMenuItemTarget alloc] init]; } + MOZ_COUNT_CTOR(nsMenuX); + + NS_OBJC_END_TRY_ABORT_BLOCK; +} + +nsMenuX::~nsMenuX() { + NS_OBJC_BEGIN_TRY_ABORT_BLOCK; + + // Prevent the icon object from outliving us. + if (mIcon) { + mIcon->Destroy(); + } + + RemoveAll(); + + [mNativeMenu setDelegate:nil]; + [mNativeMenu release]; + [mMenuDelegate release]; + // autorelease the native menu item so that anything else happening to this + // object happens before the native menu item actually dies + [mNativeMenuItem autorelease]; + + // alert the change notifier we don't care no more + if (mContent) { + mMenuGroupOwner->UnregisterForContentChanges(mContent); + } + + MOZ_COUNT_DTOR(nsMenuX); + + NS_OBJC_END_TRY_ABORT_BLOCK; +} + +nsresult nsMenuX::Create(nsMenuObjectX* aParent, nsMenuGroupOwnerX* aMenuGroupOwner, + nsIContent* aContent) { + NS_OBJC_BEGIN_TRY_ABORT_BLOCK; + + mContent = aContent; if (mContent->IsElement()) { mContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::label, mLabel); } mNativeMenu = CreateMenuWithGeckoString(mLabel); // register this menu to be notified when changes are made to our content object + mMenuGroupOwner = aMenuGroupOwner; // weak ref NS_ASSERTION(mMenuGroupOwner, "No menu owner given, must have one"); mMenuGroupOwner->RegisterForContentChanges(mContent, this); + mParent = aParent; // our parent could be either a menu bar (if we're toplevel) or a menu (if we're a submenu) #ifdef DEBUG @@ -159,66 +198,47 @@ nsMenuX::nsMenuX(nsMenuObjectX* aParent, nsMenuGroupOwnerX* aMenuGroupOwner, nsI // menu gets selected, which is bad. MenuConstruct(); - mIcon = MakeUnique(this, mContent, mNativeMenuItem); + mIcon = new nsMenuItemIconX(this, mContent, mNativeMenuItem); + + return NS_OK; NS_OBJC_END_TRY_ABORT_BLOCK; } -nsMenuX::~nsMenuX() { +nsresult nsMenuX::AddMenuItem(nsMenuItemX* aMenuItem) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; - RemoveAll(); - - mNativeMenu.delegate = nil; - [mNativeMenu release]; - [mMenuDelegate release]; - // autorelease the native menu item so that anything else happening to this - // object happens before the native menu item actually dies - [mNativeMenuItem autorelease]; - - // alert the change notifier we don't care no more - if (mContent) { - mMenuGroupOwner->UnregisterForContentChanges(mContent); + if (!aMenuItem) { + return NS_ERROR_INVALID_ARG; } - MOZ_COUNT_DTOR(nsMenuX); - - NS_OBJC_END_TRY_ABORT_BLOCK; -} - -void nsMenuX::AddMenuItem(UniquePtr&& aMenuItem) { - NS_OBJC_BEGIN_TRY_ABORT_BLOCK; - - nsMenuItemX* menuItem = aMenuItem.get(); - mMenuObjectsArray.AppendElement(std::move(aMenuItem)); - - if (nsMenuUtilsX::NodeIsHiddenOrCollapsed(menuItem->Content())) { - return; + mMenuObjectsArray.AppendElement(aMenuItem); + if (nsMenuUtilsX::NodeIsHiddenOrCollapsed(aMenuItem->Content())) { + return NS_OK; } - ++mVisibleItemsCount; - NSMenuItem* newNativeMenuItem = (NSMenuItem*)menuItem->NativeData(); + NSMenuItem* newNativeMenuItem = (NSMenuItem*)aMenuItem->NativeData(); // add the menu item to this menu [mNativeMenu addItem:newNativeMenuItem]; // set up target/action - newNativeMenuItem.target = nsMenuBarX::sNativeEventTarget; - newNativeMenuItem.action = @selector(menuItemHit:); + [newNativeMenuItem setTarget:nsMenuBarX::sNativeEventTarget]; + [newNativeMenuItem setAction:@selector(menuItemHit:)]; // set its command. we get the unique command id from the menubar - newNativeMenuItem.tag = mMenuGroupOwner->RegisterForCommand(menuItem); + [newNativeMenuItem setTag:mMenuGroupOwner->RegisterForCommand(aMenuItem)]; MenuItemInfo* info = [[MenuItemInfo alloc] initWithMenuGroupOwner:mMenuGroupOwner]; - newNativeMenuItem.representedObject = info; + [newNativeMenuItem setRepresentedObject:info]; [info release]; - menuItem->SetupIcon(); + return NS_OK; NS_OBJC_END_TRY_ABORT_BLOCK; } -void nsMenuX::AddMenu(UniquePtr&& aMenu) { +nsMenuX* nsMenuX::AddMenu(UniquePtr aMenu) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; // aMenu transfers ownership to mMenuObjectsArray and becomes nullptr, so @@ -227,7 +247,7 @@ void nsMenuX::AddMenu(UniquePtr&& aMenu) { mMenuObjectsArray.AppendElement(std::move(aMenu)); if (nsMenuUtilsX::NodeIsHiddenOrCollapsed(menu->Content())) { - return; + return menu; } ++mVisibleItemsCount; @@ -236,10 +256,10 @@ void nsMenuX::AddMenu(UniquePtr&& aMenu) { NSMenuItem* newNativeMenuItem = menu->NativeMenuItem(); if (newNativeMenuItem) { [mNativeMenu addItem:newNativeMenuItem]; - newNativeMenuItem.submenu = (NSMenu*)menu->NativeData(); + [newNativeMenuItem setSubmenu:(NSMenu*)menu->NativeData()]; } - menu->SetupIcon(); + return menu; NS_OBJC_END_TRY_ABORT_BLOCK; } @@ -276,15 +296,11 @@ nsMenuObjectX* nsMenuX::GetVisibleItemAt(uint32_t aPos) { } // Otherwise, traverse the array until we find the the item we're looking for. + nsMenuObjectX* item; uint32_t visibleNodeIndex = 0; for (uint32_t i = 0; i < count; i++) { - nsMenuObjectX* item = mMenuObjectsArray[i].get(); - MOZ_RELEASE_ASSERT(item->MenuObjectType() == eSubmenuObjectType || - item->MenuObjectType() == eMenuItemObjectType); - RefPtr content = item->MenuObjectType() == eSubmenuObjectType - ? static_cast(item)->Content() - : static_cast(item)->Content(); - if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(content)) { + item = mMenuObjectsArray[i].get(); + if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(item->Content())) { if (aPos == visibleNodeIndex) { // we found the visible node we're looking for, return it return item; @@ -301,12 +317,12 @@ nsresult nsMenuX::RemoveAll() { if (mNativeMenu) { // clear command id's - int itemCount = mNativeMenu.numberOfItems; + int itemCount = [mNativeMenu numberOfItems]; for (int i = 0; i < itemCount; i++) { mMenuGroupOwner->UnregisterCommand((uint32_t)[[mNativeMenu itemAtIndex:i] tag]); } // get rid of Cocoa menu items - for (int i = mNativeMenu.numberOfItems - 1; i >= 0; i--) { + for (int i = [mNativeMenu numberOfItems] - 1; i >= 0; i--) { [mNativeMenu removeItemAtIndex:i]; } } @@ -344,7 +360,8 @@ nsEventStatus nsMenuX::MenuOpened() { nsEventStatus status = nsEventStatus_eIgnore; WidgetMouseEvent event(true, eXULPopupShown, nullptr, WidgetMouseEvent::eReal); - nsCOMPtr popupContent = GetMenuPopupContent(); + nsCOMPtr popupContent; + GetMenuPopupContent(getter_AddRefs(popupContent)); nsIContent* dispatchTo = popupContent ? popupContent : mContent; EventDispatcher::Dispatch(dispatchTo, nullptr, &event, nullptr, &status); @@ -369,11 +386,12 @@ void nsMenuX::MenuClosed() { nsEventStatus status = nsEventStatus_eIgnore; WidgetMouseEvent event(true, eXULPopupHidden, nullptr, WidgetMouseEvent::eReal); - nsCOMPtr popupContent = GetMenuPopupContent(); + nsCOMPtr popupContent; + GetMenuPopupContent(getter_AddRefs(popupContent)); nsIContent* dispatchTo = popupContent ? popupContent : mContent; EventDispatcher::Dispatch(dispatchTo, nullptr, &event, nullptr, &status); - mDidFirePopupHidden = true; + mDestroyHandlerCalled = true; mConstructed = false; } } @@ -382,11 +400,15 @@ void nsMenuX::MenuConstruct() { mConstructed = false; gConstructingMenu = true; - mDidFirePopupHiding = false; - mDidFirePopupHidden = false; + // reset destroy handler flag so that we'll know to fire it next time this menu goes away. + mDestroyHandlerCalled = false; + + // printf("nsMenuX::MenuConstruct called for %s = %d \n", + // NS_LossyConvertUTF16toASCII(mLabel).get(), mNativeMenu); // Retrieve our menupopup. - nsCOMPtr menuPopup = GetMenuPopupContent(); + nsCOMPtr menuPopup; + GetMenuPopupContent(getter_AddRefs(menuPopup)); if (!menuPopup) { gConstructingMenu = false; return; @@ -404,6 +426,7 @@ void nsMenuX::MenuConstruct() { gConstructingMenu = false; mNeedsRebuild = false; + // printf("Done building, mMenuObjectsArray.Count() = %d \n", mMenuObjectsArray.Count()); } void nsMenuX::SetRebuild(bool aNeedsRebuild) { @@ -420,7 +443,7 @@ nsresult nsMenuX::SetEnabled(bool aIsEnabled) { if (aIsEnabled != mIsEnabled) { // we always want to rebuild when this changes mIsEnabled = aIsEnabled; - mNativeMenuItem.enabled = mIsEnabled; + [mNativeMenuItem setEnabled:(BOOL)mIsEnabled]; } return NS_OK; } @@ -431,17 +454,17 @@ nsresult nsMenuX::GetEnabled(bool* aIsEnabled) { return NS_OK; } -GeckoNSMenu* nsMenuX::CreateMenuWithGeckoString(nsString& aMenuTitle) { +GeckoNSMenu* nsMenuX::CreateMenuWithGeckoString(nsString& menuTitle) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; - NSString* title = [NSString stringWithCharacters:(UniChar*)aMenuTitle.get() - length:aMenuTitle.Length()]; + NSString* title = [NSString stringWithCharacters:(UniChar*)menuTitle.get() + length:menuTitle.Length()]; GeckoNSMenu* myMenu = [[GeckoNSMenu alloc] initWithTitle:title]; - myMenu.delegate = mMenuDelegate; + [myMenu setDelegate:mMenuDelegate]; // We don't want this menu to auto-enable menu items because then Cocoa // overrides our decisions and things get incorrectly enabled/disabled. - myMenu.autoenablesItems = NO; + [myMenu setAutoenablesItems:NO]; // we used to install Carbon event handlers here, but since NSMenu* doesn't // create its underlying MenuRef until just before display, we delay until @@ -453,23 +476,25 @@ GeckoNSMenu* nsMenuX::CreateMenuWithGeckoString(nsString& aMenuTitle) { NS_OBJC_END_TRY_ABORT_BLOCK; } -void nsMenuX::LoadMenuItem(nsIContent* aMenuItemContent) { - if (!aMenuItemContent) { +void nsMenuX::LoadMenuItem(nsIContent* inMenuItemContent) { + if (!inMenuItemContent) { return; } nsAutoString menuitemName; - if (aMenuItemContent->IsElement()) { - aMenuItemContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::label, menuitemName); + if (inMenuItemContent->IsElement()) { + inMenuItemContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::label, menuitemName); } + // printf("menuitem %s \n", NS_LossyConvertUTF16toASCII(menuitemName).get()); + EMenuItemType itemType = eRegularMenuItemType; - if (aMenuItemContent->IsXULElement(nsGkAtoms::menuseparator)) { + if (inMenuItemContent->IsXULElement(nsGkAtoms::menuseparator)) { itemType = eSeparatorMenuItemType; - } else if (aMenuItemContent->IsElement()) { + } else if (inMenuItemContent->IsElement()) { static Element::AttrValuesArray strings[] = {nsGkAtoms::checkbox, nsGkAtoms::radio, nullptr}; - switch (aMenuItemContent->AsElement()->FindAttrValueIn(kNameSpaceID_None, nsGkAtoms::type, - strings, eCaseMatters)) { + switch (inMenuItemContent->AsElement()->FindAttrValueIn(kNameSpaceID_None, nsGkAtoms::type, + strings, eCaseMatters)) { case 0: itemType = eCheckboxMenuItemType; break; @@ -479,12 +504,45 @@ void nsMenuX::LoadMenuItem(nsIContent* aMenuItemContent) { } } - AddMenuItem( - MakeUnique(this, menuitemName, itemType, mMenuGroupOwner, aMenuItemContent)); + // Create the item. + nsMenuItemX* menuItem = new nsMenuItemX(); + if (!menuItem) { + return; + } + + nsresult rv = menuItem->Create(this, menuitemName, itemType, mMenuGroupOwner, inMenuItemContent); + if (NS_FAILED(rv)) { + delete menuItem; + return; + } + + AddMenuItem(menuItem); + + // This needs to happen after the nsIMenuItem object is inserted into + // our item array in AddMenuItem() + menuItem->SetupIcon(); } -void nsMenuX::LoadSubMenu(nsIContent* aMenuContent) { - AddMenu(MakeUnique(this, mMenuGroupOwner, aMenuContent)); +void nsMenuX::LoadSubMenu(nsIContent* inMenuContent) { + auto menu = MakeUnique(); + if (!menu) { + return; + } + + nsresult rv = menu->Create(this, mMenuGroupOwner, inMenuContent); + if (NS_FAILED(rv)) { + return; + } + + // |menu|'s ownership is transfer to AddMenu but, if it is successfully + // added, we can access it via the returned raw pointer. + nsMenuX* menu_ptr = AddMenu(std::move(menu)); + + // This needs to happen after the nsIMenu object is inserted into + // our item array in AddMenu() + if (menu_ptr) { + menu_ptr->SetupIcon(); + } } // This menu is about to open. Returns TRUE if we should keep processing the event, @@ -493,7 +551,8 @@ bool nsMenuX::OnOpen() { nsEventStatus status = nsEventStatus_eIgnore; WidgetMouseEvent event(true, eXULPopupShowing, nullptr, WidgetMouseEvent::eReal); - nsCOMPtr popupContent = GetMenuPopupContent(); + nsCOMPtr popupContent; + GetMenuPopupContent(getter_AddRefs(popupContent)); nsresult rv = NS_OK; nsIContent* dispatchTo = popupContent ? popupContent : mContent; @@ -508,7 +567,7 @@ bool nsMenuX::OnOpen() { // Get new popup content first since it might have changed as a result of the // eXULPopupShowing event above. - popupContent = GetMenuPopupContent(); + GetMenuPopupContent(getter_AddRefs(popupContent)); if (!popupContent) { return true; } @@ -524,20 +583,21 @@ bool nsMenuX::OnOpen() { // Returns TRUE if we should keep processing the event, FALSE if the handler // wants to stop the closing of the menu. bool nsMenuX::OnClose() { - if (mDidFirePopupHiding || mDidFirePopupHidden) { + if (mDestroyHandlerCalled) { return true; } nsEventStatus status = nsEventStatus_eIgnore; WidgetMouseEvent event(true, eXULPopupHiding, nullptr, WidgetMouseEvent::eReal); - nsCOMPtr popupContent = GetMenuPopupContent(); + nsCOMPtr popupContent; + GetMenuPopupContent(getter_AddRefs(popupContent)); nsresult rv = NS_OK; nsIContent* dispatchTo = popupContent ? popupContent : mContent; rv = EventDispatcher::Dispatch(dispatchTo, nullptr, &event, nullptr, &status); - mDidFirePopupHiding = true; + mDestroyHandlerCalled = true; if (NS_FAILED(rv) || status == nsEventStatus_eConsumeNoDefault) { return false; @@ -549,22 +609,26 @@ bool nsMenuX::OnClose() { // Find the |menupopup| child in the |popup| representing this menu. It should be one // of a very few children so we won't be iterating over a bazillion menu items to find // it (so the strcmp won't kill us). -already_AddRefed nsMenuX::GetMenuPopupContent() { +void nsMenuX::GetMenuPopupContent(nsIContent** aResult) { + if (!aResult) { + return; + } + *aResult = nullptr; + // Check to see if we are a "menupopup" node (if we are a native menu). if (mContent->IsXULElement(nsGkAtoms::menupopup)) { - return do_AddRef(mContent); + NS_ADDREF(*aResult = mContent); + return; } // Otherwise check our child nodes. - for (RefPtr child = mContent->GetFirstChild(); child; - child = child->GetNextSibling()) { + for (nsIContent* child = mContent->GetFirstChild(); child; child = child->GetNextSibling()) { if (child->IsXULElement(nsGkAtoms::menupopup)) { - return child.forget(); + NS_ADDREF(*aResult = child); + return; } } - - return nullptr; } NSMenuItem* nsMenuX::NativeMenuItem() { return mNativeMenuItem; } @@ -609,7 +673,7 @@ void nsMenuX::ObserveAttributeChanged(dom::Document* aDocument, nsIContent* aCon // reuse the existing menu, to avoid rebuilding the root menu bar. NS_ASSERTION(mNativeMenu, "nsMenuX::AttributeChanged: invalid menu handle."); NSString* newCocoaLabelString = nsMenuUtilsX::GetTruncatedCocoaLabel(mLabel); - mNativeMenu.title = newCocoaLabelString; + [mNativeMenu setTitle:newCocoaLabelString]; } else if (parentType == eSubmenuObjectType) { static_cast(mParent)->SetRebuild(true); } else if (parentType == eStandaloneNativeMenuObjectType) { @@ -650,7 +714,7 @@ void nsMenuX::ObserveAttributeChanged(dom::Document* aDocument, nsIContent* aCon } NSMenu* parentMenu = (NSMenu*)mParent->NativeData(); [parentMenu insertItem:mNativeMenuItem atIndex:insertionIndex]; - mNativeMenuItem.submenu = mNativeMenu; + [mNativeMenuItem setSubmenu:mNativeMenu]; mVisible = true; } } @@ -680,7 +744,15 @@ void nsMenuX::ObserveContentInserted(dom::Document* aDocument, nsIContent* aCont SetRebuild(true); } -nsresult nsMenuX::SetupIcon() { return mIcon->SetupIcon(); } +nsresult nsMenuX::SetupIcon() { + // In addition to out-of-memory, menus that are children of the menu bar + // will not have mIcon set. + if (!mIcon) { + return NS_ERROR_OUT_OF_MEMORY; + } + + return mIcon->SetupIcon(); +} // // MenuDelegate Objective-C class, used to set up Carbon events diff --git a/widget/cocoa/nsStandaloneNativeMenu.h b/widget/cocoa/nsStandaloneNativeMenu.h index 57d5aae73e2b..76ec97622d38 100644 --- a/widget/cocoa/nsStandaloneNativeMenu.h +++ b/widget/cocoa/nsStandaloneNativeMenu.h @@ -27,7 +27,7 @@ class nsStandaloneNativeMenu : public nsMenuGroupOwnerX, } virtual void IconUpdated() override; - nsMenuX* GetMenuXObject() { return mMenu.get(); } + nsMenuX* GetMenuXObject() { return mMenu; } // If this menu is the menu of a system status bar item (NSStatusItem), // let the menu know about the status item so that it can propagate @@ -37,7 +37,7 @@ class nsStandaloneNativeMenu : public nsMenuGroupOwnerX, protected: virtual ~nsStandaloneNativeMenu(); - mozilla::UniquePtr mMenu; + nsMenuX* mMenu; NSStatusItem* mContainerStatusBarItem; }; diff --git a/widget/cocoa/nsStandaloneNativeMenu.mm b/widget/cocoa/nsStandaloneNativeMenu.mm index 67ff41c7d1ae..be1397002c8b 100644 --- a/widget/cocoa/nsStandaloneNativeMenu.mm +++ b/widget/cocoa/nsStandaloneNativeMenu.mm @@ -6,15 +6,12 @@ #import #include "nsStandaloneNativeMenu.h" -#include "nsMenuItemX.h" #include "nsMenuUtilsX.h" #include "nsIMutationObserver.h" #include "nsGkAtoms.h" #include "nsObjCExceptions.h" #include "mozilla/dom/Element.h" -using namespace mozilla; - using mozilla::dom::Element; NS_IMPL_ISUPPORTS_INHERITED(nsStandaloneNativeMenu, nsMenuGroupOwnerX, nsIMutationObserver, @@ -22,7 +19,11 @@ NS_IMPL_ISUPPORTS_INHERITED(nsStandaloneNativeMenu, nsMenuGroupOwnerX, nsIMutati nsStandaloneNativeMenu::nsStandaloneNativeMenu() : mMenu(nullptr), mContainerStatusBarItem(nil) {} -nsStandaloneNativeMenu::~nsStandaloneNativeMenu() {} +nsStandaloneNativeMenu::~nsStandaloneNativeMenu() { + if (mMenu) { + delete mMenu; + } +} NS_IMETHODIMP nsStandaloneNativeMenu::Init(Element* aElement) { @@ -39,7 +40,14 @@ nsStandaloneNativeMenu::Init(Element* aElement) { return rv; } - mMenu = MakeUnique(this, this, aElement); + mMenu = new nsMenuX(); + rv = mMenu->Create(this, this, aElement); + if (NS_FAILED(rv)) { + delete mMenu; + mMenu = nullptr; + return rv; + } + mMenu->SetupIcon(); return NS_OK; @@ -64,7 +72,7 @@ nsStandaloneNativeMenu::MenuWillOpen(bool* aResult) { // Force an update on the mMenu by faking an open/close on all of // its submenus. - UpdateMenu(mMenu.get()); + UpdateMenu(mMenu); *aResult = true; return NS_OK; @@ -101,8 +109,8 @@ nsStandaloneNativeMenu::ActivateNativeMenuItemAt(const nsAString& indexString) { // We can't perform an action on an item with a submenu, that will raise // an obj-c exception. - if (item && !item.hasSubmenu) { - NSMenu* parent = item.menu; + if (item && ![item hasSubmenu]) { + NSMenu* parent = [item menu]; if (parent) { // NSLog(@"Performing action for native menu item titled: %@\n", // [[currentSubmenu itemAtIndex:targetIndex] title]); @@ -127,17 +135,17 @@ nsStandaloneNativeMenu::ForceUpdateNativeMenuAt(const nsAString& indexString) { NSString* locationString = [NSString stringWithCharacters:reinterpret_cast(indexString.BeginReading()) length:indexString.Length()]; - NSArray* indexes = [locationString componentsSeparatedByString:@"|"]; - unsigned int indexCount = indexes.count; + NSArray* indexes = [locationString componentsSeparatedByString:@"|"]; + unsigned int indexCount = [indexes count]; if (indexCount == 0) { return NS_OK; } - nsMenuX* currentMenu = mMenu.get(); + nsMenuX* currentMenu = mMenu; // now find the correct submenu for (unsigned int i = 1; currentMenu && i < indexCount; i++) { - int targetIndex = [indexes objectAtIndex:i].intValue; + int targetIndex = [[indexes objectAtIndex:i] intValue]; int visible = 0; uint32_t length = currentMenu->GetItemCount(); for (unsigned int j = 0; j < length; j++) { @@ -145,12 +153,7 @@ nsStandaloneNativeMenu::ForceUpdateNativeMenuAt(const nsAString& indexString) { if (!targetMenu) { return NS_OK; } - MOZ_RELEASE_ASSERT(targetMenu->MenuObjectType() == eSubmenuObjectType || - targetMenu->MenuObjectType() == eMenuItemObjectType); - RefPtr content = targetMenu->MenuObjectType() == eSubmenuObjectType - ? static_cast(targetMenu)->Content() - : static_cast(targetMenu)->Content(); - if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(content)) { + if (!nsMenuUtilsX::NodeIsHiddenOrCollapsed(targetMenu->Content())) { visible++; if (targetMenu->MenuObjectType() == eSubmenuObjectType && visible == (targetIndex + 1)) { currentMenu = static_cast(targetMenu); @@ -172,11 +175,11 @@ void nsStandaloneNativeMenu::IconUpdated() { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; if (mContainerStatusBarItem) { - NSImage* menuImage = mMenu->NativeMenuItem().image; + NSImage* menuImage = [mMenu->NativeMenuItem() image]; if (menuImage) { - [menuImage setTemplate:YES]; + [menuImage setTemplate:true]; } - mContainerStatusBarItem.image = menuImage; + [mContainerStatusBarItem setImage:menuImage]; } NS_OBJC_END_TRY_ABORT_BLOCK; diff --git a/widget/cocoa/nsSystemStatusBarCocoa.mm b/widget/cocoa/nsSystemStatusBarCocoa.mm index 38878c0e89ff..842698764bfe 100644 --- a/widget/cocoa/nsSystemStatusBarCocoa.mm +++ b/widget/cocoa/nsSystemStatusBarCocoa.mm @@ -44,9 +44,9 @@ nsSystemStatusBarCocoa::StatusItem::StatusItem(nsStandaloneNativeMenu* aMenu) : mMenu->GetNativeMenu(reinterpret_cast(&nativeMenu)); mStatusItem = - [[NSStatusBar.systemStatusBar statusItemWithLength:NSSquareStatusItemLength] retain]; - mStatusItem.menu = nativeMenu; - mStatusItem.highlightMode = YES; + [[[NSStatusBar systemStatusBar] statusItemWithLength:NSSquareStatusItemLength] retain]; + [mStatusItem setMenu:nativeMenu]; + [mStatusItem setHighlightMode:YES]; // We want the status item to get its image from menu item that mMenu was // initialized with. Icon loads are asynchronous, so we need to let the menu @@ -61,7 +61,7 @@ nsSystemStatusBarCocoa::StatusItem::~StatusItem() { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; mMenu->SetContainerStatusBarItem(nil); - [NSStatusBar.systemStatusBar removeStatusItem:mStatusItem]; + [[NSStatusBar systemStatusBar] removeStatusItem:mStatusItem]; [mStatusItem release]; mStatusItem = nil;