Martijn Wargers (dead) Reporter
	Description • 17 years ago

When clicking on the document of the testcase, Mozilla crashes.

https://crash-reports.mozilla.com/reports/report/index/88bd6778-1d8c-11dc-99b8-001a4bd43ef6
0  	nsPopupSetFrame::Destroy()
1 	nsFrameList::DestroyFrames()
2 	nsContainerFrame::Destroy()
3 	nsBoxFrame::RemoveFrame(nsIAtom *,nsIFrame *)
4 	nsRootBoxFrame::RemoveFrame(nsIAtom *,nsIFrame *)
5 	nsFrameManager::RemoveFrame(nsIFrame *,nsIAtom *,nsIFrame *)
6 	nsCSSFrameConstructor::ContentRemoved(nsIContent *,nsIContent *,int,int)
7 	PresShell::ContentRemoved(nsIDocument *,nsIContent *,nsIContent *,int)
8 	nsBindingManager::ContentRemoved(nsIDocument *,nsIContent *,nsIContent *,int)
9 	nsNodeUtils::ContentRemoved(nsINode *,nsIContent *,int)

It also crashes branch builds, so I'm marking it security sensitive for now.
I guess this depends on the fix for bug 279703.

	Mats Palmgren (inactive) Assignee
	Comment 2 • 17 years ago

Return early in LaunchPopup() if the popup id refers to the popup itself.
Tooltips are probably not affected (at least I couldn't find a testcase
that triggered it) but I added a similar check to ShowTooltip() just in case.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 3 • 17 years ago

I'm a bit worried that there is nsDocElementBoxFrame created for the popup, 
though what should be done in this case...

	neil@parkwaycc.co.uk
	Comment 4 • 17 years ago

I think Enn is best placed to comment on this... I'm curious as to whether there's a way to avoid opening an open popup; might there be an exploit caused by having a popup be the context menu for a menuitem in a submenu of itself?

	Neil Deakin
	Comment 5 • 17 years ago

The patch in bug 279703 already adds a check for the XULPopupListener change, but the tooltip change looks OK too.

	Mats Palmgren (inactive) Assignee
	Comment 6 • 17 years ago

mozilla/content/xul/content/src/nsXULPopupListener.cpp 	1.146 	mozilla/layout/xul/base/src/nsPopupSetFrame.cpp 	1.167 	mozilla/layout/xul/base/src/nsXULTooltipListener.cpp 	1.63 

-> FIXED

	Samuel Sidler (old account; do not CC)
	Comment 7 • 17 years ago

This still affects the branch. Requesting blocking.

	Martijn Wargers (dead) Reporter
	Comment 8 • 17 years ago

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre

	Mats Palmgren (inactive) Assignee
	Comment 9 • 17 years ago

Comment on attachment 269144 [details] [diff] [review]
Patch rev. 1

Low-risk crash fix.

	Daniel Veditz [:dveditz]
	Comment 10 • 17 years ago

Comment on attachment 269144 [details] [diff] [review]
Patch rev. 1

approved for 1.8.1.7 and 1.8.0.14, a=dveditz for release-drivers

	Mats Palmgren (inactive) Assignee
	Comment 11 • 17 years ago

MOZILLA_1_8_BRANCH
mozilla/content/xul/content/src/nsXULPopupListener.cpp 	1.125.12.6
mozilla/layout/xul/base/src/nsPopupSetFrame.cpp 	1.123.10.11
mozilla/layout/xul/base/src/nsXULTooltipListener.cpp 	1.43.4.4 

MOZILLA_1_8_0_BRANCH
mozilla/content/xul/content/src/nsXULPopupListener.cpp 	1.125.16.4
mozilla/layout/xul/base/src/nsPopupSetFrame.cpp 	1.123.10.2.2.8
mozilla/layout/xul/base/src/nsXULTooltipListener.cpp 	1.43.14.4 

	Carsten Book [:Tomcat]
	Comment 12 • 17 years ago

verified fixed 1.8.1.7 using the testcase from this bug and:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.7pre) Gecko/2007090308 BonEcho/2.0.0.7pre

and : Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.7pre) Gecko/20070903 BonEcho/2.0.0.7pre ID:2007090304

no crash on testcase - adding verified keyword

	Al Billings [:abillings - ex-MoCo]
	Comment 13 • 17 years ago

The test case doesn't crash the FF 1.5.0.12 release: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.14pre) Gecko/20071210 Firefox/1.5.0.13pre.

	Bob Clary [:bc] (inactive)
	Comment 14 • 16 years ago

I tried synthesizing the mouse click to automate the crash but have failed. Is there any trick to dispatch a mouse click to the popup that will cause the crash?

	Martijn Wargers (dead) Reporter
	Comment 15 • 16 years ago

This crashes automatically, without having to click in the document in builds without the patch that fixed this.
In Firefox 3 and beyond there is now openPopup instead of showPopup, that's why the difference in code paths.
I also tried to do a check on the state property of the popup, but that thing doesn't seem to work correctly at all in this case.
https://developer.mozilla.org/en/XUL/menupopup#p-state

	Bob Clary [:bc] (inactive)
	Comment 16 • 15 years ago

crash test added
http://hg.mozilla.org/mozilla-central/rev/cbef28dd9944