Closed Bug 415394 Opened 14 years ago Closed 12 years ago

Crash [@ nsBoxFrame::GetContentInsertionFrame] with <xul:listboxbody>


(Core :: XUL, defect, P4)




Tracking Status
blocking1.9.1 --- .4+
status1.9.1 --- .4-fixed


(Reporter: jruderman, Assigned: bzbarsky)


(Blocks 2 open bugs)


(Keywords: crash, testcase, verified1.9.1, Whiteboard: [ccbr])

Crash Data


(3 files)

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.
Attached file stack trace
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-
Flags: tracking1.9+
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
Still crashes on mozilla-central.
Whiteboard: [ccbr]
Whiteboard: [ccbr]
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();
      while (currBox) {
        nsIFrame *nextBox = currBox->GetNextSibling();
        RemoveChildFrame(state, currBox);
        currBox = nextBox;

Of course in this case the only thing in mFrames is the wrapper block.
And have I mentioned how much I hate the listboxbody setup?
Attached patch Proposed fixSplinter Review
Attachment #390235 - Flags: review?(dbaron)
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?
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+

Probably worth taking on the 1.9.1 branch...
Closed: 12 years ago
status1.9.1: --- → ?
Flags: in-testsuite+
Resolution: --- → FIXED
It's been waiting on 1.9.1 branch triage for over a month now...
blocking1.9.1: --- → ?
Attachment #390235 - Flags: approval1.9.1.4?
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.
Assignee: nobody → bzbarsky
blocking1.9.1: ? → .4+
Comment on attachment 390235 [details] [diff] [review]
Proposed fix

Approved for, a=dveditz for release-drivers
Attachment #390235 - Flags: approval1.9.1.4? → approval1.9.1.4+
Verified for 1.9.1 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv: Gecko/20090930 Shiretoko/3.5.4pre and attached testcase. Crashes pretty reliably in, no crashes in
Keywords: verified1.9.1
Crash Signature: [@ nsBoxFrame::GetContentInsertionFrame]
You need to log in before you can comment on or make changes to this bug.