283861 - Want Text3.isElementContentWhitespace

Status: RESOLVED FIXED

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

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

To make it easier to figure out what content is ignorable from apps.

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: nsIDOM3Text

Comment 2

[Boris Zbarsky [:bzbarsky]]

Does this really not need to be done on character data nodes, etc?  Also, why
not implement getWholeText/replaceWholeText?  Those shouldn't be too hard,
should they?

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 175668 [details] [diff] [review]
nsIDOM3Text

r+sr=jst

Comment 4

[Alex Vincent [:WeirdAl]]

*** Bug 343906 has been marked as a duplicate of this bug. ***

Comment 5

[Alex Vincent [:WeirdAl]]

Comment on attachment 175668 [details] [diff] [review]
nsIDOM3Text

This is entirely new code, which has somehow sat around, not checked in, for a very long time...

Comment 6

[Boris Zbarsky [:bzbarsky]]

That patch looks bitrotted to me.  For one thing, the tearoff needs to implement cycle collection.

Comment 7

[Boris Zbarsky [:bzbarsky]]

And shipping something that will start throwing exceptions on enumeration at this stage in the game worries me in any case.

Comment 8

[Damon Sicore (:damons)]

Comment on attachment 175668 [details] [diff] [review]
nsIDOM3Text

I agree with Boris here.  I don't want to approve this until we know such an old patch is still valid.  Please test and re-request approval when done.

Comment 9

[Alex Vincent [:WeirdAl]]

Reading current code in mozilla-central against the patch, it looks like most of the code is already in place - the one major difference being nsITextContent doesn't exist anymore, and various methods are using nsIContent's TextIsOnlyWhitespace().

Given that, I am crafting an updated patch for this bug, but I would love it if someone would write a mochitest for it.

Comment 10

[Alex Vincent [:WeirdAl]]

Attachment: updated patch [Checkin: Comment 17]

Comment 11

[Alex Vincent [:WeirdAl]]

Whoops, I almost repeated caillon's mistake of not looking to get this checked in.  ;-)

Comment 12

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

Would be great to get some tests here - to prevent regressions.

Comment 13

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Possible mochitest

Not quits sure if this is what you guys were looking for and I don't have a build system setup to test this stuff out. Feel free to modify as you see fit.

Comment 14

[Alex Vincent [:WeirdAl]]

Ryan, the code returns true if the text node's whole contents are white space.

Comment 15

[Ryan Jones-Ward [:sciguyryan]]

(In reply to comment #14)
> Ryan, the code returns true if the text node's whole contents are white space.

That should be simpler to test for then. I'll re-write the tests.

Comment 16

[Ryan Jones-Ward [:sciguyryan]]

Attachment: Possible mochitest mark 2

This should be better then. Would appreciate it is someone could test and see if it actually does what it should though!

Comment 17

[Serge Gautherie (:sgautherie)]

Comment on attachment 341580 [details] [diff] [review]
updated patch
[Checkin: Comment 17]

http://hg.mozilla.org/mozilla-central/rev/4755421bae8f

Comment 18

[Peter Van der Beken [:peterv]]

Our DOM doesn't have a way to distinguish ignorable whitespace from regular whitespace. I'd rather we didn't implement this instead of implementing it differently from what the specification defines.

Comment 19

[Alex Vincent [:WeirdAl]]

peterv: do you mean we should back this patch out?

Comment 20

[Alex Vincent [:WeirdAl]]

I've been thinking about this lately.  I think the gist of comment 18 is that because we're not a validating XML processor, there's no definition of ignorable white space as far as we're concerned:
http://www.w3.org/TR/REC-xml/#sec-white-space

I don't see any way to improve on what we've got, even with regular expressions work being done for bug 345512.