Biju Reporter
	Description • 17 years ago

Is this a bug?

  a=<a><a>B</a></a>;
  a.a +=(<a a="A">a</a>).@a;
  s=[];
  for each(i in a) s.push(i.nodeKind());
  s.push(a);
  s.join('\n');


Gives ===>

  element
  attribute
  <a>
    <a>B</a>
    A
  </a>

I was expecting ==>

  element
  text
  <a>
    <a>B</a>
    A
  </a>

ie, a "text" instead of "attribute" as second array item


I also tried
  a.normalize(); 
after appending two attribute node consecutively 
to check whether it has an effect. But was none !!!


PS: I did not got time to try the same with XML or HTML DOM objects, 
if somebody found error please file a bug and cc me too.

	James Justin Harrell
	Comment 1 • 17 years ago

I believe the ECMA-357 specification contains an error such that the correctness of this behavior cannot be determined.

Section 11.6.3 clarified that these two are the same:
	a += b
	a = a + b

Section 11.4.1 specifies the behavior of the addition operator (+):
---
The production AdditiveExpression : AdditiveExpression + MultiplicativeExpression is evaluated as follows:
1. Let a be the result of evalutating AdditiveExpression
2. Let left = GetValue(a)
3. Let m be the result of evaluating MultiplicativeExpression
4. Let right = GetValue(m)
5. If (Type(left) ∈ {XML, XMLList}) and (Type(right) ∈ {XML, XMLList})
    a. Let list be a new XMLList
    b. Call the [[Append]] method of list with argument x
    c. Call the [[Append]] method of list with argument y
    d. Return list
---

Section 9.2.1.6 specifies the internal [[Append]] method of XMLList objects:
---
When the [[Append]] method of an XMLList object x is called with value V, the following steps are taken:
1. Let i = x.[[Length]]
2. Let n = 1
3. If Type(V) is XMLList,
    a. Let x.[[TargetObject]] = V.[[TargetObject]]
    b. Let x.[[TargetProperty]] = V.[[TargetProperty]]
    c. Let n = V.[[Length]]
    d. If n == 0, Return
    e. For j = 0 to V.[[Length]]-1, let x[i + j] = V[j]
    f.
4. Let x.[[Length]] = x.[[Length]] + n
5. Return
---

These steps given in section 9.2.1.6 are clearly wrong. For example, calling [[Append]] with a value that is not an XMLList will increment the [[Length]] of the list even though nothing is added. The empty step f implies that this section has not been given a very close look.

What are implementors to do in this situation? Is there a mailing list or something?

	Brendan Eich [:brendan]
	Comment 2 • 17 years ago

James: what version of ECMA-357 are you citing? The latest (see, e.g., here:

http://developer.mozilla.org/en/docs/JavaScript_Language_Resources

see also the E4X errata link there) has this under 9.2.1.6 [[Append]] (V), Semantics:

1. Let i = x.[[Length]]
2. Let n = 1 - 24 -
3. If Type(V) is XMLList,
   a. Let x.[[TargetObject]] = V.[[TargetObject]]
   b. Let x.[[TargetProperty]] = V.[[TargetProperty]]
   c. Let n = V.[[Length]]
   d. If n == 0, Return
   e. For j = 0 to V.[[Length]]-1, let x[i + j] = V[j]
4. Else [Note: Type(V) is XML]
   a. Let x.[[TargetObject]] = V.[[Parent]]
   b. If V.[[Class]] == "processing-instruction", let x.[[TargetProperty]] = null
   c. Else let x.[[TargetProperty]] = V.[[Name]]
   d. Let x[i] = V
5. Let x.[[Length]] = x.[[Length]] + n
6. Return

Of course there are lots of bugs in E4X's spec. But this bug is INVALID, it does not describe a known erratum. The testcase is overcomplicated; the essential step is the second input line, which can be simplified to:

js> (<t a="A"/>).@a.nodeKind()
attribute

Per ECMA-357, 9.1.1.1 XML [[Get]], asking for the value of the attribute name @a results in a single-item XMLList containing an attribute-kind XML node. This is not a text node, it's similar but with a distinguished "XML class" or "node kind".

/be

	Biju Reporter
	Comment 3 • 17 years ago

Brendan,

Is not it weird to have an attribute node inside a xml node at the place where standard xml child node, text node, comment, PI goes

also
    a.toXMLString() 
