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

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Mon Oct 31 18:28:28 PST 2005

Darin Adler <darin at apple.com> has denied Anders Carlsson <andersca at mac.com>'s
request for review:
Bug 5572: Implement textContent property

Attachment 4542: Implement textContent

------- 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).

More information about the webkit-reviews mailing list