Closed Bug 309732 Opened 19 years ago Closed 19 years ago

Crash moving <treechildren> out of tree, then moving <treeitem> out of it [@ nsTreeContentView::InsertRowFor]

Categories

(Core :: XUL, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED

People

(Reporter: jruderman, Assigned: neil)

References

Details

(Keywords: crash, testcase)

Crash Data

Attachments

(3 files)

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20050922 Firefox/1.6a1 Same signature as bug 307298, new testcase, crashes in a build that contains the fix for bug 307298.
0 nsTreeContentView::InsertRowFor(nsIContent*, nsIContent*) + 72 1 nsTreeContentView::ContentInserted(nsIDocument*, nsIContent*, nsIContent*, int) + 888 2 nsDocument::ContentAppended(nsIContent*, int) + 116 3 nsGenericElement::InsertChildAt(nsIContent*, unsigned, int) + 504 4 nsGenericElement::InsertBefore(nsIDOMNode*, nsIDOMNode*, nsIDOMNode**) + 1640 5 _XPTC_InvokeByIndex + 216 6 XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) + 2508 7 XPC_WN_CallMethod(JSContext*, JSObject*, unsigned, long*, long*) + 220 8 js_Invoke + 1768 9 js_Interpret + 28824 ...
OS: MacOS X → All
Hardware: Macintosh → All
Attached file Alternate test case (deleted) —
This crashes for the same reason, in this case mRoot is null which means that the root element check fails completely.
Attached patch Proposed patch (deleted) — Splinter Review
Don't listen to document notifications after our tree body's been removed. When a tree body is created it calls SetTree to set the view up (again).
Assignee: Jan.Varga → neil.parkwaycc.co.uk
Status: NEW → ASSIGNED
Attachment #197293 - Flags: superreview?(bzbarsky)
Attachment #197293 - Flags: review?(Jan.Varga)
Hmm, it would be better to put that observer removal somewhere else. Take a look at DocumentWillBeDestroyed(), it removes the observer and calls ClearRows() A new inline method would be nice, e.g. RemoveDocumentObserver()
(In reply to comment #5) >Hmm, it would be better to put that observer removal somewhere else. Take a >look at DocumentWillBeDestroyed(), it removes the observer and calls ClearRows() I'm just moving the code, not duplicating it... or have I misunderstood you?
Attachment #197293 - Flags: superreview?(bzbarsky) → superreview+
Ah, sorry, you're right. r=varga
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Verified FIXED using SeaMonkey 1.1a: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050926 Mozilla/1.0 with the two testcases here.
Status: RESOLVED → VERIFIED
Attachment #197293 - Flags: review?(Jan.Varga) → review+
Crashtests checked in.
Flags: in-testsuite+
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: xptoolkit.trees → xptoolkit.widgets
Crash Signature: [@ nsTreeContentView::InsertRowFor]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: