From 1d1add2df0b5e6f0fb69b661c650600f2906994d Mon Sep 17 00:00:00 2001 From: Steven Michaud Date: Mon, 13 Jan 2014 16:01:52 -0600 Subject: [PATCH] Bug 722676 - View menu item stays selected when returning from opened window. r=josh --- widget/cocoa/nsMenuBarX.h | 23 ++++++- widget/cocoa/nsMenuBarX.mm | 123 ++++++++++++++++++++++++++++++++++--- 2 files changed, 135 insertions(+), 11 deletions(-) diff --git a/widget/cocoa/nsMenuBarX.h b/widget/cocoa/nsMenuBarX.h index c482b646988..f1563f3023b 100644 --- a/widget/cocoa/nsMenuBarX.h +++ b/widget/cocoa/nsMenuBarX.h @@ -16,6 +16,7 @@ #include "nsString.h" class nsMenuX; +class nsMenuBarX; class nsMenuItemX; class nsIWidget; class nsIContent; @@ -32,11 +33,26 @@ public: NS_IMETHOD CreateNativeMenuBar(nsIWidget* aParent, nsIContent* aMenuBarNode); }; +@interface NSMenu (Undocumented) +// Undocumented method, present unchanged since OS X 10.6, used to temporarily +// highlight a top-level menu item when an appropriate Cmd+key combination is +// pressed. +- (void)_performActionWithHighlightingForItemAtIndex:(NSInteger)index; +@end + // Objective-C class used to allow us to intervene with keyboard event handling. // We allow mouse actions to work normally. @interface GeckoNSMenu : NSMenu { +@private + nsMenuBarX *mMenuBarOwner; // Weak -- if non-null it owns us + bool mDelayResignMainMenu; } +- (id)initWithTitle:(NSString *)aTitle andMenuBarOwner:(nsMenuBarX *)aMenuBarOwner; +- (void)resetMenuBarOwner; +- (bool)delayResignMainMenu; +- (void)setDelayResignMainMenu:(bool)aShouldDelay; +- (void)delayedPaintMenuBar:(id)unused; @end // Objective-C class used as action target for menu items @@ -80,6 +96,7 @@ public: static NativeMenuItemTarget* sNativeEventTarget; static nsMenuBarX* sLastGeckoMenuBarPainted; + static nsMenuBarX* sCurrentPaintDelayedMenuBar; // The following content nodes have been removed from the menu system. // We save them here for use in command handling. @@ -103,7 +120,9 @@ public: nsMenuX* GetMenuAt(uint32_t aIndex); nsMenuX* GetXULHelpMenu(); void SetSystemHelpMenu(); - nsresult Paint(); + nsresult Paint(bool aDelayed = false); + void PaintMenuBarAfterDelay(); + void ResetAwaitingDelayedPaint() { mAwaitingDelayedPaint = false; } void ForceUpdateNativeMenuAt(const nsAString& indexString); void ForceNativeMenuReload(); // used for testing static char GetLocalizedAccelKey(const char *shortcutID); @@ -121,6 +140,8 @@ protected: nsTArray< nsAutoPtr > mMenuArray; nsIWidget* mParentWindow; // [weak] GeckoNSMenu* mNativeMenu; // root menu, representing entire menu bar + + bool mAwaitingDelayedPaint; }; #endif // nsMenuBarX_h_ diff --git a/widget/cocoa/nsMenuBarX.mm b/widget/cocoa/nsMenuBarX.mm index 250448587f1..eb9330d2f61 100644 --- a/widget/cocoa/nsMenuBarX.mm +++ b/widget/cocoa/nsMenuBarX.mm @@ -28,7 +28,8 @@ #include "nsIDOMElement.h" NativeMenuItemTarget* nsMenuBarX::sNativeEventTarget = nil; -nsMenuBarX* nsMenuBarX::sLastGeckoMenuBarPainted = nullptr; +nsMenuBarX* nsMenuBarX::sLastGeckoMenuBarPainted = nullptr; // Weak +nsMenuBarX* nsMenuBarX::sCurrentPaintDelayedMenuBar = nullptr; // Weak NSMenu* sApplicationMenu = nil; BOOL gSomeMenuBarPainted = NO; @@ -55,11 +56,11 @@ NS_IMETHODIMP nsNativeMenuServiceX::CreateNativeMenuBar(nsIWidget* aParent, nsIC } nsMenuBarX::nsMenuBarX() -: nsMenuGroupOwnerX(), mParentWindow(nullptr) +: nsMenuGroupOwnerX(), mParentWindow(nullptr), mAwaitingDelayedPaint(false) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK; - mNativeMenu = [[GeckoNSMenu alloc] initWithTitle:@"MainMenuBar"]; + mNativeMenu = [[GeckoNSMenu alloc] initWithTitle:@"MainMenuBar" andMenuBarOwner:this]; NS_OBJC_END_TRY_ABORT_BLOCK; } @@ -91,6 +92,7 @@ nsMenuBarX::~nsMenuBarX() // before the registration hash table is destroyed. mMenuArray.Clear(); + [mNativeMenu resetMenuBarOwner]; [mNativeMenu release]; NS_OBJC_END_TRY_ABORT_BLOCK; @@ -339,10 +341,15 @@ void nsMenuBarX::SetSystemHelpMenu() } } -nsresult nsMenuBarX::Paint() +nsresult nsMenuBarX::Paint(bool aDelayed) { NS_OBJC_BEGIN_TRY_ABORT_BLOCK_NSRESULT; + if (!aDelayed && mAwaitingDelayedPaint) { + return NS_OK; + } + mAwaitingDelayedPaint = false; + // Don't try to optimize anything in this painting by checking // sLastGeckoMenuBarPainted because the menubar can be manipulated by // native dialogs and sheet code and other things besides this paint method. @@ -352,13 +359,36 @@ nsresult nsMenuBarX::Paint() 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]; - [outgoingMenu removeItemAtIndex:0]; - [mNativeMenu insertItem:appMenuItem atIndex:0]; - [appMenuItem release]; + // To work around bug 722676, we sometimes need to delay making mNativeMenu + // the main menu. This is an Apple bug that sometimes causes a top-level + // menu item to remain highlighted after pressing a Cmd+key combination that + // opens a new window, then closing the window. The OS temporarily + // highlights the appropriate top-level menu item whenever you press the + // Cmd+key combination for one of its submenus. (It does this by setting a + // "pressed" attribute on it.) The OS then uses a timer to remove this + // "pressed" attribute. But without our workaround we sometimes change the + // main menu before the timer has fired, so when it fires the menu item it + // was intended to unhighlight is no longer present in the main menu. This + // causes the item to remain semi-permanently highlighted (until you quit + // Firefox or navigate the main menu by hand). + if ((outgoingMenu != mNativeMenu) && + [outgoingMenu isKindOfClass:[GeckoNSMenu class]]) { + if (aDelayed) { + [(GeckoNSMenu *)outgoingMenu setDelayResignMainMenu:false]; + } else if ([(GeckoNSMenu *)outgoingMenu delayResignMainMenu]) { + PaintMenuBarAfterDelay(); + return NS_OK; + } + } - // Set menu bar and event target. - [NSApp setMainMenu:mNativeMenu]; + if (outgoingMenu != mNativeMenu) { + NSMenuItem* appMenuItem = [[outgoingMenu itemAtIndex:0] retain]; + [outgoingMenu removeItemAtIndex:0]; + [mNativeMenu insertItem:appMenuItem atIndex:0]; + [appMenuItem release]; + // Set menu bar and event target. + [NSApp setMainMenu:mNativeMenu]; + } SetSystemHelpMenu(); nsMenuBarX::sLastGeckoMenuBarPainted = this; @@ -369,6 +399,19 @@ nsresult nsMenuBarX::Paint() NS_OBJC_END_TRY_ABORT_BLOCK_NSRESULT; } +// Used to delay a call to nsMenuBarX::Paint(). Needed to work around +// bug 722676. +void nsMenuBarX::PaintMenuBarAfterDelay() +{ + mAwaitingDelayedPaint = true; + nsMenuBarX::sCurrentPaintDelayedMenuBar = this; + [mNativeMenu retain]; + // The delay for Apple's unhighlight timer is 0.1f, so we make ours a bit longer. + [mNativeMenu performSelector:@selector(delayedPaintMenuBar:) + withObject:nil + afterDelay:0.15f]; +} + // Returns the 'key' attribute of the 'shortcutID' object (if any) in the // currently active menubar's DOM document. 'shortcutID' should be the id // (i.e. the name) of a component that defines a commonly used (and @@ -727,6 +770,66 @@ static BOOL gMenuItemsExecuteCommands = YES; @implementation GeckoNSMenu +- (id)initWithTitle:(NSString *)aTitle +{ + if (self = [super initWithTitle:aTitle]) { + mMenuBarOwner = nullptr; + mDelayResignMainMenu = false; + } + return self; +} + +- (id)initWithTitle:(NSString *)aTitle andMenuBarOwner:(nsMenuBarX *)aMenuBarOwner +{ + if (self = [super initWithTitle:aTitle]) { + mMenuBarOwner = aMenuBarOwner; + mDelayResignMainMenu = false; + } + return self; +} + +- (void)resetMenuBarOwner +{ + mMenuBarOwner = nil; +} + +- (bool)delayResignMainMenu +{ + return mDelayResignMainMenu; +} + +- (void)setDelayResignMainMenu:(bool)aShouldDelay +{ + mDelayResignMainMenu = aShouldDelay; +} + +// Used to delay a call to nsMenuBarX::Paint(). Needed to work around +// bug 722676. +- (void)delayedPaintMenuBar:(id)unused +{ + if (mMenuBarOwner) { + if (mMenuBarOwner == nsMenuBarX::sCurrentPaintDelayedMenuBar) { + mMenuBarOwner->Paint(true); + nsMenuBarX::sCurrentPaintDelayedMenuBar = nullptr; + } else { + mMenuBarOwner->ResetAwaitingDelayedPaint(); + } + } + [self release]; +} + +// Undocumented method, present unchanged since OS X 10.6, used to temporarily +// highlight a top-level menu item when an appropriate Cmd+key combination is +// pressed. +- (void)_performActionWithHighlightingForItemAtIndex:(NSInteger)index; +{ + NSMenu *mainMenu = [NSApp mainMenu]; + if ([mainMenu isKindOfClass:[GeckoNSMenu class]]) { + [(GeckoNSMenu *)mainMenu setDelayResignMainMenu:true]; + } + [super _performActionWithHighlightingForItemAtIndex:index]; +} + // Keyboard commands should not cause menu items to invoke their // commands when there is a key window because we'd rather send // the keyboard command to the window. We still have the menus