Closed
Bug 415394
Opened 16 years ago
Closed 15 years ago
Crash [@ nsBoxFrame::GetContentInsertionFrame] with <xul:listboxbody>
Categories
(Core :: XUL, defect, P4)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: bzbarsky)
References
Details
(Keywords: crash, testcase, verified1.9.1, Whiteboard: [ccbr])
Crash Data
Attachments
(3 files)
755 bytes,
application/xhtml+xml
|
Details | |
6.06 KB,
text/plain
|
Details | |
2.48 KB,
patch
|
dbaron
:
review+
dveditz
:
approval1.9.1.4+
|
Details | Diff | Splinter Review |
Loading this testcase often triggers a crash [@ nsBoxFrame::GetContentInsertionFrame]. The crash is most reliable if the testcase is the first thing loaded in the Firefox instance.
Reporter | ||
Comment 1•16 years ago
|
||
Assignee | ||
Comment 2•16 years ago
|
||
Fun busted frame tree... 1095 nsBoxFrame::GetContentInsertionFrame() 1096 { 1097 if (GetStateBits() & NS_STATE_BOX_WRAPS_KIDS_IN_BLOCK) 1098 return GetFirstChild(nsnull)->GetContentInsertionFrame(); The bit is set, but GetFirstChild(nsnull) is null.
Flags: blocking1.9?
Hrm, that means somebody removed my wrapper frame despite it being the content insertion frame!
Flags: blocking1.9? → blocking1.9+
Priority: -- → P4
Flags: wanted-next+
Flags: blocking1.9-
Updated•16 years ago
|
Flags: tracking1.9+
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
Reporter | ||
Comment 4•15 years ago
|
||
Still crashes on mozilla-central.
Reporter | ||
Updated•15 years ago
|
Whiteboard: [ccbr]
Reporter | ||
Updated•15 years ago
|
Whiteboard: [ccbr]
Assignee | ||
Comment 5•15 years ago
|
||
OK, so what happens here is this stack: #1 0x18e2b417 in nsListBoxBodyFrame::RemoveChildFrame (this=0x1f0af698, aState=@0xbfffcd8c, aFrame=0x1f0b0bc8) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/xul/base/src/nsListBoxBodyFrame.cpp:1518 #2 0x18e2c84b in nsListBoxBodyFrame::DoInternalPositionChanged (this=0x1f0af698, aUp=0, aDelta=1) at /Users/bzbarsky/mozilla/vanilla/mozilla/layout/xul/base/src/nsListBoxBodyFrame.cpp:939 #3 0x18e2f87b in nsListBoxBodyFrame::nsPositionChangedEvent::Run (this=0x1daa7280) at nsListBoxBodyFrame.h:178 DoInternalPositionChanged has this wonderful code: // We have scrolled so much that all of our current frames will // go off screen, so blow them all away. Weeee! nsIFrame *currBox = mFrames.FirstChild(); nsCSSFrameConstructor* fc = presContext->PresShell()->FrameConstructor(); fc->BeginUpdate(); while (currBox) { nsIFrame *nextBox = currBox->GetNextSibling(); RemoveChildFrame(state, currBox); currBox = nextBox; } fc->EndUpdate(); Of course in this case the only thing in mFrames is the wrapper block.
Assignee | ||
Comment 6•15 years ago
|
||
And have I mentioned how much I hate the listboxbody setup?
Assignee | ||
Comment 7•15 years ago
|
||
Attachment #390235 -
Flags: review?(dbaron)
Comment 8•15 years ago
|
||
Hmmm, is this going to cause problems with DestroyRows/ReverseDestroyRows because they call RemoveChildFrame exactly aRowsToLose times regardless if it removes a frame or not? Should RemoveChildFrame only be removing listitems?
Assignee | ||
Comment 9•15 years ago
|
||
Problems in the sense of misrendering? Probably! But since the code I added will only get hit if someone has random non-listitem crap inside your listbox, I don't really care about the resulting rendering. ;) As long as it's not adding a security problem, of course.
Comment on attachment 390235 [details] [diff] [review] Proposed fix I suppose it might be nice if we did the wrapping differently for listboxes: wrapping each non-box child in a block would probably work in this case. But I suppose this is ok given that we don't.
Attachment #390235 -
Flags: review?(dbaron) → review+
Assignee | ||
Comment 11•15 years ago
|
||
Pushed http://hg.mozilla.org/mozilla-central/rev/b38510c3949f Probably worth taking on the 1.9.1 branch...
Status: NEW → RESOLVED
Closed: 15 years ago
status1.9.1:
--- → ?
Flags: in-testsuite+
Resolution: --- → FIXED
Assignee | ||
Comment 13•15 years ago
|
||
It's been waiting on 1.9.1 branch triage for over a month now...
blocking1.9.1: --- → ?
Assignee | ||
Updated•15 years ago
|
Attachment #390235 -
Flags: approval1.9.1.4?
Comment 14•15 years ago
|
||
We don't triage status1.9.1 much. We look at blocking noms and approval requests regularly, and look at '?' when we get to the next release since those appear to be less urgent requests.
Updated•15 years ago
|
Comment 15•15 years ago
|
||
Comment on attachment 390235 [details] [diff] [review] Proposed fix Approved for 1.9.1.4, a=dveditz for release-drivers
Attachment #390235 -
Flags: approval1.9.1.4? → approval1.9.1.4+
Assignee | ||
Comment 16•15 years ago
|
||
Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/39907e147d42
Assignee | ||
Comment 17•15 years ago
|
||
Er, and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/395c09066d1c
Comment 18•15 years ago
|
||
Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.1.4pre) Gecko/20090930 Shiretoko/3.5.4pre and attached testcase. Crashes pretty reliably in 1.9.1.3, no crashes in 1.9.1.4.
Keywords: verified1.9.1
Updated•13 years ago
|
Crash Signature: [@ nsBoxFrame::GetContentInsertionFrame]
You need to log in
before you can comment on or make changes to this bug.
Description
•