Bug 5572 - Implement textContent property
Summary: Implement textContent property
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks: 3249
  Show dependency treegraph
 
Reported: 2005-10-31 10:44 PST by Anders Carlsson
Modified: 2019-02-06 09:03 PST (History)
1 user (show)

See Also:


Attachments
Implement textContent (18.57 KB, patch)
2005-10-31 10:45 PST, Anders Carlsson
darin: review-
Details | Formatted Diff | Diff
Address comments (18.51 KB, patch)
2005-11-01 00:42 PST, Anders Carlsson
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2005-10-31 10:44:15 PST
In DOM3, nodes have a textContent property. This should be implemented in order to pass more of the 
DOM3 test cases.
Comment 1 Anders Carlsson 2005-10-31 10:45:11 PST
Created attachment 4542 [details]
Implement textContent
Comment 2 Eric Seidel (no email) 2005-10-31 11:00:06 PST
Looks great!  I think i'd like a secondary approval from hyatt, darin or mjs though.
Comment 3 Darin Adler 2005-10-31 18:28:28 PST
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).
Comment 4 Anders Carlsson 2005-11-01 00:42:06 PST
(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.
Comment 5 Anders Carlsson 2005-11-01 00:42:41 PST
Created attachment 4547 [details]
Address comments
Comment 6 Darin Adler 2005-11-01 08:37:40 PST
Comment on attachment 4547 [details]
Address comments

r=me
Comment 7 Lucas Forschler 2019-02-06 09:03:28 PST
Mass moving XML DOM bugs to the "DOM" Component.