352097 - E4X XMLPI that does not contain embedded spaces decompiles with extra space

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

js> function() { var y = <?foo?> }   
function () {
    var y = <?foo ?>;
}

js> function() { var y = <?foo ?> }
function () {
    var y = <?foo  ?>;
}

Comment 1

[Robert Sayre]

Hmm, tricky.

http://www.w3.org/TR/xml-c14n#Example-OutsideDoc

Comment 2

[Jeff Walden [:Waldo]]

Attachment: Patch

Removing the added space and eliminating trailing spaces for PIs without data requires two changes: first, only print the space when the PI's data has non-zero length (the jsopcode.c change), and second, trim the tokenbuf containing the target so that we don't associate ("f ", "") with the PI denoted by <?f ?>, making sure to do so *after* js_AtomizeChars (the jsscan.c change).

I also folded bug 352103's fix (|if (targetLength==0) goto bad_xml_markup;|) since it was so small.

Comment 3

[Jeff Walden [:Waldo]]

Attachment: Testcase

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 237812 [details] [diff] [review]
Patch

Local style prefers *rval = = '\0' to !*rval. r=me with that, and nominate the updated patch for 1.8.1. Thanks,

/be

Comment 5

[Jeff Walden (gone starting June 8)]

Attachment: Fixes nit

Checked in on trunk.

Small, tightly-contained patch to make XML PI decompilation saner and to reject a certain type of invalid XML PI in E4X processing...

Comment 6

[Mike Schroepfer]

Comment on attachment 237829 [details] [diff] [review]
Fixes nit

a=schrep

Comment 7

[Jeff Walden [:Waldo]]

Fixed on branch.

Comment 8

[Bob Clary [:bc] (inactive)]

(In reply to comment #3)
> Created an attachment (id=237816) [edit]
> Testcase
> 

Checking in regress-352097.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-352097.js,v  <--  regress-352097.js
initial revision: 1.1
done

thanks!

Comment 9

[Bob Clary [:bc] (inactive)]

verified fixed 1.8 20060914 windows/linux 1.9 20060914 windows/mac*/linux