321549 - E4X: inconsistencies in the use of {} syntax

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Biju]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051219 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051219 Firefox/1.6a1

During the testing of  bug 318922 many inconsistencies where found in the use of {} expression value substitution in E4X syntax in the Mozilla implementation.


Example

// test 1

b='b';
a=<a xmlns:p='http://a.uri/'>
  <p:b{b}>x</p:bb>
</a>;


is valid per mozilla implementation

But


// test 2

b='b';
a=<a xmlns:p='http://a.uri/'>
  <p:{b}b>x</p:bb>
</a>;


gives
SyntaxError: invalid XML qualified name



Reproducible: Always

Steps to Reproduce:
go thru attachment e4x_inconsistancy.html


Actual Results:  
some INVALID SYNTAX other VALID SYNTAX



Expected Results:  
Expect for the last test, all of the should be either INVALID SYNTAX or all should be VALID SYNTAX to avoid any inconsistency.

Comment 1

[Biju]

Attachment: e4x_inconsistency.html

Comment 2

[Brendan Eich [:brendan]]

Thanks for this report -- it's an easily fixed bug.

/be

Comment 3

[Brendan Eich [:brendan]]

Attachment: proposed fix

Note use of comma operator to unobfuscate the dependent nested boolean expression. If (xml-only-mode or next != '{') then insist on nextc being a valid NC.

/be

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 206872 [details] [diff] [review]
proposed fix

Sure.

Comment 5

[Brendan Eich [:brendan]]

Fixed on trunk.  Waiting for shaver's sr= and some baking before nominating for 1.8.0 branch.

/be

Comment 6

[Biju]

great..

I also see another inconsistency in E4X see bug 321564#c3

Comment 7

[Bob Clary [:bc] (inactive)]

Checking in 11.4.1-07.js;
/cvsroot/mozilla/js/tests/e4x/Expressions/11.4.1-07.js,v  <--  11.4.1-07.js
initial revision: 1.1

Comment 8

[Biju]

tested on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051227 Firefox/1.6a1

Looks Better....

Issues

// test 3

u='http://a.uri/';
p='p';
a=<a xmlns:pp={u}>
  <{p}p:b>x</pp:b>
</a>;



Valid !!!



// test 4

u='http://a.uri/';
p='p';
a=<a xmlns:pp={u}>
  <p{p}:b>x</pp:b>
</a>;


SyntaxError: illegal XML character



// test 5

u='http://a.uri/';
p='pp';
a=<a xmlns:pp={u}>
  <{p}:b>x</pp:b>
</a>;


SyntaxError: illegal XML character


6 : valid
7, 8: invalid !!!

13, 15, 16, 17, 18, 19 : invalid 
(let me know whether those test are also valid)

Comment 9

[Brendan Eich [:brendan]]

More work needed -- thanks for keeping after this!

/be

Comment 10

[Biju]

(In reply to comment #9)
> More work needed -- thanks for keeping after this!

No probs. 
As far as I concern {} sutff is working as required for all most all possible common use. 
And any software always will have the last bug.

One question: Will javascript will be one day available as an OS shell script language like AWK, perl, tcl, Python?

Hey there is also another good use for {} syntax javascript. 
Using it one can create a Mail Merge like stuff. 


NAME = "Biju"; STREET = "123 Blue heaven";
CITY = "The Atlantis"; STATE = "ZA"; ZIP = "97887";

a = <>Dear {NAME}:

Our records show that your address is:

  {STREET}
  {CITY}, {STATE} {ZIP}

If this is incorrect, please let us know 
immediate so that we can update our database.
And send all junk mails to you every week.

Thanks,
Bob
www.TheJunkMailSender.com
</>;


a;

Comment 11

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 206872 [details] [diff] [review]
proposed fix

sr=shaver, now that I've unwound the state space in my head a little.  Ah, good to be back.

Comment 12

[Daniel Veditz [:dveditz]]

Would consider approving a baked patch.

Comment 13

[Brendan Eich [:brendan]]

Comment on attachment 206872 [details] [diff] [review]
proposed fix

This has been baking on the trunk since 12/26.

/be

Comment 14

[Brendan Eich [:brendan]]

Patch makes things better only, not worse (for workarounds, e.g.).

/be

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 206872 [details] [diff] [review]
proposed fix

a=dveditz for drivers, land quickly

Comment 16

[Brendan Eich [:brendan]]

Comment on attachment 206872 [details] [diff] [review]
proposed fix

I landed this patch on the 1.8* branches.

/be

Comment 17

[Brendan Eich [:brendan]]

Really should spin out the remaining problems into a new bug report at this point, since we've landed the patch.  Biju, can you do that?  Thanks,

/be

Comment 18

[Daniel Veditz [:dveditz]]

fixed per comment 5.
Fixed on branches per comment 16

Comment 19

[Jay Patel [:jay]]

v.fixed on 1.8.0.1 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1 using testcases in comment #1, results match those found to be acceptable in comment #8.

Comment 20

[Bob Clary [:bc] (inactive)]

v 2006-01-11 1.8.0.1, 1.8.1, trunk windows/linux/mac

Comment 21

[Brendan Eich [:brendan]]

(In reply to comment #17)
> Really should spin out the remaining problems into a new bug report at this
> point, since we've landed the patch.  Biju, can you do that?  Thanks,

We really need the other testcases unfixed by this bug's patch spun out into another bug.  Bob, could you please do that?  Thanks,

/be

Comment 22

[Bob Clary [:bc] (inactive)]

(In reply to comment #21)

will do.

Comment 23

[Bob Clary [:bc] (inactive)]

filed bug 325750