David Caldwell Reporter
	Description • 18 years ago

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: HEAD since 3-Feb-2006

In-development E4X implementation for Rhino fails this test.  It gives the output "there" with no whitespace surrounding it and no newline.  Isn't this right -- since ignoreComments = true and ignoreWhitespace = true, shouldn't the expression evaluate to the trimmed string?

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

	David Caldwell Reporter
	Comment 1 • 18 years ago

More precisely (I should have been more careful phrasing the above), the implementation should:

* ignore the whitespace before the comment because it's ignoring whitespace, 
* then should ignore the comment because it's ignoring comments, 
* then should ignore the whitespace before "there" because it's ignoring whitespace, 
* then pick up "there", 
* then ignore the whitespace after "there" because it's ignoring whitespace.  

So we should end up with an element with one child, the string "there" with no surrounding whitespace.  Is what I think right now, though I concede I may have missed something ...

	Jeff Walden [:Waldo]
	Comment 2 • 18 years ago

(In reply to comment #1)
> * ignore the whitespace before the comment because it's ignoring whitespace, 
> * then should ignore the comment because it's ignoring comments, 

You have me up to here.

> * then should ignore the whitespace before "there" because it's ignoring
> whitespace, 

This is where the problem lies.  XML.ignoreWhitespace only affects max-extent text nodes (i.e., neither followed by nor following another text node) which consist entirely of XMLWhitespace (CR/LF/SP/HT), according to MapInfoItemToXML, section 10.3.2.1.  The first space is discarded, the comment is discarded, but the last string of text is passed through as-is because it contains non-whitespace characters.

I briefly looked at Rhino source to see if the bug was obvious, and it looks to be something specific to Apache's XMLBeans stuff -- I'm guessing the whitespace handling isn't the same handling E4X specifies.

	David Caldwell Reporter
	Comment 3 • 18 years ago

Well, hang on, then.  What about this?

var x = 
<a>
  b
</a>

Now if ignoreWhitespace only affects max-extent text nodes which are ENTIRELY XMLWhitespace, then the correct value of x[0].toString() is "\n  b\n", right?  This would mean several other tests are broken (e.g., e4x/Types/9.1.1.1.js sections 1, 6, 9, 12, 20, 21, 22, 23, 24, 25).  Or am I missing something?

	Jeff Walden [:Waldo]
	Comment 4 • 18 years ago

(In reply to comment #3)
> Now if ignoreWhitespace only affects max-extent text nodes which are ENTIRELY
> XMLWhitespace, then the correct value of x[0].toString() is "\n  b\n", right?

Yes.  This is correct, and SpiderMonkey agrees.

> This would mean several other tests are broken (e.g., e4x/Types/9.1.1.1.js
> sections 1, 6, 9, 12, 20, 21, 22, 23, 24, 25).  Or am I missing something?

I think you're right.  Here's what seems to be a reduced testcase:

print(<a> b<c/></a> == <a>b<c/></a>); // == XML.ignoreWhitespace, not false

I see two possible places in the code that could be the problem:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsxml.c&rev=3.140&mark=1572#1567
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsxml.c&rev=3.140&mark=3357#3350

In both cases flags maps directly onto the XML.* settings.  The former seems more likely to be the problem in my reduction, whereas the latter may apply to some of the tests you mention (which I haven't fully investigated yet).  This looks to be a bit more wag-the-dog.

	Jeff Walden [:Waldo]
	Comment 5 • 18 years ago

I filed the described issues as bug 369394.  (By the way, the problem is the first line; the second one is actually in dead code, according to the reasoning given in the bug.)

The problems discovered recently in the E4X tests are starting to make me wonder just how accurate the E4X tests really are.  :-\

	David Caldwell Reporter
	Comment 6 • 18 years ago

I think I agree that the tests violate the spec in this case, but I (gingerly) wonder whether the spec isn't "wrong."  Meaning that this is an unintended result.  Given the semantics of the words "ignoreWhitespace" and the way pretty printing works in the spec and the way that a construct like <a>\n\tb\n</a> usually is interpreted in XML ... well, I'm just sayin' that's my 2 cents' worth.

I have many more test issues to come, so we should decide whether I'll be filing a bunch of bugs or how to move forward most effectively.  I no longer have much hope for our MozillaWiki strategy. :)  I should be able to quantify the issues pretty soon as I review the test failures individually and decide what I think of them. :)

I'll come visit you at 369394 now ... :)