384644 Cookie dialog opens behind menu and blocks menu; menu may obscure dialog causing a mousy hang. Cancel all menu tracking before showing any modal dialog or sheet. r=smorgan sr=me a/1.6b1=me

camino/src/application/MainController.mm

@@ -319,6 +319,11 @@ NSString* const kPreviousSessionTerminatedNormallyKey = @"PreviousSessionTermina
       [restoreAfterCrashAlert setMessageText:NSLocalizedString(@"RestoreAfterCrashTitle", nil)];
       [restoreAfterCrashAlert setInformativeText:NSLocalizedString(@"RestoreAfterCrashMessage", nil)];
       [restoreAfterCrashAlert setAlertStyle:NSWarningAlertStyle];
+
+      // It should be impossible for a menu to be open so soon, but this
+      // should be called before displaying any modal dialogs.
+      [NSMenu cancelAllTracking];
+
       if ([restoreAfterCrashAlert runModal] == NSAlertFirstButtonReturn)
         shouldRestoreWindowState = YES;
     }
@@ -1108,6 +1113,7 @@ NSString* const kPreviousSessionTerminatedNormallyKey = @"PreviousSessionTermina
                           contextInfo:browserController];
   }
   else {
+    [NSMenu cancelAllTracking];
     int result = [openPanel runModalForTypes:fileTypes];
     [self openPanelDidEnd:openPanel returnCode:result contextInfo:nil];
   }
@@ -1270,6 +1276,7 @@ NSString* const kPreviousSessionTerminatedNormallyKey = @"PreviousSessionTermina
   [savePanel setAccessoryView:mExportPanelView];
 
   // start the save panel
+  [NSMenu cancelAllTracking];
   int saveResult = [savePanel runModalForDirectory:nil file:NSLocalizedString(@"ExportedBookmarkFile", @"Exported Bookmarks")];
   int selectedButton = [button indexOfSelectedItem];
   if (saveResult != NSFileHandlingPanelOKButton)

camino/src/bookmarks/BookmarkImportDlgController.mm

@@ -43,6 +43,7 @@
 #import "BrowserWindowController.h"
 #import "BookmarkViewController.h"
 #import "NSFileManager+Utils.h"
+#import "NSMenu+Utils.h"
 
 @interface BookmarkImportDlgController (Private)
 
@@ -195,6 +196,7 @@
   [openPanel setAllowsMultipleSelection:NO];
   [openPanel setPrompt:@"Import"];
   NSArray* array = [NSArray arrayWithObjects:@"htm", @"html", @"plist", nil];
+  [NSMenu cancelAllTracking];
   int result = [openPanel runModalForDirectory:nil
                                           file:nil
                                          types:array];

camino/src/browser/nsAlertController.h

@@ -47,22 +47,33 @@
 - (IBAction)hitButton3:(id)sender;
 
 // 
-// This is a version of [NSApp runModalForWindow:(relativeToWindow:)] that does three
-// things:
+// This is a version of [NSApp runModalForWindow:(relativeToWindow:)] that does
+// some extra things:
 // 
 //  1. It verifies that inParentWindow is a valid window to show a sheet on
 //     (i.e. that it's not nil, is visible, and doesn't already have a sheet).
+//     If inParentWindow can't take a sheet, a modal dialog is displayed.
 //
-//  2. It doesn't show inWindow as a sheet if there is already a modal (non-sheet)
-//     dialog on the screen, because that fubars AppKit.
+//  2. It doesn't show inWindow as a sheet if there is already a modal
+//     (non-sheet) dialog on the screen, because that fubars AppKit.  Instead,
+//     another modal dialog is displayed.
 // 
-//  3. It does some JS context stack magic that pushes a "native code" security principle
-//     ("trust label") so that Gecko knows we're running native code, and not calling
-//     from JS. This is important, because we can remain on the stack while PLEvents
-//     are being handled in the sheet's event loop, and those PLEvents can cause
-//     code to run that is sensitive to the security context. See bug 179307 for details.
+//  3. It does some JS context stack magic that pushes a "native code" security
+//     principal ("trust label") so that Gecko knows we're running native code,
+//     and not calling from JS. This is important, because we can remain on the
+//     stack while PLEvents are being handled in the sheet's event loop, and
+//     those PLEvents can cause code to run that is sensitive to the security
+//     context. See bug 179307 for details.
+//
+//  4. It closes any pull-down, pop-up, and contextual menus that are open
+//     prior to displaying the sheet or dialog.  Open menus are displayed
+//     on top of sheets or dialogs, but stop accepting input once a sheet or
+//     dialog is displayed, leading to a situation where the UI can get stuck.
 // 
