90151 - Trunk & M092 bookmark crash using drag and drop [@ nsMenuFrame::ActivateMenu]

Status: VERIFIED FIXED

(SeaMonkey :: Bookmarks & History, defect, P1)

Description

[greer]

The scant user comments point to bookmark problems with drag and drop:

    (32454323) Comments: When I drag URL on bookmark on bar which located under 
URL window, Mozilla freeze and crash. 
     (32432146) Comments: I was trying to rearrange a toolbar bookmark folder 
using drag and drop. I wasn't in the Manage Bookmarks window. I was doing this 
in the toolbar.
     (32381197) URL: www.cnn.com
     (32381197) Comments: Had brought up cnn.com main page then started looking 
through bookmark menues.

Stack Trace:
        nsMenuFrame::ActivateMenu
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsMenuFrame.cpp  line 563]
         nsMenuPopupFrame::HideChain
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsMenuPopupFrame.cpp  line 
1372]
         nsMenuDismissalListener::Rollup
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsMenuDismissalListener.cpp  
line 90]
         nsWindow::Destroy      
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line
1235]
         nsView::~nsView        [d:\builds\seamonkey\mozilla\view\src\nsView.cpp  
line 158]
         nsView::`scalar deleting destructor'
         nsView::Destroy        [d:\builds\seamonkey\mozilla\view\src\nsView.cpp  
line 255]
         nsFrame::Destroy       
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsFrame.cpp  line
430]
         nsContainerFrame::Destroy
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp  line 
120]
         nsBoxFrame::Destroy    
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp
line 1009]
         nsGfxScrollFrame::Destroy
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsGfxScrollFrame.cpp  line 
446]
         nsFrameList::DestroyFrames
[d:\builds\seamonkey\mozilla\layout\base\src\nsFrameList.cpp  line 116]
         nsMenuFrame::Destroy   
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsMenuFrame.cpp
line 270]
         nsFrameList::DestroyFrame
[d:\builds\seamonkey\mozilla\layout\base\src\nsFrameList.cpp  line 202]
         nsBoxFrame::RemoveFrame
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 1063]
         FrameManager::RemoveFrame
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsFrameManager.cpp  line 905]
         nsCSSFrameConstructor::ContentRemoved
[d:\builds\seamonkey\mozilla\layout\html\style\src\nsCSSFrameConstructor.cpp  
line 9263]
         StyleSetImpl::ContentRemoved
[d:\builds\seamonkey\mozilla\content\base\src\nsStyleSet.cpp  line 1127]
         PresShell::ContentRemoved
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 4956]
         nsXULDocument::ContentRemoved
[d:\builds\seamonkey\mozilla\content\xul\document\src\nsXULDocument.cpp  line 
1750]
         nsXULElement::RemoveChildAt
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp  line 
2770]
         nsXULContentBuilder::RemoveMember
[d:\builds\seamonkey\mozilla\content\xul\templates\src\nsXULContentBuilder.cpp  
line 1104]
         nsXULContentBuilder::ReplaceMatch
[d:\builds\seamonkey\mozilla\content\xul\templates\src\nsXULContentBuilder.cpp  
line 1849]
         nsXULTemplateBuilder::Retract
[d:\builds\seamonkey\mozilla\content\xul\templates\src\nsXULTemplateBuilder.cpp  
line 601]
         nsXULTemplateBuilder::OnUnassert
[d:\builds\seamonkey\mozilla\content\xul\templates\src\nsXULTemplateBuilder.cpp  
line 636]
         CompositeDataSourceImpl::OnUnassert
[d:\builds\seamonkey\mozilla\rdf\base\src\nsCompositeDataSource.cpp  line 1577]
         nsBookmarksService::OnUnassert
[d:\builds\seamonkey\mozilla\xpfe\components\bookmarks\src\nsBookmarksService.cp
p  line 4742]
         InMemoryDataSource::Unassert
[d:\builds\seamonkey\mozilla\rdf\base\src\nsInMemoryDataSource.cpp  line 1296]
         nsBookmarksService::Unassert
[d:\builds\seamonkey\mozilla\xpfe\components\bookmarks\src\nsBookmarksService.cp
p  line 2982]
         CompositeDataSourceImpl::Unassert
[d:\builds\seamonkey\mozilla\rdf\base\src\nsCompositeDataSource.cpp  line 975]
         RDFContainerImpl::RemoveElement
[d:\builds\seamonkey\mozilla\rdf\base\src\nsRDFContainer.cpp  line 262]
         XPTC_InvokeByIndex
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp  
line 139]
         XPCWrappedNative::CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp  line 
1883]
         XPC_WN_CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp  
line 1253]
         js_Invoke      [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 
809]
         js_Interpret   [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 
2703]
         js_Invoke      [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 
825]
         js_Interpret   [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 
2703]
         js_Invoke      [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 
825]
         js_Interpret   [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 
2703]
         js_Invoke      [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 
825]
         js_InternalInvoke      [d:\builds\seamonkey\mozilla\js\src\jsinterp.c  
line 897]
         JS_CallFunctionValue   [d:\builds\seamonkey\mozilla\js\src\jsapi.c  
line 3322]
         nsJSContext::CallEventHandler
[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp  line 944]
         nsJSEventListener::HandleEvent
[d:\builds\seamonkey\mozilla\dom\src\events\nsJSEventListener.cpp  line 140]
         nsEventListenerManager::HandleEventSubType
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line 
1162]
         nsEventListenerManager::HandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line 
1978]
         nsXULElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp  line 
3635]
         nsXULElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp  line 
3654]
         nsXULElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp  line 
3654]
         nsXULElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp  line 
3654]
         PresShell::HandleEventInternal
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5570]
         PresShell::HandleEvent 
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp
line 5492]
         nsView::HandleEvent    [d:\builds\seamonkey\mozilla\view\src\nsView.cpp  
line 377]
         nsViewManager::DispatchEvent   
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp
line 2051]
         HandleEvent    [d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 
68]
         nsWindow::DispatchEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 724]
         nsNativeDragTarget::DispatchDragDropEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsNativeDragTarget.cpp  line 
184]
         nsNativeDragTarget::ProcessDrag
[d:\builds\seamonkey\mozilla\widget\src\windows\nsNativeDragTarget.cpp  line 
209]
         nsNativeDragTarget::Drop
[d:\builds\seamonkey\mozilla\widget\src\windows\nsNativeDragTarget.cpp  line 
308]
         ole32.dll + 0xcf6b2 (0x77b1f6b2)
         ole32.dll + 0xcf889 (0x77b1f889)
         ole32.dll + 0xa236e (0x77af236e)
         ole32.dll + 0xa2130 (0x77af2130)

Comment 1

[greer]

Doh! Meant to put this one under bookmarks.

Comment 2

[:deanis74]

reassigning to default owner and qa

Comment 3

[Viswanath Ramachandran]

nav triage: topcrash => this could be an rtm stopper. we need to get on top of 
it asap. 

Comment 4

[Viswanath Ramachandran]

adding hyatt and saari as its in xul layout. 

Comment 5

[John Morrison]

Attachment: reproducible testcase; set your personal toolbar up like this

Comment 6

[John Morrison]

Okay, I can crash consistently in a current trunk build with the following 
steps (see attachment for image of personal toolbar setup). 

1) with the personal toolbar setup as pictured (a folder on the toolbar which 
   contains a few bookmarks and another folder with more bookmarks). 
2) open 'New Folder2' and hover on 'New Folder3' to get its popup to show.
3) now drag 'New Folder3' to the personal toolbar.
4) drop.
5) crash.

It crashes with the reported stack trace, specifically at this point in 
nsMenuFrame::ActivateMenu, with |view| being null.

617   } else {
618     nsIView* view = nsnull;
619     menuPopup->GetView(mPresContext, &view);
620     nsCOMPtr<nsIViewManager> viewManager;
621     view->GetViewManager(*getter_AddRefs(viewManager));       // <- Crash!!
622     viewManager->SetViewVisibility(view, nsViewVisibility_kHide);
623     viewManager->ResizeView(view, 0, 0);
624     // set here so hide chain can close the menu as well.
625     mMenuOpen = PR_FALSE;
626   }

Comment 7

[Blake Ross]

--> me

Comment 8

[Blake Ross]

Attachment: patch

Comment 9

[:deanis74]

Makes sense to me, that's pretty much what I was going to track down.
r=[e-mail]

Comment 10

[Blake Ross]

ben sez 'sr=ben'

Comment 11

[lchiang]

PDT+ per PDT email for branch checkin.
Trunk checkin doesn't need PDT+ if you have r and sr.

jrgm - if you get a chance before tomorrow's build, can you apply Blake's patch
to your build (trunk will be fine) and see if your particular test case you
commented on is fixed?  Thanks.

Comment 12

[selmer (gone)]

Blake, please check in on the *branch* ASAP!  We need this to be in the
morning's build.

Comment 13

[John Morrison]

I didn't have a branch build to apply this to and the trunk is in a different 
state w.r.t. personal TB D&D. But I did apply the JS fix to the current branch 
candidate and that stopped the crash from occurring. 

I did, however, manage to get the crash a different way, dragging a folder 
along the TB somehow. I'll see if I can figure out how I made it crash the 
other way. It may well be the case that the C++ part of the patch (if view)...) 
would have caught that crash. 

But, either way, Blake's patch closes off a lot of this crash. And, gee, looky
there, it's on the branch now ...

Comment 14

[John Morrison]

There is a small opening where if you mousedown on a folder in the personal 
toolbar, get its popup to show, then drag (perhaps with a little upward or 
downward gesture), then you can get in the state where the popup is not rolled
up. You will then crash on the drop gesture. However, it is very difficult 
to reproduce (less than 1 in 20 times, even when you know (kinda) what you 
are looking for). And, it may be that the null check would prevent this 
crash anyways (I'm just running with the JS changes). 

Comment 15

[Blake Ross]

I just fixed the case John pointed out from the FE (the view null check probably
prevented it anyways).

Leaving this open for trunk checkin, which I'll do tomorrow.

Comment 16

[Claudius Gayle]

This bug is VERIFIED Fixed on a 2001072406 BRANCH build. Did this
fix ever get checked in to the trunk? I'll know the answer is 'yes'
when this bug gets a resolved fixed. Changing vbranch to vtrunk
as in the old-style "this needs to be verified on the trunk"

Comment 17

[John Morrison]

No, this is not on the trunk. I assume Blake left this a 0.9.3 because he
is going to check this in for this milestone. Da, Blake?

Comment 18

[Blake Ross]

Yeah.  This will go in tonight.  I have to do my part to spank the tree on
freeze night.

Comment 19

[Claudius Gayle]

note to self: on Win98 rather than crashing the current trunk build(01072403)
adds the url for the home button as the result of your drop when following
jrgm's original steps using the setup he outlined there. (and leaves the popup
open when you leave the menu with a mousedown)

Comment 20

[Blake Ross]

Fixed on trunk.  Claudius, the problem you mentioned is also fixed.

Comment 21

[Blake Ross]

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

Comment 22

[Jay Patel [:jay]]

Verifying fixed.  Talkback data only has one incident reported in the last 30
days:   38550640     2001072700   Netscape6.10   Windows 98 4.10 build 67766446
  2001-11-26 23:12:57   nsMenuFrame::ActivateMenu c9b4209f   11939   750560