Last Comment Bug 384105 - Crash [@ PresShell::AttributeChanged] with menuitem sizetopopup="always", position: absolute and tree stuff
: Crash [@ PresShell::AttributeChanged] with menuitem sizetopopup="always", pos...
[sg:critical?] fixed by 279703 on trunk
: crash, fixed1.8.0.15, testcase, verified1.8.1.8
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 All
-- critical (vote)
: ---
Assigned To: Olli Pettay [:smaug] (pto-ish for couple of days)
: Jet Villegas (:jet)
Depends on: 279703 400744 400976
Blocks: 344486
  Show dependency treegraph
Reported: 2007-06-11 19:05 PDT by Martijn Wargers [:mwargers]
Modified: 2011-06-13 10:01 PDT (History)
12 users (show)
dveditz: blocking1.8.1.8+
dveditz: wanted1.8.1.x+
dveditz: wanted1.8.0.x+
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

testcase (529 bytes, application/vnd.mozilla.xul+xml)
2007-06-11 19:05 PDT, Martijn Wargers [:mwargers]
no flags Details
Crash log from branch build on Mac (23.02 KB, text/plain)
2007-07-16 17:43 PDT, Samuel Sidler (old account; do not CC)
no flags Details
WIP for 1.8 (2.47 KB, patch)
2007-09-12 13:32 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
enndeakin: review+
roc: superreview+
dveditz: approval1.8.1.8+
Details | Diff | Splinter Review
do not notify when unsetting the attribute (937 bytes, patch)
2007-09-12 13:51 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
no flags Details | Diff | Splinter Review

Description User image Martijn Wargers [:mwargers] 2007-06-11 19:05:41 PDT
Created attachment 268038 [details]

See testcase, which crashes Mozilla within 100ms.
It also crashes branch builds, so marking security sensitive for now.
I guess bug 279703 might fix things here.

Talkback ID: TB33045810E
PresShell::AttributeChanged  [mozilla/layout/base/nspresshell.cpp, line 4408]
nsNodeUtils::AttributeChanged  [mozilla/content/base/src/nsnodeutils.cpp, line 100]
nsXULElement::UnsetAttr  [mozilla/content/xul/content/src/nsxulelement.cpp, line 1316]
nsXULElement::GetAttrInfo  [mozilla/content/xul/content/src/nsxulelement.cpp, line 2108]
PresShell::AddRef  [mozilla/layout/base/nspresshell.cpp, line 1394]
Comment 1 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-06-15 05:10:07 PDT
taking for now. Need to think this a bit. Ungeneratemenu() shouldn't be called in ::Destroy().
Probably need to call that asynchronously, but only if menu doesn't 
have a new frame.
Comment 2 User image Martijn Wargers [:mwargers] 2007-07-04 13:04:38 PDT
This is worksforme now that the patch for bug 279703 went in. (tested with a tinderbox build)
Comment 3 User image Samuel Sidler (old account; do not CC) 2007-07-16 17:43:00 PDT
Created attachment 272571 [details]
Crash log from branch build on Mac

This still affects a branch build. Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv: Gecko/20070716 BonEcho/

