[webkit-reviews] review denied: [Bug 5572] Implement textContent property : [Attachment 4542] Implement textContent

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied Anders Carlsson <andersca at mac.com>'s
request for review:
Bug 5572: Implement textContent property
http://bugzilla.opendarwin.org/show_bug.cgi?id=5572

Attachment 4542: Implement textContent
http://bugzilla.opendarwin.org/attachment.cgi?id=4542&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
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).