Bug 584965: Improve key event handling for Cocoa widgets, follow Cocoa event propagation model properly. Fixes bug 582052 and bug 379199. r=smichaud

widget/src/cocoa/nsChildView.h

@@ -225,6 +225,8 @@ extern "C" long TSMProcessRawKeyEvent(EventRef carbonEvent);
 - (void)lockFocus;
 - (void) _surfaceNeedsUpdate:(NSNotification*)notification;
 
+- (BOOL)isPluginView;
+
 // Simple gestures support
 //
 // XXX - The swipeWithEvent, beginGestureWithEvent, magnifyWithEvent,

widget/src/cocoa/nsChildView.mm

@@ -119,6 +119,9 @@ extern "C" {
   extern CGError CGSGetWindowLevel(const CGSConnection cid, CGSWindow wid, CGWindowLevel *level);
 }
 
+// defined in nsMenuBarX.mm
+extern NSMenu* sApplicationMenu; // Application menu shared by all menubars
+
 // these are defined in nsCocoaWindow.mm
 extern PRBool gConsumeRollupEvent;
 
@@ -179,8 +182,6 @@ PRUint32 nsChildView::sLastInputEventCount = 0;
 
 - (void)processPendingRedraws;
 
-- (PRBool)processKeyDownEvent:(NSEvent*)theEvent keyEquiv:(BOOL)isKeyEquiv;
-
 - (void)maybeInitContextMenuTracking;
 
 + (NSEvent*)makeNewCocoaEventWithType:(NSEventType)type fromEvent:(NSEvent*)theEvent;
@@ -4059,14 +4060,14 @@ static PRBool IsSpecialGeckoKey(UInt32 macKeyCode)
 static PRBool IsNormalCharInputtingEvent(const nsKeyEvent& aEvent)
 {
   // this is not character inputting event, simply.
-  if (!aEvent.isChar || !aEvent.charCode)
+  if (!aEvent.isChar || !aEvent.charCode || aEvent.isMeta)
     return PR_FALSE;
   // if this is unicode char inputting event, we don't need to check
   // ctrl/alt/command keys
   if (aEvent.charCode > 0x7F)
     return PR_TRUE;
   // ASCII chars should be inputted without ctrl/alt/command keys
-  return !aEvent.isControl && !aEvent.isAlt && !aEvent.isMeta;
+  return !aEvent.isControl && !aEvent.isAlt;
 }
 
 // Basic conversion for cocoa to gecko events, common to all conversions.
@@ -5010,7 +5011,7 @@ static const char* ToEscapedString(NSString* aString, nsCAutoString& aBuf)
 // We only send Carbon plugin events with NS_KEY_DOWN gecko events, and only send
 // Cocoa plugin events with NS_KEY_PRESS gecko events. This is because we want to
 // send repeat key down events to Cocoa plugins but not Carbon plugins.
-- (PRBool)processKeyDownEvent:(NSEvent*)theEvent keyEquiv:(BOOL)isKeyEquiv
+- (PRBool)processKeyDownEvent:(NSEvent*)theEvent
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK_RETURN;
 
@@ -5094,19 +5095,17 @@ static const char* ToEscapedString(NSString* aString, nsCAutoString& aBuf)
   }
 
   // Let Cocoa interpret the key events, caching IsIMEComposing first.
-  // We don't do it if this came from performKeyEquivalent because
-  // interpretKeyEvents isn't set up to handle those key combinations.
   PRBool wasComposing = mGeckoChild->TextInputHandler()->IsIMEComposing();
   PRBool interpretKeyEventsCalled = PR_FALSE;
