101510 - crash when clicking cursive or fantasy droplists in the Fonts prefs dialog - N620 & Trunk [@ nsMenuPopupFrame::InstallKeyboardNavigator]

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect)

Description

[rbs]

STEPS TO REPRODUCE
 1. Edits -> Prefs -> Fonts
 2. Click the dropdown list for 'cursive'
    -> crash 

The crash doesn't seem to happen with options other than cursive.

ADDITIONAL INFO
  The crash is happening in nsMenuPopupFrame::InstallKeyboardNavigator().
  A stack trace is provided below.


NS_IMETHODIMP
nsMenuPopupFrame::InstallKeyboardNavigator()
{
  if (mKeyboardNavigator)
    return NS_OK;

  nsCOMPtr<nsIDocument> doc;
  mContent->GetDocument(*getter_AddRefs(doc));
  nsCOMPtr<nsIDOMEventReceiver> target = do_QueryInterface(doc);

  mTarget = target;  <------------------ target is null, crash will happen below
  mKeyboardNavigator = new nsMenuListener(this);
  NS_IF_ADDREF(mKeyboardNavigator);

  target->AddEventListener(NS_LITERAL_STRING("keypress"), 
(nsIDOMKeyListener*)mKeyboardNavigator, PR_TRUE); 
  target->AddEventListener(NS_LITERAL_STRING("keydown"), 
(nsIDOMKeyListener*)mKeyboardNavigator, PR_TRUE);  
  target->AddEventListener(NS_LITERAL_STRING("keyup"), 
(nsIDOMKeyListener*)mKeyboardNavigator, PR_TRUE);   
  
  return NS_OK;
}

stack trace
===========

nsMenuPopupFrame::InstallKeyboardNavigator(nsMenuPopupFrame * const 0x0478c518) 
line 1655 + 40 bytes
nsMenuFrame::OpenMenuInternal(int 1) line 724
nsMenuFrame::AttributeChanged(nsMenuFrame * const 0x047829fc, nsIPresContext * 
0x04eadad0, nsIContent * 0x04ea20c0, int 0, nsIAtom * 0x011a7b80 {"open"}, int 
2, int 3) line 649
nsCSSFrameConstructor::AttributeChanged(nsCSSFrameConstructor * const 
0x04e83130, nsIPresContext * 0x04eadad0, nsIContent * 0x04ea20c0, int 0, nsIAtom 
* 0x011a7b80 {"open"}, int 2, int 3) line 9972 + 39 bytes
StyleSetImpl::AttributeChanged(StyleSetImpl * const 0x04e87130, nsIPresContext * 
0x04eadad0, nsIContent * 0x04ea20c0, int 0, nsIAtom * 0x011a7b80 {"open"}, int 
2, int -1) line 1252
PresShell::AttributeChanged(PresShell * const 0x04e821e8, nsIDocument * 
0x04eaa8f0, nsIContent * 0x04ea20c0, int 0, nsIAtom * 0x011a7b80 {"open"}, int 
2, int -1) line 5006 + 61 bytes
nsXULDocument::AttributeChanged(nsXULDocument * const 0x04eaa8f0, nsIContent * 
0x04ea20c0, int 0, nsIAtom * 0x011a7b80 {"open"}, int 2, int -1) line 1768
nsXULElement::SetAttr(nsXULElement * const 0x04ea20c0, nsINodeInfo * 0x04c51f20, 
const nsAString & {...}, int 1) line 3100
nsXULElement::SetAttribute(nsXULElement * const 0x04ea20c4, const nsAString & 
{...}, const nsAString & {...}) line 1430 + 31 bytes
nsMenuFrame::OpenMenu(nsMenuFrame * const 0x04782a78, int 1) line 671 + 57 bytes
nsMenuFrame::ToggleMenuState(nsMenuFrame * const 0x047829fc) line 528
nsMenuFrame::HandleEvent(nsMenuFrame * const 0x047829fc, nsIPresContext * 
0x04eadad0, nsGUIEvent * 0x0012f780, nsEventStatus * 0x0012f674) line 396
PresShell::HandleEventInternal(nsEvent * 0x0012f780, nsIView * 0x04e80a30, 
unsigned int 1, nsEventStatus * 0x0012f674) line 5723 + 41 bytes
PresShell::HandleEvent(PresShell * const 0x04e821e4, nsIView * 0x04e80a30, 
nsGUIEvent * 0x0012f780, nsEventStatus * 0x0012f674, int 1, int & 1) line 5633 + 
25 bytes
nsView::HandleEvent(nsView * const 0x04e80a30, nsGUIEvent * 0x0012f780, unsigned 
int 28, nsEventStatus * 0x0012f674, int 1, int & 1) line 377
nsViewManager::DispatchEvent(nsViewManager * const 0x04e81030, nsGUIEvent * 
0x0012f780, nsEventStatus * 0x0012f674) line 2062
HandleEvent(nsGUIEvent * 0x0012f780) line 68
nsWindow::DispatchEvent(nsWindow * const 0x04e807b4, nsGUIEvent * 0x0012f780, 
nsEventStatus & nsEventStatus_eIgnore) line 733 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f780) line 754
nsWindow::DispatchMouseEvent(unsigned int 302, nsPoint * 0x00000000) line 4256 + 
21 bytes
ChildWindow::DispatchMouseEvent(unsigned int 302, nsPoint * 0x00000000) line 
4508
nsWindow::ProcessMessage(unsigned int 513, unsigned int 1, long 12452148, long * 
0x0012fb94) line 3221 + 24 bytes
nsWindow::WindowProc(HWND__ * 0x0013074a, unsigned int 513, unsigned int 1, long 
12452148) line 1001 + 27 bytes
USER32! 77e148dc()
USER32! 77e14aa7()
USER32! 77e266fd()
nsAppShellService::Run(nsAppShellService * const 0x0112d6e0) line 442
main1(int 1, char * * 0x004843c0, nsISupports * 0x00000000) line 1278 + 32 bytes
main(int 1, char * * 0x004843c0) line 1606 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e992a6()