gives ==>

  <a>
    <a>B</a>
    A
  </a>

so when we do eval(a.toXMLString()) we dont get the exact structure back.
 

	Biju Reporter
	Comment 4 • 17 years ago

(In reply to comment #2)
> does not describe a known erratum. The testcase is overcomplicated; the
> essential step is the second input line, which can be simplified to:
> 
> js> (<t a="A"/>).@a.nodeKind()
> attribute

Brendan,

Please recheck, the second line is not equivalent to just getting nodeKind() of an attribute. It is getting nodeKind() of an attribute second XML after attaching it as child to the node of first XML.

I am rewriting same code for better clarity.

  a = <a>   <b> B </b>   </a>;

  x = <x  y = "Z"/>;

  a.b += x.@y;


  s=[];
  for each(i in a) s.push(i.nodeKind());
  s.push(a);
  s.join('\n');

	Brendan Eich [:brendan]
	Comment 5 • 17 years ago

If you want attributes to become text when appended, I'd like to say "file a bug on ECMA-357". We're following the spec, and it's true that the result does not round-trip via toXMLString/eval, but it does allow the attribute to be extracted later as an attribute (perverse, probably not a motivating use-case).

A few years ago, I'd reopen and fix the bug in SpiderMonkey, listing it as yet another erratum against ECMA-357. But I'm too busy with other things that I think are much more important than E4X. I'll reopen and leave this in the pool in case someone else is motivated.

E4X has been a disappointment, not least because the spec's pseudo-code, hacked from a Java prototype, uses fairly deep (in logic levels) "random logic" -- so it is inevitably full of bugs and holes or odd corners such as this one. If we ever revisit E4X in ECMA TG1 (TC39), we should look at XDuce etc. and use a proper type system, for which proofs and consistency checks can be mechanized.

/be

	James Justin Harrell
	Comment 6 • 17 years ago

Brendan Eich,

I was citing the second edition of ECMA-357. The latest errata I can find does not provide an updated algorithm for the [[Append]] method of XMLList objects. 

I agree that, according to the algorithm described in your post, Mozilla's current behavior is correct. However, I do not understand where that algorithm came from, as I cannot find it anywhere else.

	Brendan Eich [:brendan]
	Comment 7 • 17 years ago

James: no need for last names, I'm the only Brendan here ;-).

The copy we host of ECMA-357 is at http://www.mozilla.org/js/language/ECMA-357.pdf. Aha! This file is *not* the same as the one on Ecma's site:

http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-357.pdf

That one indeed has a badly broken 9.2.1.6 [[Append]] (V) spec. Cc'ing jeffdyer in case he has access to Word revbars or other change logs.

/be

	David Caldwell
	Comment 8 • 17 years ago

Brendan:  I've always relied on the one at ECMA's site, not even being aware of the one on the Mozilla site.  Glancing now at both (and without doing a global comparison), the most straightforward interpretation is that the one on the Mozilla site is the 1st edition and the one on ECMA's site is the 2nd edition.  Do you think this the case, or is it more complicated than that?  (And I guess I realize you may have been asking jeffdyer the same question...)

	Biju Reporter
	Comment 9 • 17 years ago

(In reply to comment #5)
> If you want attributes to become text when appended, I'd like to say "file a
> bug on ECMA-357". 

Brendan,

Let be know the URL/email-id to do so.

Did you got chance to raise Bug 356454 (a^=b^=a^=b SWAP) 
issue with ECMA TG1?
Let me know if I should do any thing for that too.

	Brendan Eich [:brendan]
	Comment 10 • 17 years ago

(In reply to comment #9)
> (In reply to comment #5)
> > If you want attributes to become text when appended, I'd like to say "file a
> > bug on ECMA-357". 
> 
> Brendan,
> 
> Let be know the URL/email-id to do so.

There really isn't a good way to record this, short of the bug here and a wiki-based errata sheet. See the main E4X bug (bug id e4x), which has an older attempt at a consolidated errata list attached.

> Did you got chance to raise Bug 356454 (a^=b^=a^=b SWAP) 
> issue with ECMA TG1?

A while ago -- apologies for not relaying the sparse response. For those not in the know, the bug is bug 356454. I will post an update there, although that bug is already RESOLVED INVALID.

/be

	Matthias Versen [:Matti]
	Comment 11 • 12 years ago

E4X will be removed again from Spidermonkey (bug 788293)