Closed Bug 563980 Opened 10 years ago Closed 10 years ago

random "ASSERTION: Please remove this from the document properly" during crashtest

Categories

(Core :: DOM: Editor, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: dbaron, Assigned: dbaron)

Details

Attachments

(2 files)

The most common random assertion remaining during crashtest seems to be occurrences of "ASSERTION: Please remove this from the document properly".  There's on existing bug on this, bug 475132, but I'm filing this one separately for investigation of these random assertions.

I plan to land some debugging code to print information about the elements with this problem in the hopes that it will help us figure out the problem.

I suspect it could be due to a cycle collection unlink method that fails to call UnbindFromTree.
Component: DOM → Editor
QA Contact: general → editor
My guess is that we're hitting one of the early returns (other than the first) in nsHTMLEditor::HideResizers / nsHTMLEditor::HideGrabber / nsHTMLEditor::HideInlineTableEditingUI ; likely either (a) the null-check on the pres shell or (b) the parent content / root (because nsEditor's Unlink clears mRootElement without calling the anonymous content clearing in ~nsHTMLEditor).
It seems to be the no pres shell case.
And it looks like the code is already able to handle a null pres shell so we should just be able to stop null-checking it.
Attached patch patchSplinter Review
I think this should be fine; I just pushed it to try to make sure it doesn't do anything bad.
Attachment #443763 - Flags: review?(ehsan)
(Some of the other early returns are also potentially scary, but they don't seem to be the present problem...)
Comment on attachment 443763 [details] [diff] [review]
patch

Makes sense to me.

Have you tried attachment 358544 [details] with this patch?
Attachment #443763 - Flags: review?(ehsan) → review+
I couldn't reproduce this assertion on that attachment without the patch... so I hadn't.  But I checked that I still see the other three assertions even with this patch (though not all the time).
Debugging code backed out:
http://hg.mozilla.org/mozilla-central/rev/166e9ae1bb5d
and patch landed:
http://hg.mozilla.org/mozilla-central/rev/41ffa62580c4
Assignee: nobody → dbaron
Status: NEW → RESOLVED
Closed: 10 years ago
Priority: -- → P3
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
You need to log in before you can comment on or make changes to this bug.