Comment 1

[rbs]

More info: It is with a tip of the trunk.
The crash happens with the Classic skin but doesn't happen with the Modern skin.

Comment 2

[Jerome Lacoste]

reproduced on w2k/2001092308/classic theme

TB35849695Y

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

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

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

Critical bug. Changing OS and Platform to All since dupe bug 101636 mentioned
the crash occurs in linux and mac also.

And clarifying summary for easier querying...

Comment 5

[ji]

It's still happening on 09/26 0.9.4 build. Added nsbranch keyword.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This started showing up in talkback with the 9-22 builds.  It's a trunk
topcrash.  I'm not sure about the branch.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This is showing up as a topcrash on branch and trunk, and I suspect the cause is
hyatt's fix for bug 96291.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It was that change.  The following backout fixed the bug on the branch:

cvs update -j1.129.2.3 -j1.129.2.2 mozilla/layout/xul/base/src/nsMenuPopupFrame.cpp
cvs update -j1.78.2.2 -j1.78.2.1 mozilla/layout/xul/base/src/nsPopupSetFrame.cpp
cvs update -j1.129.2.2 -j1.129.2.1 mozilla/layout/xul/base/src/nsMenuPopupFrame.cpp
cvs update -j1.31.2.1 -j1.31 mozilla/layout/xul/base/src/nsPopupSetFrame.h
cvs update -j1.78.2.1 -j1.78 mozilla/layout/xul/base/src/nsPopupSetFrame.cpp
cvs update -j1.129.2.1 -j1.129 mozilla/layout/xul/base/src/nsMenuPopupFrame.cpp
cvs update -j1.6.2.1 -j1.6 mozilla/layout/xul/base/public/nsIPopupSetFrame.h
cvs update -j1.626.2.1 -j1.626
mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp

Comment 9

[David Hyatt]

Well, this is just weird.

Comment 10

[Blake Ross]

odd...I thought I just reassigned this to you minutes ago.

Comment 11

[sairuh (rarely reading bugmail)]

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

Comment 12

[sairuh (rarely reading bugmail)]

wacky, not a problem when selecting serif, sans-serif or monospace droplists.
only with the fantasy and cursive ones. tested using 2001.09.25-branch comm bits
on linux, winnt and mac 10.0.4.

has this been seen with other droplists in the app? just curious. i haven't seen
it with droplists in the other prefs panels...

Comment 13

[David Hyatt]

Does this happen in every skin?

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

rbs's comment above says this occurs in classic but not modern, which is also
what I see.

Comment 15

[sairuh (rarely reading bugmail)]

hm, at least the branch i see this happening with both modern and classic.

Comment 16

[David Hyatt]

Attachment: Screwed up ContentRemoved for popups

Comment 17

[David Hyatt]

Ready for r/sr.

Comment 18

[Brendan Eich [:brendan]]

Wahhh, don't add useless spaces:

-
+    
       // Locate the root popup set and remove ourselves from the popup set's list
       // of popup frames.
       nsIFrame* rootFrame;

(the + line, I mean).

/be

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 51015 [details] [diff] [review]
Screwed up ContentRemoved for popups

r=dbaron, other than the whitespace brendan mentioned

Comment 20

[David Hyatt]

sr anyone?

Comment 21

[TucsonTester2]

I'm seeing this with both the classic and modern skin on OSX with the 
20010924 build.

Comment 22

[Chris Waterson]

sr=waterson

Comment 23

[Chris Waterson]

(FWIW, do you know why the popup doesn't have a placeholder?)

Comment 24

[David Hyatt]

Yes.  There are two kinds of popups in the code right now, and one flavor
doesn't use a placeholder.

Comment 25

[Jaime Rodriguez, Jr.]

check it in - PDT+

Comment 26

[David Hyatt]

Fix checked in.

Comment 27

[sairuh (rarely reading bugmail)]

vrfy'd using commercial *branch* bits, 2001.10.01.0x on linux, winnt and mac os
10.0.4. tested with both classic and modern themes.

Comment 28

[Jay Patel [:jay]]

Adding N620 and Trunk to summary since we know this has been crashing on both
the MozillaTrunk and the Netscape6.20 branch.

Comment 29

[sairuh (rarely reading bugmail)]

vrfy'd fixed using 2001.10.04.0x-trunk bits on winnt and linux; also tested with
2001.10.03.20-trunk bits os 10.0.4, after using the workaround i mentioned at
2001-10-03 19:04 in bug 103036. [and, tested both themes.]