-  if (!isKeyEquiv &&
-      (mGeckoChild->TextInputHandler()->IsIMEEnabled() ||
-       mGeckoChild->TextInputHandler()->IsASCIICapableOnly())) {
+  if (mGeckoChild->TextInputHandler()->IsIMEEnabled() ||
+      mGeckoChild->TextInputHandler()->IsASCIICapableOnly()) {
     [super interpretKeyEvents:[NSArray arrayWithObject:theEvent]];
     interpretKeyEventsCalled = PR_TRUE;
   }
 
-  if (!mGeckoChild)
-    return (mKeyDownHandled || mKeyPressHandled);;
+  if (!mGeckoChild) {
+    return (mKeyDownHandled || mKeyPressHandled);
+  }
 
   if (!mKeyPressSent && nonDeadKeyPress && !wasComposing &&
       !mGeckoChild->TextInputHandler()->IsIMEComposing()) {
@@ -5180,6 +5179,15 @@ static const char* ToEscapedString(NSString* aString, nsCAutoString& aBuf)
 }
 #endif // NP_NO_CARBON
 
+// This is a private API that Cocoa uses.
+// Cocoa will call this after the menu system returns "NO" for "performKeyEquivalent:".
+// We want all they key events we can get so just return YES. In particular, this fixes
+// ctrl-tab - we don't get a "keyDown:" call for that without this.
+- (BOOL)_wantsKeyDownForEvent:(NSEvent*)event
+{
+  return YES;
+}
+
 - (void)keyDown:(NSEvent*)theEvent
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
@@ -5244,7 +5252,13 @@ static const char* ToEscapedString(NSString* aString, nsCAutoString& aBuf)
 #endif
   }
 
-  [self processKeyDownEvent:theEvent keyEquiv:NO];
+  PRBool handled = [self processKeyDownEvent:theEvent];
+  
+  // We always allow keyboard events to propagate to keyDown: but if they are not
+  // handled we give special Application menu items a chance to act.
+  if (!handled && sApplicationMenu) {
+    [sApplicationMenu performKeyEquivalent:theEvent];
+  }
 
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
@@ -5327,154 +5341,6 @@ static const char* ToEscapedString(NSString* aString, nsCAutoString& aBuf)
   NS_OBJC_END_TRY_ABORT_BLOCK;
 }
 
