63452 - Crash clicking somewhere after failing to dismiss colorpicker popup

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Blake Ross]

Build ID: 12/20 trunk (tip)

Steps to Reproduce:

(1) Open Preferences.
(2) Mail and Newsgroups > Message Display.
(3) Open the colorpicker next to `Color:'.
(4) Press the up or down arrow key.
(5) Click anywhere within the Preferences dialog or outside the Mozilla window.

Result: crash.  I'm guessing the popup isn't properly getting dismissed.

Here's the strange stack I got:

nsCOMPtr<nsIMenuParent>::nsCOMPtr<nsIMenuParent>(nsIMenuParent * 0x02af2694) 
line 532 + 10 bytes
nsMenuDismissalListener::GetSubmenuWidgetChain(nsMenuDismissalListener * const 
0x03b457a4, nsISupportsArray * * 0x007bcff8) line 116
GKWIDGET! 01659c23()
GKWIDGET! 01659dc3()
KERNEL32! bff63613()
KERNEL32! bff848f7()

Comment 1

[Mike Pinkerton (not reading bugmail)]

icky. is this all platforms, or just win32?

Comment 2

[Keyser Sose]

I am seeing this on Linux

Platform: PC
OS: Linux 2.2.16 Red Hat
Mozilla Build: 2000122208 M18 Trunk Build

Hope that helps.

Comment 3

[Blake Ross]

nsWindow::DealWithPopups() 
(http://lxr.mozilla.org/seamonkey/source/widget/src/windows/nsWindow.cpp#829) 
isn't called when you use the arrow keys to change pref panes, since it checks 
for mouse-related events only.  It is, however, then called when you click, but 
by then something's gone wrong...(I haven't investigated enough to find out 
what, but this isn't helped by the fact that each pref pane is a completely 
separate document).  mMenuParent isn't null at that point, so I'm not 
completely sure why it's crashing.  How do we fix this?  We can't just roll up 
when an arrow key is pressed, obviously...

http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsMenuListener.cpp#1
73 is never reached since the key events aren't going to the <menupopup/>.  I 
guess an easy fix here would be to make these events go to the <colorpicker/> 
popup, but we should still have safeguards in the menu code itself.

cc'ing hyatt

Comment 4

[Blake Ross]

*** Bug 64022 has been marked as a duplicate of this bug. ***

Comment 5

[John Morrison]

(Note to the lame-ass qa dude on this bug: check the other test scenario(s)
from bug 64022). 

Although, that does raise a question: why is the keypress going to the menulist
in the modern skin, but to the preftree when using the classic skin. (yeah, 
yeah, yeah, -moz-user-focus or some such).

Comment 6

[Jesse Ruderman]

In bug 64161, I suggested a way for the color picker to go away when you click 
something else that involves giving it focus and making it go away when it 
loses focus.

Comment 7

[Joe Hewitt (gone)]

I was able to cause this crash prior to checking in the fix for 56150, but after
the fix this crash no longer happens.  Fixed.

Comment 8

[Blake Ross]

Well...there's still a crash lying around in menu code.  I guess I'll reopen 
64022 and hope that that's still around.

Comment 9

[Blake Ross]

reopening this to fix the deeper menu problem, but this is no longer a 
priority...

Comment 10

[John Morrison]

I don't get the crash with colorpicker now, but you can get this same crash on 
Mac in Classic skin (see bug 64022), and can use the same steps, with the 
addition of "menulist{-moz-user-focus:ignore;}" added to the CSS for 
win32/linux classic.

Comment 11

[Mike Pinkerton (not reading bugmail)]

how about we do this...i close this bug as fixed (since it is) and someone who
understands the "deeper menu problem" can reopen with a test case. This bug has
morphed so much, I can't tell priority or severity.

Comment 12

[Blake Ross]

Ok, i'll attach a testcase soon.  The point is that this was only fixed by 
fixing the colorpicker.  The former colorpicker, which didn't accept keyboard 
navigation, was perfectly valid, and yet it crashed.