Last Comment Bug 384877 - Crash [@ nsPopupSetFrame::Destroy] when clicking on popup id="d" popup="d"
: Crash [@ nsPopupSetFrame::Destroy] when clicking on popup id="d" popup="d"
Status: VERIFIED FIXED
[sg:nse dos] infinite stack recursion
: crash, testcase, verified1.8.0.14, verified1.8.1.8
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: x86 All
: -- critical (vote)
: mozilla1.9alpha8
Assigned To: Mats Palmgren (:mats)
:
: Neil Deakin
Mentors:
Depends on: 279703
Blocks:
  Show dependency treegraph
 
Reported: 2007-06-18 05:13 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2011-06-13 10:01 PDT (History)
12 users (show)
dveditz: blocking1.8.1.8+
dveditz: wanted1.8.1.x+
dveditz: blocking1.8.0.14+
dveditz: wanted1.8.0.x+
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (97 bytes, application/vnd.mozilla.xul+xml)
2007-06-18 05:13 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
stack (9.77 KB, text/html)
2007-06-20 17:00 PDT, Mats Palmgren (:mats)
no flags Details
Patch rev. 1 (4.26 KB, patch)
2007-06-20 17:10 PDT, Mats Palmgren (:mats)
enndeakin: review+
roc: superreview+
dveditz: approval1.8.1.8+
dveditz: approval1.8.0.14+
Details | Diff | Splinter Review
testcase (378 bytes, application/vnd.mozilla.xul+xml)
2009-04-02 06:10 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2007-06-18 05:13:14 PDT
Created attachment 268784 [details]
testcase

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.
Comment 1 Mats Palmgren (:mats) 2007-06-20 17:00:15 PDT
Created attachment 269143 [details]
stack
Comment 2 Mats Palmgren (:mats) 2007-06-20 17:10:03 PDT
Created attachment 269144 [details] [diff] [review]
Patch rev. 1

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.
Comment 3 Olli Pettay [:smaug] 2007-06-20 23:50:18 PDT
I'm a bit worried that there is nsDocElementBoxFrame created for the popup, 
though what should be done in this case...
Comment 4 neil@parkwaycc.co.uk 2007-06-21 02:08:29 PDT
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?
Comment 5 Neil Deakin 2007-06-22 07:32:31 PDT
The patch in bug 279703 already adds a check for the XULPopupListener change, but the tooltip change looks OK too.
Comment 6 Mats Palmgren (:mats) 2007-07-03 15:55:09 PDT
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
Comment 7 Samuel Sidler (old account; do not CC) 2007-07-16 18:18:00 PDT
This still affects the branch. Requesting blocking.
Comment 8 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-07-19 14:27:48 PDT
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007071905 Minefield/3.0a7pre
Comment 9 Mats Palmgren (:mats) 2007-07-19 16:30:11 PDT
Comment on attachment 269144 [details] [diff] [review]
Patch rev. 1

Low-risk crash fix.
Comment 10 Daniel Veditz [:dveditz] 2007-08-28 10:47:11 PDT
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
Comment 11 Mats Palmgren (:mats) 2007-08-30 18:55:27 PDT
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 
Comment 12 Carsten Book [:Tomcat] 2007-09-03 15:11:34 PDT
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
Comment 13 Al Billings [:abillings] 2007-12-11 17:39:03 PST
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.
Comment 14 Bob Clary [:bc:] 2009-04-01 21:16:17 PDT
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?
Comment 15 Martijn Wargers [:mwargers] (not working for Mozilla) 2009-04-02 06:10:54 PDT
Created attachment 370631 [details]
testcase

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
Comment 16 Bob Clary [:bc:] 2009-04-24 10:20:19 PDT
crash test added
http://hg.mozilla.org/mozilla-central/rev/cbef28dd9944

Note You need to log in before you can comment on or make changes to this bug.