Closed
Bug 312064
Opened 19 years ago
Closed 19 years ago
E4X - XML Initializers with CDATA sections
Categories
(Core :: JavaScript Engine, defect, P1)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla1.8beta5
People
(Reporter: bc, Assigned: brendan)
References
()
Details
(Keywords: fixed1.8, js1.6)
Attachments
(1 file)
1.24 KB,
patch
|
brendan
:
review+
brendan
:
approval1.8rc1+
|
Details | Diff | Splinter Review |
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•19 years ago
|
||
Looks like you're missing a [ before the C in that CDATA section. /be
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → INVALID
Reporter | ||
Comment 2•19 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•19 years ago
|
||
Oops! Sorry. /be
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Assignee | ||
Updated•19 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•19 years ago
|
||
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•19 years ago
|
Status: NEW → ASSIGNED
Flags: blocking1.8rc1+
Assignee | ||
Comment 5•19 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•19 years ago
|
||
Fixed in both places. Sigh. /be
Status: ASSIGNED → RESOLVED
Closed: 19 years ago → 19 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Reporter | ||
Updated•19 years ago
|
Flags: testcase+
Reporter | ||
Comment 7•19 years ago
|
||
x = <bye> <![CDATA[ duh ]]> there </bye> '[' + x.toString().replace(/ /g, '_') + ']' gives [duhthere] Is this correct?
Assignee | ||
Comment 8•19 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•19 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•19 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•19 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•19 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•19 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•19 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•19 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•19 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.
Description
•