384644 - Cookie dialog opens behind menu and blocks menu; menu may obscure dialog causing a mousy hang

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Jeff Jordan]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.8.1.4) Gecko/20070509 Camino/1.5
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en; rv:1.8.1.4) Gecko/20070509 Camino/1.5

When the bookmarks menu, or another menu, is open while a page loads, and the cookie dialogue appears for Deny/Allow/This Session Only, the cookie dialogue is behind the menu, and the menu is frozen open.  Can result in the cookie dialogue being completely blocked, and the user stuck.

Reproducible: Always

Steps to Reproduce:
1. Set Camino Preferences to Ask Before Accepting each cookie.
2. Go to any website that wants to set cookies.
3. Open the Bookmarks menu (or any of the menus) quickly as the webpage starts to load, and then leave the menu open.
4. When the cookie dialogue appears, it's behind the menu, and the menu is unresponsive to the mouse.  Camino will be waiting for a cookie dialogue button to be clicked, but the dialogue is behind the open menu, and the menu is unresponsive to clicks.  The user is blocked out.
Actual Results:  
Could not see cookie dialogue box, and could not close menu.

Expected Results:  
The menu should still be responsive with the cookie dialogue behind it.  Or, the menu should close before the cookie dialogue opens.

Comment 1

[Stuart Morgan]

Odd, I thought this was fixed by one or more of the bugs hanging off of bug 294129.

Comment 2

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

This is the second bug that should have been/was fixed by that (bug 380792 is the other); could something have regressed those fixes?

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I can reproduce this in 1.0.4, so either it regressed sometime between 1.0 and now, or we didn't actually "fix" bug 200397.  That is, the sheets still repsond to the keyboard, and, if positioned correctly, the mouse, so that state is not locked, but sheets coming up don't actually close menus.

Jeff: are you saying that if a button is visible (not behind the menu) on the cookie sheet, you can't click the visible button with the mouse?  (I can't reproduce that.)

(Bug 380792 might be slightly different, depending on how we have to close those; see the still-open bug 189298.)

Comment 4

[Jeff Jordan]

Smokey:  if a button is visible, I can click it, yes.

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Confirming, then; the real issue is we didn't actually "fix" bug 200397, so sheets coming up don't actually close menus.

Comment 7

[Mark Mentovai]

Attachment: Cancel all menu tracking when calling runModalForWindow:relativeToWindow:

Try this.

Seriously, try it.  This Carbon-play in a Cocoa app is the kind of thing that might work on one OS release but not another.  I also didn't test it out with all kinds of sheets.  Knock yourself out.  Have a good time.

There's another call to runModalForWindow: (but not runModalForWindow:relativeToWindow:) in ViewCertificateDialogController.  We should decide if we want to roll up menus in that case too.  This patch is structured so that any modal window, sheet or otherwise, will cancel menu tracking if it goes through [nsAlertController safeRunModalForWindow:relativeToWindow:] and converts a straggler in KeychainService to use our safe method.  We could convert the straggler in ViewCertificateDialogController too, and just pass relativeToWindow:nil.  I think it's probably a good idea for consistency, but I didn't test that at all and I have to leave now.

Comment 8

[Mark Mentovai]

I've been playing with this and attachment 121777 [details] (bug 200397) and have found that the sucky behavior occurs with modal dialogs in addition to sheets.  For example:

1. Load attachment 121777 [details]
2. Before its delayed alert fires (1.5 seconds), quickly hit command-D to open up another sheet and then open some pull-down menu from the menu bar
3. Wait for the delayed alert to fire as a modal dialog instead of a sheet

Note similar UI lockup.

So we should do this for all modal dialogs, not just sheets opened with [NSApplication runModalForWindow:relativeToWindow:].

Comment 9

[Mark Mentovai]

Attachment: Cancel all menu tracking before showing any modal dialog or sheet

I found the call sites for this with |grep -r runModal mozilla/camino|.

Comment 10

[Mark Mentovai]

Comment on attachment 292883 [details] [diff] [review]
Cancel all menu tracking before showing any modal dialog or sheet

Simon, let me know if you're not doing reviews, and I'll pick another victim.

Comment 11

[Mark Mentovai]

I also want to kill:
        NSLog([file pathExtension]);

in SaveHeaderSniffer.mm.

Comment 12

[Stuart Morgan]

(In reply to comment #11)
> I also want to kill:
>         NSLog([file pathExtension]);
> 
> in SaveHeaderSniffer.mm.

Yes please! r=me to land that as a no-bug change.

Comment 13

[Stuart Morgan]

Comment on attachment 292883 [details] [diff] [review]
Cancel all menu tracking before showing any modal dialog or sheet

r=me.

Have you filed the modal dialog case with Apple? That seems like a case that other apps would have as well (Safari, for example, has exactly the same issue), so presumably the OS should take care of it.

Comment 14

[Mark Mentovai]

Checked in on the trunk and MOZILLA_1_8_BRANCH for Camino 1.6b1.  The SaveHeaderSniffer NSLog change was also checked in.  I'll file a bug with Apple once I create a tiny self-contained reduced testcase.

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Clearing the obsolete flag.

(In reply to comment #14)
> I'll file a bug with Apple
> once I create a tiny self-contained reduced testcase.

Mark, have you had a chance to file a bug yet?