-+ (int)safeRunModalForWindow:(NSWindow*)inWindow relativeToWindow:(NSWindow*)inParentWindow;
++ (int)safeRunModalForWindow:(NSWindow*)inWindow
+            relativeToWindow:(NSWindow*)inParentWindow;
+
++ (int)safeRunModalForWindow:(NSWindow*)window;
 
 // 
 // Nota Bene: all of these methods can throw Objective-C exceptions

camino/src/browser/nsAlertController.mm

@@ -37,6 +37,7 @@
 
 #include "nsServiceManagerUtils.h"
 #include "GeckoUtils.h"
+#include "NSMenu+Utils.h"
 
 #import "nsAlertController.h"
 #import "CHBrowserService.h"
@@ -118,39 +119,55 @@ const int kLabelCheckboxAdjustment = 2; // # pixels the label must be pushed dow
 
 @implementation nsAlertController
 
-+ (int)safeRunModalForWindow:(NSWindow*)inWindow relativeToWindow:(NSWindow*)inParentWindow
-{
-  if (inParentWindow)
-  {
-    // If there is already a modal window up, convert a sheet into a modal window,
-    // because AppKit will hang if you try to do this (possibly because we're using
-    // the deprecated and sucky runModalForWindow:relativeToWindow:).
-    // Also, if the parent window already has an attached sheet, or is not visible,
-    // also null out the parent and show this as a modal dialog.
-    if ([NSApp modalWindow] || [inParentWindow attachedSheet] || ![inParentWindow isVisible])
-      inParentWindow = nil;
++ (int)safeRunModalForWindow:(NSWindow*)window
+            relativeToWindow:(NSWindow*)parentWindow {
+  if (parentWindow) {
+    // If there is already a modal window up, convert a sheet into a modal
+    // window, otherwise AppKit will hang if a sheet is shown, possibly because
+    // we're using the deprecated and sucky runModalForWindow:relativeToWindow:.
+    // Also, if the parent window already has an attached sheet, or is not
+    // visible, also null out the parent and show this as a modal dialog.
+    if ([NSApp modalWindow] || [parentWindow attachedSheet] ||
+        ![parentWindow isVisible])
+      parentWindow = nil;
   }
 
   int result = NSAlertErrorReturn;
   nsresult rv = NS_OK;
   StNullJSContextScope hack(&rv);
-  if (NS_SUCCEEDED(rv))
-  {
+  if (NS_SUCCEEDED(rv)) {
+    // If a menu is open (pull-down, pop-up, context, whatever) when a
+    // modal dialog or sheet is displayed, the menu will hang and be unusable
+    // (not responding to any input) but visible.  The dialog will be usable
+    // but possibly obscured by the menu, and will be unable to receive mouse
+    // events in the obscured area.  If this happens, the user could wind up
+    // stuck.  To account for this, close any open menus before showing a
+    // modal dialog.
+    [NSMenu cancelAllTracking];
+
     // be paranoid; we don't want to throw Obj-C exceptions over C++ code
     @try {
-      if (inParentWindow)
-        result = [NSApp runModalForWindow:inWindow relativeToWindow:inParentWindow];
-      else
-        result = [NSApp runModalForWindow:inWindow];
+      if (parentWindow) {
+        result = [NSApp runModalForWindow:window
+                         relativeToWindow:parentWindow];
+      }
+      else {
+        result = [NSApp runModalForWindow:window];
+      }
     }
     @catch (id exception) {
-      NSLog(@"Exception caught in safeRunModalForWindow:relativeToWindow: %@", exception);
+      NSLog(@"Exception caught in safeRunModalForWindow:relativeToWindow: %@",
+            exception);
     }
   }
-    
+
   return result;
 }
 
++ (int)safeRunModalForWindow:(NSWindow*)window {
+  return [nsAlertController safeRunModalForWindow:window relativeToWindow:nil];
+}
+
 - (IBAction)hitButton1:(id)sender
 {
   [NSApp stopModalWithCode:NSAlertDefaultReturn];
@@ -500,12 +517,14 @@ const int kLabelCheckboxAdjustment = 2; // # pixels the label must be pushed dow
 
 - (int)runModalWindow:(NSWindow*)inDialog relativeToWindow:(NSWindow*)inParentWindow
 {
-  int result = [nsAlertController safeRunModalForWindow:inDialog relativeToWindow:inParentWindow];
-  
+  int result = [nsAlertController safeRunModalForWindow:inDialog
+                                       relativeToWindow:inParentWindow];
+
   // Convert any error into an exception
   if (result == NSAlertErrorReturn)
-      [NSException raise:NSInternalInconsistencyException format:@"-runModalForWindow returned error"];
-  
+      [NSException raise:NSInternalInconsistencyException
+                  format:@"-runModalForWindow returned error"];
+
   return result;
 }
 

camino/src/download/ProgressDlgController.mm

@@ -43,6 +43,7 @@
 
 #import "NSString+Utils.h"
 #import "NSView+Utils.h"
+#import "nsAlertController.h"
 
 #import "ProgressDlgController.h"
 
@@ -756,7 +757,7 @@ static id gSharedProgressController = nil;
         modalDelegate:self
        didEndSelector:@selector(sheetDidEnd:returnCode:contextInfo:)
           contextInfo:NULL];
-    int sheetResult = [NSApp runModalForWindow: panel];
+    int sheetResult = [nsAlertController safeRunModalForWindow:panel];
     [NSApp endSheet: panel];
     [panel orderOut: self];
     NSReleaseAlertPanel(panel);

camino/src/download/SaveHeaderSniffer.mm

@@ -38,6 +38,7 @@
 
 #import "NSString+Utils.h"
 #import "NSString+Gecko.h"
+#import "NSMenu+Utils.h"
 
 #import "ChimeraUIConstants.h"
 
@@ -347,7 +348,8 @@ nsHeaderSniffer::PerformSave(nsIURI* inOriginalURI)
         [savePanel setRequiredFileType:[file pathExtension]];
         [savePanel setCanSelectHiddenExtension: YES];
     }
-        
+
+    [NSMenu cancelAllTracking];
     if ([savePanel runModalForDirectory: nil file: file] == NSFileHandlingPanelCancelButton)
         return NS_OK;
        

camino/src/embedding/CHBrowserListener.mm

@@ -370,7 +370,7 @@ CHBrowserListener::ShowAsModal()
   }
 
   mIsModal = PR_TRUE;
-  //int result = [NSApp runModalForWindow:window];
+  //int result = [nsAlertController safeRunModalForWindow:window];
   mIsModal = PR_FALSE;
 
   return NS_OK;

camino/src/embedding/CHBrowserService.mm

@@ -37,6 +37,7 @@
  * ***** END LICENSE BLOCK ***** */
 
 #import "NSString+Gecko.h"
+#import "NSMenu+Utils.h"
 
 #import "CHBrowserService.h"
 #import "CHBrowserView.h"
@@ -339,6 +340,7 @@ CHBrowserService::PromptForSaveToFile(nsIHelperAppLauncher* aLauncher, nsISuppor
   // Note: although the docs for NSSavePanel specifically state "path and filename can be empty strings, but
   // cannot be nil" if you want the last used directory to persist between calls to display the save panel
   // use nil for the path given to runModalForDirectory
+  [NSMenu cancelAllTracking];
   int runResult = [thePanel runModalForDirectory: nil file:filename];
   if (runResult == NSOKButton) {
     // NSLog(@"Saving to %@", [thePanel filename]);

camino/src/extensions/NSMenu+Utils.h

@@ -45,6 +45,9 @@ extern NSString* const NSMenuClosedNotification;
 
 @interface NSMenu(ChimeraMenuUtils)
 
+// Closes all open menus.  Use this before displaying a modal sheet or dialog.
++ (void)cancelAllTracking;
+
 // turn on "NSMenuWillDisplayNotification" firing
 + (void)setupMenuWillDisplayNotifications;
 

camino/src/extensions/NSMenu+Utils.m

@@ -85,6 +85,76 @@ static OSStatus MenuEventHandler(EventHandlerCallRef inHandlerCallRef, EventRef
 
 #pragma mark -
 
+@interface CarbonMenuList : NSObject
+{
+  NSMutableArray* mList;
+}
+
++ (CarbonMenuList*)instance;
+- (id)init;
+- (void)dealloc;
+- (NSArray*)list;
+- (void)setupNotifications;
+- (void)menuOpen:(NSNotification*)notification;
+- (void)menuClose:(NSNotification*)notification;
+@end
+
+@implementation CarbonMenuList
+
++ (CarbonMenuList*)instance {
+  static CarbonMenuList* sInstance;
+  if (!sInstance) {
+    sInstance = [[self alloc] init];
+  }
+  return sInstance;
+}
+
+- (id)init {
+  if ((self = [super init])) {
+    // 16 slots is more than enough, as it's really only possible to wind up
+    // with as many open menus as the maximum submenu depth.  Even if the
+    // capacity here lowballs it, the array will expand dynamically.
+    mList = [[NSMutableArray alloc] initWithCapacity:16];
+  }
+  return self;
+}
+
+- (void)dealloc {
+  [[NSNotificationCenter defaultCenter] removeObserver:self];
+  [mList release];
+  [super dealloc];
+}
+
+- (NSArray*)list {
+  return mList;
+}
+
+- (void)setupNotifications {
+  NSNotificationCenter* notificationCenter = [NSNotificationCenter defaultCenter];
+  [notificationCenter addObserver:self
+                         selector:@selector(menuOpen:)
+                             name:NSMenuWillDisplayNotification
+                           object:nil];
+  [notificationCenter addObserver:self
+                         selector:@selector(menuClose:)
+                             name:NSMenuClosedNotification
+                           object:nil];
+}
+
+- (void)menuOpen:(NSNotification*)notification {
+  NSValue* value = [notification object];
+  if (![mList containsObject:value]) {
+    [mList addObject:value];
+  }
+}
+
+- (void)menuClose:(NSNotification*)notification {
+  [mList removeObject:[notification object]];
+}
+
+@end
+
+
 @implementation NSMenu(ChimeraMenuUtils)
 
 + (void)setupMenuWillDisplayNotifications
@@ -103,6 +173,35 @@ static OSStatus MenuEventHandler(EventHandlerCallRef inHandlerCallRef, EventRef
                                    menuEventList, (void*)self, NULL);
     sInstalled = YES;
   }
+
+  [[CarbonMenuList instance] setupNotifications];
+}
+
++ (void)cancelAllTracking {
+  // This method uses Carbon functions to do its dirty work, which seems to
+  // work.  It's no hackier than the other Carbon MenuRef action that the
+  // rest of this class uses.  There isn't really a good Cocoa substitute for
+  // CancelMenuTracking.  In Leopard, there's -[NSMenu cancelMenuTracking],
+  // but that doesn't seem to work to stop menu tracking when a sheet is
+  // about to be displayed.  Perhaps that's because it tries to fade the
+  // menu out, as CancelMenuTracking would do with |false| as its second
+  // argument.  (That doesn't work either.)
+
+  // Stop tracking the menu bar.  Even though the CarbonMenuList contains
+  // the menu bar's submenus, it doesn't contain the menu bar itself, and
+  // CancelMenuTracking will only stop tracking if called with the same
+  // MenuRef used for MenuSelect.  For menu bar pull-down menu tracking,
+  // that's the menu bar.
+  MenuRef rootMenu = AcquireRootMenu();
+  CancelMenuTracking(rootMenu, true, 0);
+  DisposeMenu(rootMenu);
+
+  // Stop tracking other types of menus, like pop-ups and contextual menus.
+  NSArray* list = [[CarbonMenuList instance] list];
+  for (unsigned int index = 0; index < [list count]; ++index) {
+    MenuRef menuRef = [(NSValue*)[list objectAtIndex:index] pointerValue];
+    CancelMenuTracking(menuRef, true, 0);
+  }
 }
 
 - (void)checkItemWithTag:(int)tag uncheckingOtherItems:(BOOL)uncheckOthers

camino/src/formfill/KeychainService.mm

@@ -48,6 +48,7 @@
 #import "CHBrowserService.h"
 #import "PreferenceManager.h"
 #import "KeyEquivView.h"
+#import "nsAlertController.h"
 
 #include "nsIPref.h"
 #include "nsIObserverService.h"
@@ -571,7 +572,8 @@ int KeychainPrefChangedCallback(const char* inPref, void* unused)
 //
 - (BOOL)confirmFillPassword:(NSWindow*)parent
 {
-  int result = [NSApp runModalForWindow:mConfirmFillPasswordPanel relativeToWindow:parent];
+  int result = [nsAlertController safeRunModalForWindow:mConfirmFillPasswordPanel
+                                       relativeToWindow:parent];
   [mConfirmFillPasswordPanel close];
   // Default is not to fill
   return (result != NSAlertDefaultReturn);

camino/src/security/ViewCertificateDialogController.mm

@@ -38,19 +38,20 @@
 
 #import "NSDate+Utils.h"
 
-#import "nsCOMPtr.h"
-#import "nsString.h"
-#import "nsIMutableArray.h"
-
-#import "nsIX509Cert.h"
-#import "nsIX509CertValidity.h"
-#import "nsIX509CertDB.h"
-
-#import "nsServiceManagerUtils.h"
-
 #import "CertificateItem.h"
 #import "CertificateView.h"
 #import "ViewCertificateDialogController.h"
+#import "nsAlertController.h"
+
+#include "nsCOMPtr.h"
+#include "nsString.h"
+#include "nsIMutableArray.h"
+
+#include "nsIX509Cert.h"
+#include "nsIX509CertValidity.h"
+#include "nsIX509CertDB.h"
+
+#include "nsServiceManagerUtils.h"
 
 
 @interface ViewCertificateDialogController(Private)
@@ -128,7 +129,7 @@
 - (int)runModally
 {
   mRunningModally = YES;
-  return [NSApp runModalForWindow:[self window]];
+  return [nsAlertController safeRunModalForWindow:[self window]];
 }
 
 - (void)allowTrustSaving:(BOOL)inAllow