The default bug view has changed. See this FAQ.

E4X - XML Initializers with CDATA sections

RESOLVED FIXED in mozilla1.8beta5

Status

()

Core
JavaScript Engine
P1
normal
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: bc, Assigned: brendan)

Tracking

({fixed1.8, js1.6})

Trunk
mozilla1.8beta5
fixed1.8, js1.6
Points:
---
Bug Flags:
blocking1.8rc1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

12 years ago
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: ..............................^
(Assignee)

Comment 1

12 years ago
Looks like you're missing a [ before the C in that CDATA section.

/be
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → INVALID
(Reporter)

Comment 2

12 years ago
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>;
(Assignee)

Comment 3

12 years ago
Oops!  Sorry.

/be
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
(Assignee)

Updated

12 years ago
Assignee: general → brendan
Status: REOPENED → NEW
Keywords: js1.6
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.8beta5
(Assignee)

Comment 4

12 years ago
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
Attachment #199263 - Flags: review?(mrbkap)
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
Flags: blocking1.8rc1+
(Assignee)

Comment 5

12 years ago
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
Attachment #199263 - Flags: review?(mrbkap)
Attachment #199263 - Flags: review+
Attachment #199263 - Flags: approval1.8rc1+
(Assignee)

Comment 6

12 years ago
Fixed in both places.  Sigh.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
(Reporter)

Updated

12 years ago
Flags: testcase+
(Reporter)

Comment 7

12 years ago
x = <bye> <![CDATA[ duh ]]>
    there </bye>

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

Is this correct?
(Assignee)

Comment 8

12 years ago
(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
(Reporter)

Comment 9

12 years ago
(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.
(Assignee)

Comment 10

12 years ago
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
(Reporter)

Comment 11

12 years ago
(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.
(Assignee)

Comment 12

12 years ago
(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
(Assignee)

Comment 13

12 years ago
(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

12 years ago
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>|.
(Assignee)

Comment 15

12 years ago
(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

12 years ago
(In reply to comment #15)
> Could you please file a new bug on this?  Thanks,

Sure, please see bug 313929.
You need to log in before you can comment on or make changes to this bug.