-- (BOOL)performKeyEquivalent:(NSEvent*)theEvent
-{
-  NS_OBJC_BEGIN_TRY_ABORT_BLOCK_RETURN;
-
-  // don't do anything if we don't have a gecko widget
-  if (!mGeckoChild)
-    return NO;
-
-  nsAutoRetainCocoaObject kungFuDeathGrip(self);
-
-  // If we're not the first responder and the first responder is an NSView
-  // object, pass the event on.  Otherwise (if, for example, the first
-  // responder is an NSWindow object) we should trust the OS to have called
-  // us correctly.
-  id firstResponder = [[self window] firstResponder];
-  if (firstResponder != self) {
-    // Special handling if the other first responder is a ChildView.
-    if ([firstResponder isKindOfClass:[ChildView class]])
-      return [(ChildView *)firstResponder performKeyEquivalent:theEvent];
-    if ([firstResponder isKindOfClass:[NSView class]])
-      return [super performKeyEquivalent:theEvent];
-  }
-
-  // don't process if we're composing, but don't consume the event
-  if (mGeckoChild->TextInputHandler()->IsIMEComposing()) {
-    return NO;
-  }
-
-  UInt32 modifierFlags = [theEvent modifierFlags] & NSDeviceIndependentModifierFlagsMask;
-
-  // Set to true if embedding menus handled the event when a plugin has focus.
-  // We give menus a crack at handling commands before Gecko in the plugin case.
-  BOOL handledByEmbedding = NO;
-
-  // Perform native menu UI feedback even if we stop the event from propagating to it normally.
-  // Recall that the menu system won't actually execute any commands for keyboard command invocations.
-  //
-  // If this is a plugin, we do actually perform the action on keyboard commands. See bug 428047.
-  // If the action on plugins here changes the first responder, don't continue.
-  NSMenu* mainMenu = [NSApp mainMenu];
-  if (mIsPluginView) {
-    if ([mainMenu isKindOfClass:[GeckoNSMenu class]]) {
-      // Maintain a list of cmd+key combinations that we never act on (in the
-      // browser) when the keyboard focus is in a plugin.  What a particular
-      // cmd+key combo means here (to the browser) is governed by browser.dtd,
-      // which "contains the browser main menu items".
-      PRBool dontActOnKeyEquivalent = PR_FALSE;
-      if (modifierFlags == NSCommandKeyMask) {
-        NSString *unmodchars = [theEvent charactersIgnoringModifiers];
-        if ([unmodchars length] == 1) {
-          if ([unmodchars characterAtIndex:0] ==
-              nsMenuBarX::GetLocalizedAccelKey("key_selectAll"))
-            dontActOnKeyEquivalent = PR_TRUE;
-        }
-      }
-      if (dontActOnKeyEquivalent) {
-        [(GeckoNSMenu*)mainMenu performMenuUserInterfaceEffectsForEvent:theEvent];
-      } else {
-        [(GeckoNSMenu*)mainMenu actOnKeyEquivalent:theEvent];
-      }
-    }
-    else {
-      // This is probably an embedding situation. If the native menu handle the event
-      // then return YES from pKE no matter what Gecko or the plugin does.
-      handledByEmbedding = [mainMenu performKeyEquivalent:theEvent];
-    }
-    if ([[self window] firstResponder] != self)
-      return YES;
-  }
-  else {
-    if ([mainMenu isKindOfClass:[GeckoNSMenu class]])
-      [(GeckoNSMenu*)mainMenu performMenuUserInterfaceEffectsForEvent:theEvent];
-  }
-
-  // With Cmd key or Ctrl+Tab or Ctrl+Esc, keyDown will be never called.
-  // Therefore, we need to call processKeyDownEvent from performKeyEquivalent.
-  UInt32 keyCode = [theEvent keyCode];
-  PRBool keyDownNeverFiredEvent = (modifierFlags & NSCommandKeyMask) ||
-           ((modifierFlags & NSControlKeyMask) &&
-            (keyCode == kEscapeKeyCode || keyCode == kTabKeyCode));
-
-  // don't handle this if certain modifiers are down - those should
-  // be sent as normal key up/down events and cocoa will do so automatically
-  // if we reject here
-  if (!keyDownNeverFiredEvent &&
-      (modifierFlags & (NSFunctionKeyMask| NSNumericPadKeyMask)))
-    return handledByEmbedding;
-
-  // Control and option modifiers are used when changing input sources in the
-  // input menu. We need to send such key events via "keyDown:", which will
-  // happen if we return NO here. This only applies to Mac OS X 10.5 and higher,
-  // previous OS versions just call "keyDown:" and not "performKeyEquivalent:"
-  // for such events.
-  if (!keyDownNeverFiredEvent &&
-      (modifierFlags & (NSControlKeyMask | NSAlternateKeyMask)))
-    return handledByEmbedding;
-
-  // At this point we're about to hand the event off to "processKeyDownEvent:keyEquiv:".
-  // Don't bother if this is a Cocoa plugin event, just handle it directly.
-  if (mGeckoChild && mIsPluginView && mPluginEventModel == NPEventModelCocoa) {
-    // Reset complex text input request.
-    mPluginComplexTextInputRequested = NO;
-
-    // Send key down event.
-    nsGUIEvent pluginEvent(PR_TRUE, NS_NON_RETARGETED_PLUGIN_EVENT, mGeckoChild);
-    NPCocoaEvent cocoaEvent;
-    ConvertCocoaKeyEventToNPCocoaEvent(theEvent, cocoaEvent);
-    pluginEvent.pluginEvent = &cocoaEvent;
-    mGeckoChild->DispatchWindowEvent(pluginEvent);
-    if (!mGeckoChild)
-      return YES;
-
-    if (!mPluginComplexTextInputRequested) {
-      // Ideally we'd cancel any TSM composition here.
-      return YES;
-    }
-
-    // We send these events to Carbon TSM but not to the ComplexTextInputPanel.
-    // We assume that events coming in through pKE: are only relevant to TSM.
-#ifndef NP_NO_CARBON
-    [self activatePluginTSMDoc];
-    // We use the active TSM document to pass a pointer to ourselves (the
-    // currently focused ChildView) to PluginKeyEventsHandler().  Because this
-    // pointer is weak, we should retain and release ourselves around the call
-    // to TSMProcessRawKeyEvent().
-    nsAutoRetainCocoaObject kungFuDeathGrip(self);
-    ::TSMSetDocumentProperty(mPluginTSMDoc, kFocusedChildViewTSMDocPropertyTag,
-                             sizeof(ChildView *), &self);
-    ::TSMProcessRawKeyEvent([theEvent _eventRef]);
-    ::TSMRemoveDocumentProperty(mPluginTSMDoc, kFocusedChildViewTSMDocPropertyTag);
-#endif
-
-    return YES;
-  }
-
-  if ([theEvent type] == NSKeyDown) {
-    // We trust the Gecko handled status for cmd key events. See bug 417466 for more info.
-    if (modifierFlags & NSCommandKeyMask)
-      return ([self processKeyDownEvent:theEvent keyEquiv:YES] || handledByEmbedding);
-    else
-      [self processKeyDownEvent:theEvent keyEquiv:YES];
-  }
-
-  return YES;
-
-  NS_OBJC_END_TRY_ABORT_BLOCK_RETURN(NO);
-}
-
 - (void)flagsChanged:(NSEvent*)theEvent
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;

