Closed Bug 514300 Opened 11 years ago Closed 11 years ago

Crash [@ FindBodyContent] with xul:listbox, XBL

Categories

(Core :: XUL, defect)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(4 files, 1 obsolete file)

No description provided.
Attached file stack trace
Severity: normal → critical
In FindBodyContent, we have aParent->IsInDoc() true, but childContent->IsInDoc() is false, so we crash when trying to get the binding manager off childContent->GetDocument() in FindBodyContent when recursing on it.

aParent above is a <xul:listitem> and the child is <html:span>.  It looks like this:

 document.getElementById('listbox').removeChild(document.getElementById('span'))

doesn't actually remove the <span> from the XBL child list for the <listitem> for some reason.
The xbl part is the usual mess where nsBindingManager::GetXBLChildNodesInternal will return the "other" content list, the one that doesn't get updated, if the GetAnonymousNodesInternal return value has length 0.
OK, I'm sick of the GetXBLChildNodes thing.  Patch coming up for it; that patch also fixes bug 415301 and bug 495354 by doing bug 495354 comment 9 item 1.

As a separate note, GetDocument here is wrong.  It should be GetOwnerDoc(), and that would also fix the crash.  I'll put up a patch for that too.
Blocks: 415301, 495354
Attachment #420005 - Flags: review?(jonas)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #420006 - Flags: review?(jonas)
Attachment #420006 - Attachment is obsolete: true
Attachment #420035 - Flags: review?(jonas)
Attachment #420006 - Flags: review?(jonas)
Blocks: 538070
Pushed:
  http://hg.mozilla.org/mozilla-central/rev/7963a18d3373
  http://hg.mozilla.org/mozilla-central/rev/710cca46f64d

Jonas, Jesse, what do you think of taking this on 1.9.2 once it's baked a bit?
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
I would like that, given the kinds of assertions it fixes.  How much of a behavior change is it?
And pushed http://hg.mozilla.org/mozilla-central/rev/52081e8986b7 to actually add the reftests to reftest.list.

As far as behavior change...  I think this should be pretty safe.  Offhand I can't think of a situation where it would break something (else I wouldn't have asked).  But this code is... finicky.  :(
I think we should take this on 1.9.2 after some m-c baking.
Crash Signature: [@ FindBodyContent]
You need to log in before you can comment on or make changes to this bug.