Closed Bug 813919 Opened 7 years ago Closed 7 years ago

"Assertion failure: aFirstNewContent->IsNodeOfType(nsINode::eTEXT)"

Categories

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

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: smaug)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(4 files)

Assertion failure: aFirstNewContent->IsNodeOfType(nsINode::eTEXT), at content/base/src/nsRange.cpp:562

This assertion is part of code added in bug 803924.
Attached file stack
Lovely.
Totally forgot comment node.
Mats, could you look at this? If not, assign to me.
Assignee: nobody → matspal
Though, I believe it is just the assertion check which is wrong.
Bah, I started to look at this. Taking.
Assignee: matspal → bugs
Or, no. Need to figure out what the spec says about this.
Assignee: bugs → nobody
In Gecko "data nodes" have SplitData, and deleteContents/extractContents end up using that
if a data node needs to be split to two.
I *think* spec should have similar split handling, but need to investigate this some more.
Hmm, the offset change because of split handling should be just temporary Gecko implementation detail in this case.
Assignee: nobody → bugs
Mats, I think we should just fix the assertion to check eDATA_NODE, and add plenty of tests
for comment and pi. Sounds right?
Yeah, I think the code is correct, it's just that the assertion is too strict.
So I think we should change it to something like this:
MOZ_ASSERT(aFirstNewContent->IsNodeOfType(nsINode::eTEXT) ||
           aFirstNewContent->IsNodeOfType(nsINode::eCOMMENT) ||
           aFirstNewContent->IsNodeOfType(nsINode::ePROCESSING_INSTRUCTION));

We do handle PI although the spec only says we should split Text and Comment.
File a separate bug on that?
Yes, eDATA_NODE should work too, until we decide what to do with PI.
Didn't we change PI handling explicitly to be more consistent with
text and comment, and someone filed a spec bug... or was it just about comment.
Can't remember now.
Same problem in nsRange::ContentInserted

  MOZ_ASSERT(aChild->IsNodeOfType(nsINode::eTEXT));

Assertion failure: aChild->IsNodeOfType(nsINode::eTEXT), at content/base/src/nsRange.cpp:599
Attached patch patchSplinter Review
Sorry, took some time. Was busy with other stuff.
Attachment #684695 - Flags: review?(matspal)
Comment on attachment 684695 [details] [diff] [review]
patch

Looks good, r=mats

Please file a follow-up bug on what to do about PI.
Attachment #684695 - Flags: review?(matspal) → review+
https://hg.mozilla.org/mozilla-central/rev/e685ffb06975

Followup https://bugzilla.mozilla.org/show_bug.cgi?id=814706
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Component: DOM: Traversal-Range → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.