widget/src/cocoa/nsCocoaWindow.mm

@@ -2034,6 +2034,14 @@ static const NSString* kStateShowsToolbarButton = @"showsToolbarButton";
   mScheduledShadowInvalidation = NO;
 }
 
+- (void) doCommandBySelector:(SEL)aSelector
+{
+  // We override this so that it won't beep if it can't act.
+  // We want to control the beeping for missing or disabled
+  // commands ourselves.
+  [self tryToPerform:aSelector with:nil];
+}
+
 @end
 
 // This class allows us to have a "unified toolbar" style window. It works like this:

widget/src/cocoa/nsMenuBarX.h

@@ -62,15 +62,11 @@ public:
   NS_IMETHOD CreateNativeMenuBar(nsIWidget* aParent, nsIContent* aMenuBarNode);
 };
 
-// Objective-C class used to allow us to have keyboard commands
-// look like they are doing something but actually do nothing.
+// Objective-C class used to allow us to intervene with keyboard event handling.
 // We allow mouse actions to work normally.
 @interface GeckoNSMenu : NSMenu
 {
 }
-- (BOOL)performKeyEquivalent:(NSEvent*)theEvent;
-- (void)actOnKeyEquivalent:(NSEvent*)theEvent;
-- (void)performMenuUserInterfaceEffectsForEvent:(NSEvent*)theEvent;
 @end
 
 // Objective-C class used as action target for menu items

widget/src/cocoa/nsMenuBarX.mm

@@ -46,6 +46,7 @@
 #include "nsCocoaUtils.h"
 #include "nsCocoaWindow.h"
 #include "nsToolkit.h"
+#include "nsChildView.h"
 
 #include "nsCOMPtr.h"
 #include "nsString.h"
@@ -657,8 +658,8 @@ nsresult nsMenuBarX::CreateApplicationMenu(nsMenuX* inMenu)
     BOOL addHideShowSeparator = FALSE;
     
     // Add menu item to hide this application
-    itemBeingAdded = CreateNativeAppMenuItem(inMenu, NS_LITERAL_STRING("menu_mac_hide_app"), @selector(hide:),
-                                             0, NSApp);
+    itemBeingAdded = CreateNativeAppMenuItem(inMenu, NS_LITERAL_STRING("menu_mac_hide_app"), @selector(menuItemHit:),
+                                             eCommand_ID_HideApp, nsMenuBarX::sNativeEventTarget);
     if (itemBeingAdded) {
       [sApplicationMenu addItem:itemBeingAdded];
       [itemBeingAdded release];
@@ -668,8 +669,8 @@ nsresult nsMenuBarX::CreateApplicationMenu(nsMenuX* inMenu)
     }
     
     // Add menu item to hide other applications
-    itemBeingAdded = CreateNativeAppMenuItem(inMenu, NS_LITERAL_STRING("menu_mac_hide_others"), @selector(hideOtherApplications:),
-                                             0, NSApp);
+    itemBeingAdded = CreateNativeAppMenuItem(inMenu, NS_LITERAL_STRING("menu_mac_hide_others"), @selector(menuItemHit:),
+                                             eCommand_ID_HideOthers, nsMenuBarX::sNativeEventTarget);
     if (itemBeingAdded) {
       [sApplicationMenu addItem:itemBeingAdded];
       [itemBeingAdded release];
@@ -679,8 +680,8 @@ nsresult nsMenuBarX::CreateApplicationMenu(nsMenuX* inMenu)
     }
     
     // Add menu item to show all applications
