Bug 722676 - View menu item stays selected when returning from opened window. r=josh
authorSteven Michaud <smichaud@pobox.com>
Mon, 13 Jan 2014 16:01:52 -0600
changeset 163221 a91130616d8308883d11e17b1469d2b55b7a165e
parent 163220 0bfe8d0cc4352668abaa5e619c660dddb08a4af1
child 163222 278aae1507a8f530dc87f504862940d512099de7
push id25986
push userkwierso@gmail.com
push dateTue, 14 Jan 2014 04:44:33 +0000
treeherdermozilla-central@b114534a9386 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjosh
bugs722676
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 722676 - View menu item stays selected when returning from opened window. r=josh
widget/cocoa/nsMenuBarX.h
widget/cocoa/nsMenuBarX.mm
--- a/widget/cocoa/nsMenuBarX.h
+++ b/widget/cocoa/nsMenuBarX.h
@@ -11,37 +11,53 @@
 #include "nsMenuBaseX.h"
 #include "nsMenuGroupOwnerX.h"
 #include "nsChangeObserver.h"
 #include "nsINativeMenuService.h"
 #include "nsAutoPtr.h"
 #include "nsString.h"
 
 class nsMenuX;
+class nsMenuBarX;
 class nsMenuItemX;
 class nsIWidget;
 class nsIContent;
 
 // The native menu service for creating native menu bars.
 class nsNativeMenuServiceX : public nsINativeMenuService
 {
 public:
   NS_DECL_ISUPPORTS
 
   nsNativeMenuServiceX() {}
   virtual ~nsNativeMenuServiceX() {}
 
   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
 @interface NativeMenuItemTarget : NSObject
 {
 }
 -(IBAction)menuItemHit:(id)sender;
 @end
@@ -75,16 +91,17 @@ public:
 class nsMenuBarX : public nsMenuGroupOwnerX, public nsChangeObserver
 {
 public:
   nsMenuBarX();
   virtual ~nsMenuBarX();
 
   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.
   nsCOMPtr<nsIContent> mAboutItemContent;
   nsCOMPtr<nsIContent> mUpdateItemContent;
   nsCOMPtr<nsIContent> mPrefItemContent;
   nsCOMPtr<nsIContent> mQuitItemContent;
 
@@ -98,17 +115,19 @@ public:
   // nsMenuBarX
   nsresult          Create(nsIWidget* aParent, nsIContent* aContent);
   void              SetParent(nsIWidget* aParent);
   uint32_t          GetMenuCount();
   bool              MenuContainsAppMenu();
   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);
 
 protected:
   void              ConstructNativeMenus();
   nsresult          InsertMenuAtIndex(nsMenuX* aMenu, uint32_t aIndex);
   void              RemoveMenuAtIndex(uint32_t aIndex);
@@ -116,11 +135,13 @@ protected:
   void              AquifyMenuBar();
   NSMenuItem*       CreateNativeAppMenuItem(nsMenuX* inMenu, const nsAString& nodeID, SEL action,
                                             int tag, NativeMenuItemTarget* target);
   nsresult          CreateApplicationMenu(nsMenuX* inMenu);
 
   nsTArray< nsAutoPtr<nsMenuX> > mMenuArray;
   nsIWidget*         mParentWindow;        // [weak]
   GeckoNSMenu*       mNativeMenu;            // root menu, representing entire menu bar
+
+  bool               mAwaitingDelayedPaint;
 };
 
 #endif // nsMenuBarX_h_
--- a/widget/cocoa/nsMenuBarX.mm
+++ b/widget/cocoa/nsMenuBarX.mm
@@ -23,17 +23,18 @@
 
 #include "nsIContent.h"
 #include "nsIWidget.h"
 #include "nsIDocument.h"
 #include "nsIDOMDocument.h"
 #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;
 
 // We keep references to the first quit and pref item content nodes we find, which
 // will be from the hidden window. We use these when the document for the current
 // window does not have a quit or pref item. We don't need strong refs here because
 // these items are always strong ref'd by their owning menu bar (instance variable).
 static nsIContent* sAboutItemContent  = nullptr;
@@ -50,21 +51,21 @@ NS_IMETHODIMP nsNativeMenuServiceX::Crea
   nsRefPtr<nsMenuBarX> mb = new nsMenuBarX();
   if (!mb)
     return NS_ERROR_OUT_OF_MEMORY;
 
   return mb->Create(aParent, aMenuBarNode);
 }
 
 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;
 }
 
 nsMenuBarX::~nsMenuBarX()
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
@@ -86,16 +87,17 @@ nsMenuBarX::~nsMenuBarX()
   UnregisterForContentChanges(mContent);
 
   // We have to manually clear the array here because clearing causes menu items
   // to call back into the menu bar to unregister themselves. We don't want to
   // depend on member variable ordering to ensure that the array gets cleared
   // before the registration hash table is destroyed.
   mMenuArray.Clear();
 
+  [mNativeMenu resetMenuBarOwner];
   [mNativeMenu release];
 
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
 nsresult nsMenuBarX::Create(nsIWidget* aParent, nsIContent* aContent)
 {
   if (!aParent || !aContent)
@@ -334,46 +336,87 @@ void nsMenuBarX::SetSystemHelpMenu()
   nsMenuX* xulHelpMenu = GetXULHelpMenu();
   if (xulHelpMenu) {
     NSMenu* helpMenu = (NSMenu*)xulHelpMenu->NativeData();
     if (helpMenu)
       [NSApp setHelpMenu:helpMenu];
   }
 }
 
-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.
 
   // 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, "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;
 
   gSomeMenuBarPainted = YES;
 
   return NS_OK;
 
   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
 // localized) cmd+key shortcut, and belongs to a keyset containing similar
 // objects.  For example "key_selectAll".  Returns a value that can be
 // compared to the first character of [NSEvent charactersIgnoringModifiers]
 // when [NSEvent modifierFlags] == NSCommandKeyMask.
 char nsMenuBarX::GetLocalizedAccelKey(const char *shortcutID)
@@ -722,16 +765,76 @@ void nsMenuBarX::SetParent(nsIWidget* aP
 // We allow mouse actions to work normally.
 //
 
 // Controls whether or not native menu items should invoke their commands.
 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
 // go through the mechanics so they'll give the proper visual
 // feedback.
 - (BOOL)performKeyEquivalent:(NSEvent *)theEvent
 {
   // We've noticed that Mac OS X expects this check in subclasses before