370381 - nsContentUtils::SetNodeTextContent should check for single text node child

Status: RESOLVED INVALID

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

Description

[Jonathan Watt [:jwatt]]

I've noticed that setting .textContent on an element with a single text node child is a lot slower than setting .firstChild.nodeValue. It makes sense then that nsContentUtils::SetNodeTextContent should check for this condition and simply set the value of the existing text node rather than deleting it and creating a new one (with all the overhead that that implies).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Did you look at the SetNodeTextContent code?  It _does_ do that, unless told not to.

Now SetTextContent tells SetNodeTextContent not to attempt reuse of text nodes.

This text in the DOM spec seems to be relevant:

  textContent of type DOMString, introduced in DOM Level 3
    ... On setting, any possible children this node may have are removed
    and, if it the new string is not empty or null, replaced by a single
    Text node containing the string this attribute is set to. 

At least this is why SetTextContent doesn't reuse the node; see bug 352583 comment 0.

Comment 2

[Jonathan Watt [:jwatt]]

No, sorry. From observation of the effect I assumed that it didn't reuse, period, so I didn't look too closely. Was just quickly firing a bug into Mozilla so I'd remember to come back to this and only looked as far as the method name so I could use it in the bug title.

You're right. The spec does preclude us from doing this. Shame. -> invalid.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #2) 
> You're right. The spec does preclude us from doing this. Shame. -> invalid.
> 

Not really shame, IMO. Setting .textContent = null is the easiest way to
remove all child nodes.

Comment 4

[Jonathan Watt [:jwatt]]

It should be possible to check whether the value was null and still have that behavior as far as I can see (only reuse if a non-empty string was passed in). Anyway, it's more the perf difference that bothers me. If it wasn't for that I wouldn't really care.