Bug 5572 – Implement textContent property

• Component: DOM
• Status: RESOLVED FIXED

[Anders Carlsson]

In DOM3, nodes have a textContent property. This should be implemented in order to pass more of the 
DOM3 test cases.

[Anders Carlsson]

Created attachment 4542 [details]
Implement textContent

[Eric Seidel (no email)]

Looks great!  I think i'd like a secondary approval from hyatt, darin or mjs though.

[Darin Adler]

Comment on attachment 4542 [details]
Implement textContent

The check of value->isNull() in TextContent makes me think two things:

1) I'd like a helper routine that creates a DOMString that handles the
JavaScript null case in this way. I'm sure there are other functions like this
one elsewhere in the DOM.

2) What about undefined? Should undefined set the text contents to "undefined"?
If not, then it should be isUndefinedOrNull() rather than isNull().

I think the code in NodeImpl::textContent and setTextContent cries out for
either a switch statement or a virtual function.

The code here is not the same as in setInnerText, but it should be. If they
shared code, then we would have the empty text check in both places
(setInnerText currently lacks it).

The check of text.isNull() should be a check of text.isEmpty() I believe
(unless I'm missing something).

[Anders Carlsson]

(In reply to comment #3)
> (From update of attachment 4542 [details] [edit])
> The check of value->isNull() in TextContent makes me think two things:
> 
> 1) I'd like a helper routine that creates a DOMString that handles the
> JavaScript null case in this way. I'm sure there are other functions like this
> one elsewhere in the DOM.

I can't find any other functions like this so I think I'll just let it be.

> 
> 2) What about undefined? Should undefined set the text contents to "undefined"?
> If not, then it should be isUndefinedOrNull() rather than isNull().
>
I think it makes more sense to set the text to "undefined". This is what Mozilla does.

> I think the code in NodeImpl::textContent and setTextContent cries out for
> either a switch statement or a virtual function.
>
Yeah, I've changed it to do that.
 
> The code here is not the same as in setInnerText, but it should be. If they
> shared code, then we would have the empty text check in both places
> (setInnerText currently lacks it).
>
They should not be the same. setInnerText should ideally do parsing (converting \n to <br> for 
example) but setTextContent shouldn't do that so I'd rather have them separate functions.
 
> The check of text.isNull() should be a check of text.isEmpty() I believe
> (unless I'm missing something).
> 
Yeah, that's correct. I've changed it.

[Anders Carlsson]

Created attachment 4547 [details]
Address comments

[Darin Adler]

Comment on attachment 4547 [details]
Address comments

r=me

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.