Requesting blocking (though; should probably be changed to wanted).
Comment 4 User image Daniel Veditz [:dveditz] 2007-08-01 19:35:59 PDT
Crashes rv: as well (Firefox

I can't see bug 279703 landing on the 1.8 branch. I also get a different stack on branch, "this" is a deleted object in nsCachedStyleData::GetStyleData(). Is there some way to wallpaper this for the branch? This looks familiar actually, maybe this testcase is triggering a different bug that's already fixed on trunk but not yet backported, and once we get that out of the way we'll see the nsPresShell::AttributeChanged issue on the branch.

>	nsCachedStyleData::GetStyleData() Line 210	C++
 	nsStyleContext::GetStyleData() Line 248	C++
 	nsIFrame::GetStyleData() Line 612	C++
 	nsIFrame::GetStyleDisplay() Line 90	C++
 	nsCSSFrameConstructor::AttributeChanged() Line 10752	C++
 	PresShell::AttributeChanged() Line 5499	C++
 	nsXULDocument::AttributeChanged() Line 1133	C++
 	nsXULElement::UnsetAttr() Line 1706	C++
 	nsMenuFrame::UngenerateMenu() Line 712	C++
 	nsMenuFrame::Destroy() Line 398	C++
 	nsLineBox::DeleteLineList() Line 325	C++
 	nsBlockFrame::Destroy() Line 303	C++
 	nsAreaFrame::Destroy() Line 155	C++
 	nsBoxFrame::RemoveFrame() Line 1178	C++
 	nsFrameManager::RemoveFrame() Line 717	C++
 	nsCSSFrameConstructor::ContentRemoved() Line 10141	C++
 	nsCSSFrameConstructor::RecreateFramesForContent() Line 12102	C++
 	nsCSSFrameConstructor::RestyleElement() Line 10600	C++
 	nsCSSFrameConstructor::ProcessOneRestyle() Line 14147	C++
 	nsCSSFrameConstructor::ProcessPendingRestyles() Line 14201	C++
 	nsCSSFrameConstructor::RestyleEvent::HandleEvent() Line 14265	C++
 	HandleRestyleEvent() Line 14274	C++
 	PL_HandleEvent() Line 688	C
 	PL_ProcessPendingEvents() Line 623	C
 	_md_EventReceiverProc() Line 1408	C
 	nsAppShell::Run() Line 133	C++
 	nsAppStartup::Run() Line 151	C++
 	XRE_main() Line 2711	C++
 	main() Line 61	C++
 	mainCRTStartup() Line 398	C
Comment 5 User image Daniel Veditz [:dveditz] 2007-08-28 11:01:03 PDT
This is a 1.8.1 branch blocker, but we can't take the 500K trunk patch ("redesign XUL popups"). Please see if there's a local fix for just this crash.
Comment 6 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-09-12 13:32:37 PDT
Created attachment 280653 [details] [diff] [review]
WIP for 1.8

Fixes the crash, but I do see 
###!!! ASSERTION: frame was not removed from primary frame map before destruction or was readded to map after being removed: 'Not Reached', file /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/layout/base/nsFrameManager.cpp, line 734

That assertion is (sort of) from bug Bug 265404.
Comment 7 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-09-12 13:50:11 PDT
Other possibility is to not to notify when removing the attribute. Same assertions
happen then though. And not notifying means no mutation events when dom is mutated; that is hackish.
Comment 8 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-09-12 13:51:03 PDT
Created attachment 280657 [details] [diff] [review]
do not notify when unsetting the attribute
Comment 9 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-09-12 13:52:15 PDT
Are there any other possibilities?
Comment 10 User image Daniel Veditz [:dveditz] 2007-10-02 10:45:36 PDT
Smaug: is this likely to get a branch patch, reviews and approvals by tomorrow night ( code freeze)?
Comment 11 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-10-02 10:52:17 PDT
Comment on attachment 280653 [details] [diff] [review]
WIP for 1.8

Roc, what do you think about this. Not perfect, but possibly enough for branch. Not sure if some problem occurs when recreating a frame for menu...
Comment 12 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2007-10-02 14:12:39 PDT
Comment on attachment 280653 [details] [diff] [review]
WIP for 1.8

looks OK but Neil should take a look
Comment 13 User image Daniel Veditz [:dveditz] 2007-10-03 13:04:46 PDT
Comment on attachment 280653 [details] [diff] [review]
WIP for 1.8

approved for, a=dveditz
Comment 14 User image Daniel Veditz [:dveditz] 2007-10-04 13:47:50 PDT
Fix checked in for Olli
Comment 15 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2007-10-04 15:03:50 PDT
Thanks Daniel. Somehow I didn't notice this got reviews.
Comment 16 User image Carsten Book [:Tomcat] 2007-10-12 08:33:04 PDT
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/2007100816 Firefox/ and Mozilla/5.0 (Macintosh; U; Intel Mac OS X; ja-JP-mac; rv: Gecko/2007100816 Firefox/ and the testcase from this bug.

-> no crash on testcase - adding verified keyword
Comment 17 User image Alexander Sack 2008-02-28 06:55:10 PST
Comment on attachment 280653 [details] [diff] [review]
WIP for 1.8

a=asac for

(same patch shipped by distros for some time now)
Comment 18 User image Christopher Aillon (sabbatical, not receiving bugmail) 2008-03-20 09:27:54 PDT
checked in on 1.8.0 branch
Comment 19 User image Bob Clary [:bc:] 2009-04-24 10:44:58 PDT
crash test landed

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