13955 - [BLOCKER] Crash when selecting item in context menu

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Bill Law]

I added a context menu to navigator.xul (not checked in at the moment, but I'm
going to try to get it in later today).  When I select a menuitem in this
context menu, I get a crash.  Here's the stack trace:

nsMenuFrame::GetMenuParent(nsIMenuParent * * 0x0012f8ec) line 129 + 39 bytes
nsMenuPopupFrame::HideChain(nsMenuPopupFrame * const 0x01d8c2a8) line 549
nsMenuFrame::Execute() line 948
nsMenuFrame::HandleEvent(nsMenuFrame * const 0x00ec2880, nsIPresContext & {...},
nsGUIEvent * 0x0012fbe4, nsEventStatus & nsEventStatus_eConsumeDoDefault) line
238
PresShell::HandleEvent(PresShell * const 0x01b97f14, nsIView * 0x01b9c140,
nsGUIEvent * 0x0012fbe4, nsEventStatus & nsEventStatus_eConsumeDoDefault) line
2019 + 38 bytes
nsView::HandleEvent(nsView * const 0x01b9c140, nsGUIEvent * 0x0012fbe4, unsigned
int 0x00000008, nsEventStatus & nsEventStatus_eConsumeDoDefault, int &
0x00000000) line 828
nsView::HandleEvent(nsView * const 0x01b96370, nsGUIEvent * 0x0012fbe4, unsigned
int 0x0000001c, nsEventStatus & nsEventStatus_eConsumeDoDefault, int &
0x00000000) line 813
nsViewManager::DispatchEvent(nsViewManager * const 0x01b969a0, nsGUIEvent *
0x0012fbe4, nsEventStatus & nsEventStatus_eConsumeDoDefault) line 1647
HandleEvent(nsGUIEvent * 0x0012fbe4) line 63
nsWindow::DispatchEvent(nsWindow * const 0x01b9c864, nsGUIEvent * 0x0012fbe4,
nsEventStatus & nsEventStatus_eIgnore) line 332 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012fbe4) line 353
nsWindow::DispatchMouseEvent(unsigned int 0x0000012d, nsPoint * 0x00000000) line
3160 + 21 bytes
ChildWindow::DispatchMouseEvent(unsigned int 0x0000012d, nsPoint * 0x00000000)
line 3378
nsWindow::ProcessMessage(unsigned int 0x00000202, unsigned int 0x00000000, long
0x00440023, long * 0x0012fe04) line 2394 + 24 bytes
nsWindow::WindowProc(HWND__ * 0x003a06c4, unsigned int 0x00000202, unsigned int
0x00000000, long 0x00440023) line 401 + 27 bytes
USER32! 77e713ed()

Comment 1

[Bill Law]

Changing Severity to "blocker."

Comment 2

[Bill Law]

I've managed to come up with a fix for this.  It might be the "hack" Dave
rejected, but I'll throw this out there in case it's of any help.  I'm tempted
to check this in regardless because it at least lets me make some more progress
getting the context menus working (although I'm still blocked by bug #13986
regardless).  Just say the word and I'll go for it.

Here's my patch (which works for the 2 or 3 context menu functions I've tested:

Index: nsMenuPopupFrame.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/xul/base/src/nsMenuPopupFrame.cpp,v
retrieving revision 1.33
diff -r1.33 nsMenuPopupFrame.cpp
33a34
> #include "nsPopupSetFrame.h"
542,544c543,560
<     nsMenuFrame* menuFrame = (nsMenuFrame*)frame;
<     menuFrame->ActivateMenu(PR_FALSE);
<     menuFrame->SelectMenu(PR_FALSE);
---
>     // See if we have a popup set rather than a menu frame.
>     nsCOMPtr<nsIPopupSetFrame> popupSetFrameIndicator( do_QueryInterface(
frame ) );
>     if ( popupSetFrameIndicator ) {
>         // De-activate the popup (I'm guessing here).
>         nsPopupSetFrame *popupSetFrame = (nsPopupSetFrame*)frame;
>         popupSetFrame->ActivatePopup(PR_FALSE);
>     } else {
>         // Must be a regular menu frame.
>         nsMenuFrame *menuFrame = (nsMenuFrame*)frame;
>         menuFrame->ActivateMenu(PR_FALSE);
>         menuFrame->SelectMenu(PR_FALSE);
>
>         // Get the parent.
>         nsCOMPtr<nsIMenuParent> menuParent;
>         menuFrame->GetMenuParent(getter_AddRefs(menuParent));
>         if (menuParent)
>           menuParent->HideChain();
>     }
546,550d561
<     // Get the parent.
<     nsCOMPtr<nsIMenuParent> menuParent;
<     menuFrame->GetMenuParent(getter_AddRefs(menuParent));
<     if (menuParent)
<       menuParent->HideChain();
563,570c574,589
<     nsMenuFrame* menuFrame = (nsMenuFrame*)frame;
<     menuFrame->OpenMenu(PR_FALSE);
<
<     // Get the parent.
<     nsCOMPtr<nsIMenuParent> menuParent;
<     menuFrame->GetMenuParent(getter_AddRefs(menuParent));
<     if (menuParent)
<       menuParent->DismissChain();
---
>     // See if we have a popup set rather than a menu frame.
>     nsCOMPtr<nsIPopupSetFrame> popupSetFrameIndicator( do_QueryInterface(
frame ) );
>     if ( popupSetFrameIndicator ) {
>         // Close the popup.
>         nsPopupSetFrame *popupSetFrame = (nsPopupSetFrame*)frame;
>         popupSetFrame->OpenPopup(PR_FALSE);
>     } else {
>         nsMenuFrame* menuFrame = (nsMenuFrame*)frame;
>         menuFrame->OpenMenu(PR_FALSE);
>
>         // Get the parent.
>         nsCOMPtr<nsIMenuParent> menuParent;
>         menuFrame->GetMenuParent(getter_AddRefs(menuParent));
>         if (menuParent)
>           menuParent->DismissChain();
>     }

Comment 3

[David Hyatt]

Fixed.

Comment 4

[Bill Law]

...and verified. Good job.