-    itemBeingAdded = CreateNativeAppMenuItem(inMenu, NS_LITERAL_STRING("menu_mac_show_all"), @selector(unhideAllApplications:),
-                                             0, NSApp);
+    itemBeingAdded = CreateNativeAppMenuItem(inMenu, NS_LITERAL_STRING("menu_mac_show_all"), @selector(menuItemHit:),
+                                             eCommand_ID_ShowAll, nsMenuBarX::sNativeEventTarget);
     if (itemBeingAdded) {
       [sApplicationMenu addItem:itemBeingAdded];
       [itemBeingAdded release];
@@ -729,50 +730,74 @@ void nsMenuBarX::SetParent(nsIWidget* aParent)
 // We allow mouse actions to work normally.
 //
 
-// This tells us whether or not pKE is on the stack from a GeckoNSMenu. If it
-// is nil, it is not on the stack. The non-nil value is the object that put it
-// on the stack first.
-static GeckoNSMenu* gPerformKeyEquivOnStack = nil;
-// If this is YES, act on key equivs.
-static BOOL gActOnKeyEquiv = NO;
-// When this variable is set to NO, don't do special command processing.
-static BOOL gActOnSpecialCommands = YES;
+// Controls whether or not native menu items should invoke their commands.
+static BOOL gMenuItemsExecuteCommands = YES;
 
 @implementation GeckoNSMenu
 
+// Keyboard commands should not cause menu items to invoke their
+// 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
 {
-  NS_OBJC_BEGIN_TRY_ABORT_BLOCK_RETURN;
-
-  NS_ASSERTION(gPerformKeyEquivOnStack != self, "GeckoNSMenu pKE re-entering for the same object!");
-
-  // Don't bother doing this if we don't have any items. It appears as though
-  // the OS will sometimes expect this sort of check.
-  if ([self numberOfItems] <= 0)
+  // 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) {
     return NO;
+  }
 
-  if (!gPerformKeyEquivOnStack)
-    gPerformKeyEquivOnStack = self;
-  BOOL rv = [super performKeyEquivalent:theEvent];
-  if (gPerformKeyEquivOnStack == self)
-    gPerformKeyEquivOnStack = nil;
-  return rv;
+  NSWindow *keyWindow = [NSApp keyWindow];
 
-  NS_OBJC_END_TRY_ABORT_BLOCK_RETURN(NO);
-}
+  // 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:theEvent];
+  }
 
--(void)actOnKeyEquivalent:(NSEvent *)theEvent
-{
-  gActOnKeyEquiv = YES;
-  [self performKeyEquivalent:theEvent];
-  gActOnKeyEquiv = NO;
-}
+  // Plugins normally eat all keyboard commands, this hack mitigates
+  // the problem.
+  BOOL handleForPluginHack = NO;
+  NSResponder *firstResponder = [keyWindow firstResponder];
+  if (firstResponder &&
+      [firstResponder isKindOfClass:[ChildView class]] &&
+      [(ChildView*)firstResponder isPluginView]) {
+    handleForPluginHack = YES;
+    // Maintain a list of cmd+key combinations that we never act on (in the
+    // browser) when the keyboard focus is in a plugin.  What a particular
+    // cmd+key combo means here (to the browser) is governed by browser.dtd,
+    // which "contains the browser main menu items".
+    UInt32 modifierFlags = [theEvent modifierFlags] & NSDeviceIndependentModifierFlagsMask;
+    if (modifierFlags == NSCommandKeyMask) {
+      NSString *unmodchars = [theEvent charactersIgnoringModifiers];
+      if ([unmodchars length] == 1) {
+        if ([unmodchars characterAtIndex:0] == nsMenuBarX::GetLocalizedAccelKey("key_selectAll")) {
+          handleForPluginHack = NO;
+        }
+      }
+    }
+  }
 
