Closed Bug 664919 Opened 11 years ago Closed 11 years ago

Crash with document.normalize, DOMCharacterDataModified mutation event

Categories

(Core :: DOM: Core & HTML, defect)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla7

People

(Reporter: jruderman, Assigned: sicking)

References

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(4 files)

No description provided.
Attached file stack trace
We crash at here:

#2  0x0070d361 in nsGenericElement::Normalize (this=0x1b058240) at ../../../../mozilla/content/base/src/nsGenericElement.cpp:2791
2789              nsCOMPtr<nsIContent> sibling = GetChildAt(index + 1);
2790    
2791              if (sibling->NodeType() == nsIDOMNode::TEXT_NODE) {
(gdb) p sibling
$2 = {
  mRawPtr = 0x0
}

At this point index == 0 and count == 2.  We used to have a null-check on |sibling|, but that got removed in bug 659539 part 4....

Jonas, can we just defer mutation event firing till after normalize() is done so we don't have to worry about this sort of gunk?
Blocks: 659539
Keywords: regression
Yes! We should do that!
Assignee: nobody → jonas
hrm.. crap.. that's not trivial since we need to fire the DOMNodeRemoved events before removing any nodes :(

We could do a two-pass thing and first figure out which nodes are going to be removed, then do a second pass and actually remove them as we migrate their data to the nodes they are supposed to be merged with.
Attached patch Patch to fixSplinter Review
This can be simplified once we no longer have mutation events, but it'll be easier to simplify from this than from the current code anyway.
Attachment #541495 - Flags: review?(bzbarsky)
Comment on attachment 541495 [details] [diff] [review]
Patch to fix

r=me
Attachment #541495 - Flags: review?(bzbarsky) → review+
Attached patch interdiffSplinter Review
The previous patch failed on try. I need to reset canMerge in more code paths (i.e. even when the current node is marked as to-be-merged) if there is no following sibling.
Attachment #541760 - Flags: review?(bzbarsky)
Comment on attachment 541760 [details] [diff] [review]
interdiff

r=me
Attachment #541760 - Flags: review?(bzbarsky) → review+
http://hg.mozilla.org/mozilla-central/rev/2082bbcf2ce8
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Severity: normal → critical
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0) Gecko/20100101 Firefox/7.0

I runned the testcase on 7 beta build and Firefox did not crash.
Setting resolution to Verified Fixed.
Thanks.
Status: RESOLVED → VERIFIED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.