E4X: inconsistencies in the use of {} syntax

VERIFIED FIXED in mozilla1.9alpha1

Status

()

P1
normal
VERIFIED FIXED
13 years ago
12 years ago

People

(Reporter: BijuMailList, Assigned: brendan)

Tracking

(4 keywords)

Trunk
mozilla1.9alpha1
x86
Windows XP
js1.6, testcase, verified1.8.0.1, verified1.8.1
Points:
---
Bug Flags:
blocking1.8.1 +
blocking1.8.0.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

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

Comment 1

13 years ago
Created attachment 206858 [details]
e4x_inconsistency.html

Updated

13 years ago
Keywords: testcase
(Assignee)

Comment 2

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

/be
Assignee: general → brendan
Blocks: 309169
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: js1.6
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
(Assignee)

Comment 3

13 years ago
Created attachment 206872 [details] [diff] [review]
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
Attachment #206872 - Flags: superreview?(shaver)
Attachment #206872 - Flags: review?(mrbkap)
Comment on attachment 206872 [details] [diff] [review]
proposed fix

Sure.
Attachment #206872 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 5

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

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Flags: blocking1.8.0.1?
Resolution: --- → FIXED
(Reporter)

Comment 6

13 years ago
great..

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

Comment 7

13 years ago
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
Flags: testcase+
(Reporter)

Comment 8

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

Comment 9

13 years ago
More work needed -- thanks for keeping after this!

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

Updated

13 years ago
Status: REOPENED → ASSIGNED
(Reporter)

Comment 10

13 years ago
(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 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.
Attachment #206872 - Flags: superreview?(shaver) → superreview+
Would consider approving a baked patch.
Flags: blocking1.8.1?
Flags: blocking1.8.0.2?
Flags: blocking1.8.0.1?
Flags: blocking1.8.0.1-
(Assignee)

Comment 13

13 years ago
Comment on attachment 206872 [details] [diff] [review]
proposed fix

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

/be
Attachment #206872 - Flags: approval1.8.1?
Attachment #206872 - Flags: approval1.8.0.1?
(Assignee)

Comment 14

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

/be
Flags: blocking1.8.0.1- → blocking1.8.0.1?
Comment on attachment 206872 [details] [diff] [review]
proposed fix

a=dveditz for drivers, land quickly
Attachment #206872 - Flags: approval1.8.1?
Attachment #206872 - Flags: approval1.8.1+
Attachment #206872 - Flags: approval1.8.0.1?
Attachment #206872 - Flags: approval1.8.0.1+
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.2?
Flags: blocking1.8.0.1?
Flags: blocking1.8.0.1+
(Assignee)

Comment 16

13 years ago
Comment on attachment 206872 [details] [diff] [review]
proposed fix

I landed this patch on the 1.8* branches.

/be
(Assignee)

Comment 17

13 years ago
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
fixed per comment 5.
Fixed on branches per comment 16
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago13 years ago
Keywords: fixed1.8.0.1, fixed1.8.1
Resolution: --- → FIXED

Comment 19

13 years ago
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.
Keywords: fixed1.8.0.1 → verified1.8.0.1

Comment 20

13 years ago
v 2006-01-11 1.8.0.1, 1.8.1, trunk windows/linux/mac
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1 → verified1.8.1
(Assignee)

Comment 21

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

13 years ago
(In reply to comment #21)

will do.

Comment 23

13 years ago
filed bug 325750 
(Assignee)

Updated

12 years ago
Keywords: fixed1.8.1

Updated

12 years ago
Keywords: fixed1.8.1
You need to log in before you can comment on or make changes to this bug.