Last Comment Bug 312064 - E4X - XML Initializers with CDATA sections
: E4X - XML Initializers with CDATA sections
Status: RESOLVED FIXED
: fixed1.8, js1.6
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.8beta5
Assigned To: Brendan Eich [:brendan]
:
: Jason Orendorff [:jorendorff]
Mentors:
http://lxr.mozilla.org/mozilla/source...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-10-11 10:05 PDT by Bob Clary [:bc:]
Modified: 2005-10-26 12:51 PDT (History)
4 users (show)
brendan: blocking1.8rc1+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (1.24 KB, patch)
2005-10-11 22:36 PDT, Brendan Eich [:brendan]
brendan: review+
brendan: approval1.8rc1+
Details | Diff | Splinter Review

Description Bob Clary [:bc:] 2005-10-11 10:05:16 PDT
Branch and Trunk builds from this morning:

    Testcase e4x/Expressions/11.4.1-02.js failed Bug Number 257679
    [ Top of Page ]
    STATUS: 11.1.4 - XML Initializer
    STATUS: XML Initializer should accept single CDATA Section
    Failure messages were:
    FAILED!: Section 1 of test -
    FAILED!: Expected value:
    FAILED!: processing-instruction
    FAILED!: Actual value:
    FAILED!: text
    FAILED!:

    Testcase e4x/Expressions/11.4.1-05.js failed Bug Number 311157
    [ Top of Page ]
    Expected exit code 0, got 3
    Testcase terminated with signal 0
    Complete testcase output was:
    STATUS: Parsing - Comment hiding parsing/scanning
    BUGNUMBER: 311157
    STATUS: Comment-hiding compromise left E4X parsing/scanning inconsistent
    ./e4x/Expressions/11.4.1-05.js:48: SyntaxError: illegal XML character:
    ./e4x/Expressions/11.4.1-05.js:48: <parent xmlns=''><bye> <!CDATA[ duh ]]>
    ./e4x/Expressions/11.4.1-05.js:48: ..............................^
Comment 1 Brendan Eich [:brendan] 2005-10-11 11:01:55 PDT
Looks like you're missing a [ before the C in that CDATA section.

/be
Comment 2 Bob Clary [:bc:] 2005-10-11 11:10:30 PDT
look at the test source:

11.4.1-02: Ok, this is invalid. I forgot to change the expected value.


11.4.1-05: This is not invalid. Look at the test source.

var x = <bye> <![CDATA[ duh ]]>
    there </bye>;
Comment 3 Brendan Eich [:brendan] 2005-10-11 11:19:18 PDT
Oops!  Sorry.

/be
Comment 4 Brendan Eich [:brendan] 2005-10-11 22:36:16 PDT
Created attachment 199263 [details] [diff] [review]
fix

Argh, this is a stupid one-char patch to fix the bug.  Should go into 1.8
branch ASAP, it's trivial and obviously correct.

/be
Comment 5 Brendan Eich [:brendan] 2005-10-11 22:46:12 PDT
Comment on attachment 199263 [details] [diff] [review]
fix

mrbkap is looking over my shoulder and says "r=me" with a slight tone of
disgusted bemusement.

/be
Comment 6 Brendan Eich [:brendan] 2005-10-11 22:52:07 PDT
Fixed in both places.  Sigh.

/be
Comment 7 Bob Clary [:bc:] 2005-10-15 11:00:09 PDT
x = <bye> <![CDATA[ duh ]]>
    there </bye>

'[' + x.toString().replace(/ /g, '_') + ']' gives
[duhthere]

Is this correct?
Comment 8 Brendan Eich [:brendan] 2005-10-15 12:33:55 PDT
(In reply to comment #7)
> x = <bye> <![CDATA[ duh ]]>
>     there </bye>
> 
> '[' + x.toString().replace(/ /g, '_') + ']' gives
> [duhthere]
> 
> Is this correct?

Yes.  Set XML.ignoreWhitespace = false if you want to preserve insignificant
whitespace.  Also, x.toXMLString() gives the complete serialization, while
x.toString() is more compressed for readability, or something.

/be
Comment 9 Bob Clary [:bc:] 2005-10-16 08:30:52 PDT
(In reply to comment #8)

Ok, but the difference in behavior between 1) unadorned duh, 2) commented duh
and 3) cdata duh is bizarre. I'll fix the test and check it in later today.
Comment 10 Brendan Eich [:brendan] 2005-10-16 10:46:15 PDT
People should double-check Rhino (the actual gold standard, generally) and
ECMA-357 (a buggy recoding of Java code for Rhino into spec-language).

/be
Comment 11 Bob Clary [:bc:] 2005-10-16 14:56:28 PDT
(In reply to comment #10)
> People should double-check Rhino (the actual gold standard, generally) and

I will start doing that.

> ECMA-357 (a buggy recoding of Java code for Rhino into spec-language).

I did, and "10.1.1 ToString Applied to the XML Type" said nothing about
XML.ignoreWhitespace. Personally, I think ECMA 357 and E4X as a waste.
Comment 12 Brendan Eich [:brendan] 2005-10-16 19:27:11 PDT
(In reply to comment #11)
> I did, and "10.1.1 ToString Applied to the XML Type" said nothing about
> XML.ignoreWhitespace.

> Personally, I think ECMA 357 and E4X as a waste.

Think *of* them as a waste?  We shall see what users think.

The basic ideas seem sound (XML literals, built-in query operators, etc.).

My name is listed on ECMA-357 because mozilla.org joined ECMA just before it was
finished, and I helped compile errata while implementing and debugging.  Turns
out Macromedia was going through the same exercise.  The bulk of the errata are
not fixed in ECMA-357, but we have tried to fix as many as we can for the ISO
version that will become ECMA-357 Edition 3.

The moral here is to avoid standardizing based only on one implementation.  Or
to say it another way, don't standardize a spec derived from one implementation
without any other implementations being developed and then tested for usability
and interoperation, feeding errata and design flaws back into the spec draft.

I won't participate in any such exercise; with ECMA-357, the horse was out of
the barn by the time I showed up.

/be
Comment 13 Brendan Eich [:brendan] 2005-10-16 19:30:28 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > I did, and "10.1.1 ToString Applied to the XML Type" said nothing about
> > XML.ignoreWhitespace.

I forgot to reply to this part: XML.ignoreWhitespace affects input, not output,
so you shouldn't be looking at 10.1.1 or any To*String sub-spec.  Rather see
ECMA-357 10.3.2.1 MapInfoItemToXML, 2(c), and 13.4.3.4 XML.ignoreWhitespace.

/be
Comment 14 Aiko 2005-10-26 05:43:24 PDT
There is still a small problem with CDATA sections. Anything but the sequence ]]> should be allowed but the engine generates a syntax error if it encounters the sequence ]]

For example, |var x = <x><![CDATA[ ]] ]]></x>| should be ok and produce the same result as |var x = <x> ]] </x>|.
Comment 15 Brendan Eich [:brendan] 2005-10-26 11:58:48 PDT
(In reply to comment #14)
> There is still a small problem with CDATA sections. Anything but the sequence
> ]]> should be allowed but the engine generates a syntax error if it encounters
> the sequence ]]
> 
> For example, |var x = <x><![CDATA[ ]] ]]></x>| should be ok and produce the
> same result as |var x = <x> ]] </x>|.

Could you please file a new bug on this?  Thanks,

/be
Comment 16 Aiko 2005-10-26 12:51:50 PDT
(In reply to comment #15)
> Could you please file a new bug on this?  Thanks,

Sure, please see bug 313929.

Note You need to log in before you can comment on or make changes to this bug.