Bug 722676 - View menu item stays selected when returning from opened window. r=josh

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<nsMenuX> > mMenuArray;
   nsIWidget*         mParentWindow;        // [weak]
   GeckoNSMenu*       mNativeMenu;            // root menu, representing entire menu bar
+
+  bool               mAwaitingDelayedPaint;
 };
 
 #endif // nsMenuBarX_h_

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