-- (void)performMenuUserInterfaceEffectsForEvent:(NSEvent*)theEvent
-{
-  gActOnSpecialCommands = NO;
-  [self performKeyEquivalent:theEvent];
-  gActOnSpecialCommands = YES;
+  gMenuItemsExecuteCommands = handleForPluginHack;
+  [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 (handleForPluginHack) {
+    if (![NSApp keyWindow] || [[NSApp keyWindow] firstResponder] != firstResponder) {
+      return YES;
+    }
+  }
+
+  // Return NO so that we can handle the event via NSView's "keyDown:".
+  return NO;
 }
 
 @end
@@ -788,6 +813,10 @@ static BOOL gActOnSpecialCommands = YES;
 {
   NS_OBJC_BEGIN_TRY_ABORT_BLOCK;
 
+  if (!gMenuItemsExecuteCommands) {
+    return;
+  }
+
   int tag = [sender tag];
 
   MenuItemInfo* info = [sender representedObject];
@@ -802,52 +831,48 @@ static BOOL gActOnSpecialCommands = YES;
   if (menuGroupOwner->MenuObjectType() == eMenuBarObjectType)
     menuBar = static_cast<nsMenuBarX*>(menuGroupOwner);
 
-  // We want to avoid processing app-global commands when we are asked to
-  // perform native menu effects only. This avoids sending events twice,
-  // which can lead to major problems.
-  if (gActOnSpecialCommands) {
-    // Do special processing if this is for an app-global command.
-    if (tag == eCommand_ID_About) {
-      nsIContent* mostSpecificContent = sAboutItemContent;
-      if (menuBar && menuBar->mAboutItemContent)
-        mostSpecificContent = menuBar->mAboutItemContent;
-      nsMenuUtilsX::DispatchCommandTo(mostSpecificContent);
-    }
-    else if (tag == eCommand_ID_Prefs) {
-      nsIContent* mostSpecificContent = sPrefItemContent;
-      if (menuBar && menuBar->mPrefItemContent)
-        mostSpecificContent = menuBar->mPrefItemContent;
-      nsMenuUtilsX::DispatchCommandTo(mostSpecificContent);
-    }
-    else if (tag == eCommand_ID_Quit) {
-      nsIContent* mostSpecificContent = sQuitItemContent;
-      if (menuBar && menuBar->mQuitItemContent)
-        mostSpecificContent = menuBar->mQuitItemContent;
-      // If we have some content for quit we execute it. Otherwise we send a native app terminate
-      // message. If you want to stop a quit from happening, provide quit content and return
-      // the event as unhandled.
-      if (mostSpecificContent) {
-        nsMenuUtilsX::DispatchCommandTo(mostSpecificContent);
-      }
-      else {
-        [NSApp terminate:nil];
-        return;
-      }
-    }
-    // Quit now if the "active" menu bar has changed (as the result of
-    // processing an app-global command above).  This resolves bmo bug
-    // 430506.
-    if (menuBar != nsnull && menuBar != nsMenuBarX::sLastGeckoMenuBarPainted)
-      return;
-  }
-
-  // Don't do anything unless this is not a keyboard command and
-  // this isn't for the hidden window menu. We assume that if there
-  // is no main window then the hidden window menu bar is up, even
-  // if that isn't true for some reason we better play it safe if
-  // there is no main window.
-  if (gPerformKeyEquivOnStack && !gActOnKeyEquiv && [NSApp mainWindow])
+  // Do special processing if this is for an app-global command.
+  if (tag == eCommand_ID_About) {
+    nsIContent* mostSpecificContent = sAboutItemContent;
+    if (menuBar && menuBar->mAboutItemContent)
+      mostSpecificContent = menuBar->mAboutItemContent;
+    nsMenuUtilsX::DispatchCommandTo(mostSpecificContent);
     return;
+  }
+  else if (tag == eCommand_ID_Prefs) {
+    nsIContent* mostSpecificContent = sPrefItemContent;
+    if (menuBar && menuBar->mPrefItemContent)
+      mostSpecificContent = menuBar->mPrefItemContent;
+    nsMenuUtilsX::DispatchCommandTo(mostSpecificContent);
+    return;
+  }
+  else if (tag == eCommand_ID_HideApp) {
+    [NSApp hide:sender];
+    return;
+  }
+  else if (tag == eCommand_ID_HideOthers) {
+    [NSApp hideOtherApplications:sender];
+    return;
+  }
+  else if (tag == eCommand_ID_ShowAll) {
+    [NSApp unhideAllApplications:sender];
+    return;
+  }
+  else if (tag == eCommand_ID_Quit) {
+    nsIContent* mostSpecificContent = sQuitItemContent;
+    if (menuBar && menuBar->mQuitItemContent)
+      mostSpecificContent = menuBar->mQuitItemContent;
+    // If we have some content for quit we execute it. Otherwise we send a native app terminate
+    // message. If you want to stop a quit from happening, provide quit content and return
+    // the event as unhandled.
+    if (mostSpecificContent) {
+      nsMenuUtilsX::DispatchCommandTo(mostSpecificContent);
+    }
+    else {
+      [NSApp terminate:nil];
+    }
+    return;
+  }
 
   // given the commandID, look it up in our hashtable and dispatch to
   // that menu item.
@@ -864,7 +889,7 @@ static BOOL gActOnSpecialCommands = YES;
 
 // Objective-C class used for menu items on the Services menu to allow Gecko
 // to override their standard behavior in order to stop key equivalents from
-// firing in certain instances. When gActOnSpecialCommands is NO, we return
+// firing in certain instances. When gMenuItemsExecuteCommands is NO, we return
 // a dummy target and action instead of the actual target and action.
 
 @implementation GeckoServicesNSMenuItem
@@ -872,19 +897,19 @@ static BOOL gActOnSpecialCommands = YES;
 - (id) target
 {
   id realTarget = [super target];
-  if (gActOnSpecialCommands)
+  if (gMenuItemsExecuteCommands)
     return realTarget;
   else
-    return realTarget != nil ? self : nil;
+    return realTarget ? self : nil;
 }
 
 - (SEL) action
 {
   SEL realAction = [super action];
-  if (gActOnSpecialCommands)
+  if (gMenuItemsExecuteCommands)
     return realAction;
   else
-    return realAction != NULL ? @selector(_doNothing:) : NULL;
+    return realAction ? @selector(_doNothing:) : NULL;
 }
 
 - (void) _doNothing:(id)sender

widget/src/cocoa/nsMenuBaseX.h

@@ -92,10 +92,13 @@ class nsMenuGroupOwnerX;
 // Carbon from messing with our event handlers. See bug 346883.
 
 enum {
-  eCommand_ID_About = 1,
-  eCommand_ID_Prefs = 2,
-  eCommand_ID_Quit  = 3,
-  eCommand_ID_Last  = 4
+  eCommand_ID_About      = 1,
+  eCommand_ID_Prefs      = 2,
+  eCommand_ID_Quit       = 3,
+  eCommand_ID_HideApp    = 4,
+  eCommand_ID_HideOthers = 5,
+  eCommand_ID_ShowAll    = 6,
+  eCommand_ID_Last       = 7
 };
 
 #endif // nsMenuBaseX_h_

widget/tests/Makefile.in

@@ -94,6 +94,7 @@ _CHROME_FILES += native_menus_window.xul \
                standalone_native_menu_window.xul \
                test_bug586713.xul \
                bug586713_window.xul \
+               test_key_event_counts.xul \
                $(NULL)
 endif
 

widget/tests/test_key_event_counts.xul

@@ -0,0 +1,60 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css" type="text/css"?>
+
+<!-- We've had issues on Mac OS X where native key events either don't get processed
+     or they get processed twice. This test tests some of those scenarios. -->
+
+<window id="window1" title="Test Key Event Counts" onload="runTest()"
+  xmlns:html="http://www.w3.org/1999/xhtml"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+  <script type="application/javascript" src="chrome://mochikit/content/MochiKit/packed.js"/>
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
+  <script type="text/javascript" src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"/>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+    <p id="display"></p>
+    <div id="content" style="display: none"></div>
+    <pre id="test"></pre>
+  </body>
+
+  <script type="application/javascript"><![CDATA[
+    var gKeyPressEventCount = 0;
+
+    function onKeyPress(e)
+    {
+      gKeyPressEventCount++;
+      e.preventDefault();
+    }
+
+    function runTest()
+    {
+      netscape.security.PrivilegeManager.enablePrivilege('UniversalXPConnect');
+
+      var domWindowUtils = window.QueryInterface(Components.interfaces.nsIInterfaceRequestor).
+                             getInterface(Components.interfaces.nsIDOMWindowUtils);
+
+      window.addEventListener("keypress", onKeyPress, false);
+
+      // Test ctrl-tab
+      gKeyPressEventCount = 0;
+      domWindowUtils.sendNativeKeyEvent(0, 48, 0x0400, "\t", "\t");
+      is(gKeyPressEventCount, 1);
+
+      // Test cmd+shift+a
+      gKeyPressEventCount = 0;
+      domWindowUtils.sendNativeKeyEvent(0, 0, 0x4000 | 0x0100, "a", "A");
+      is(gKeyPressEventCount, 1);
+
+      // Test cmd-;
+      gKeyPressEventCount = 0;
+      domWindowUtils.sendNativeKeyEvent(0, 41, 0x4000, ";", ";");
+      is(gKeyPressEventCount, 1);
+
+      window.removeEventListener("keypress", onKeyPress, false);
+    }
+  ]]></script>
+
+</window>