Closed Bug 433429 Opened 17 years ago Closed 16 years ago

Crash [@ nsListBoxBodyFrame::OnContentRemoved]

Categories

(Core :: XUL, defect)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: smaug)

References

Details

(Keywords: crash, testcase, verified1.9.0.4, Whiteboard: [sg:critical?])

Crash Data

Attachments

(3 files)

Loading the testcase makes Mac trunk debug Firefox crash [@ nsListBoxBodyFrame::OnContentRemoved]. mContent and mRowCount are both 0xdddddddd, so I'm guessing |this| is a deleted nsListBoxBodyFrame.
Attached file stack trace (deleted) —
We're ending up with an nsListBoxObject that somehow holds a pointer to a dead mListBoxBody. At a guess, we have multiple nsListBoxObjects for the same node. Or something. Because nsListBoxBodyFrame::Destroy notifies about itself going away...
Can't reproduce on Linux. Error console shows: Security Error: Content at https://bugzilla.mozilla.org/attachment.cgi?id=320623 may not load data from data:text/xml,....
There is a pref that enables data urls for xbl, see discussion in bug 379644. I don't know the pref's name.
Sorry, I forgot to mention that you have to set layout.debug.enable_data_xbl to true for the testcase to "work". (See bug 379959.)
Whiteboard: [sg:critical?]
So we have nsListBoxObjects for two different <listbox>es, but one of them seems to find the listboxbody of the other one. Should FindBodyContent bail out if it runs into a listbox? roc, you know this code, right?
Blocks: 417699
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
IMO this is enough. Just ensure that there is one-to-one mapping between boxobject and frame. If someone creates strange XUL where there are several listbox elements but only one listboxbody, the first created listboxobject gets access to the frame - sort of random, but we can't support that kind of XUL structure anyway. So better to prevent the crash.
Assignee: nobody → Olli.Pettay
Status: NEW → ASSIGNED
Attachment #334304 - Flags: superreview?(bzbarsky)
Attachment #334304 - Flags: review?(bzbarsky)
Comment on attachment 334304 [details] [diff] [review] ensure that only one box object is bound to listboxbodyframe Can we check GetType() too, just to be safe? Just static-casting interface pointers like that worries me.
Attachment #334304 - Flags: superreview?(bzbarsky)
Attachment #334304 - Flags: superreview+
Attachment #334304 - Flags: review?(bzbarsky)
Attachment #334304 - Flags: review+
Well, if would be very strange if any other frame implemented nsIListBoxObject. The whole setup is a bit strange; nsListBoxObject implements nsIListBoxObject and so does nsListBoxBodyFrame. The idea is just that nsListBoxObject can forward most of the method calls easily to nsListBoxBodyFrame.
Pushed the patch
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment on attachment 334304 [details] [diff] [review] ensure that only one box object is bound to listboxbodyframe Is this too late for 1.9.0.2?
Attachment #334304 - Flags: approval1.9.0.2?
The testcase WFM on 1.8.
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x-
Comment on attachment 334304 [details] [diff] [review] ensure that only one box object is bound to listboxbodyframe Technically not too late, but I'd rather get this in 1.9.0.3 so it has more time to bake. We should also get a crash test for this landed after it lands on the branch.
Attachment #334304 - Flags: approval1.9.0.2? → approval1.9.0.3?
Flags: in-testsuite?
Comment on attachment 334304 [details] [diff] [review] ensure that only one box object is bound to listboxbodyframe Approved for 1.9.0.4, a=dveditz for release-drivers
Attachment #334304 - Flags: approval1.9.0.4? → approval1.9.0.4+
Keywords: fixed1.9.0.4
This is still reproducing in the current 1.9.0.4 build (Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.4pre) Gecko/2008102304 GranParadiso/3.0.4pre). Did this actually get checked in?
Checked in 2008-10-23 10:10, do you have the right build?
No, probably not. :-) I *assumed* that it had been checked in during September so I didn't even check. I'll check with tomorrow's daily. Thanks!
Verified for 1.9.0.4 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.4pre) Gecko/2008102704 GranParadiso/3.0.4pre. Verified for trunk with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081026 Minefield/3.1b2pre.
Status: RESOLVED → VERIFIED
Group: core-security
This test uses data: URLs for XBL bindings, so I think it can't be added to the crashtest suite as-is.
It could be added to mochitest, right? Or heck, it could use relative URLs for the bindings... would that not work in reftest?
Crash Signature: [@ nsListBoxBodyFrame::OnContentRemoved]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: