I'm trying to create a structure like: <div> a<br/> b<br/> c<br/> </div> However, if I do... js> var array = ["a","b","c"]; js> var myDiv = <div/>; js> for each (a in array) { myDiv.appendChild(a); myDiv.appendChild(<br/>); } <div> a <br/> <br>b</br> <br/> <br>c</br> <br/> </div>

step:- 1. navigate attachment "e4x_appendChild.html" 2. click Input Case "0" link 3. Click "test" button 4. repeat for links 1, 2, 3, 4 ....

jminta convinced me that calendar printing couldn't possibly work without this bug being fixed, so I looked into it. It appears to be an E4X spec bug that brendan translated into SpiderMonkey code. The root of the problem is XML.prototype.appendChild calls [[Get]](x, "*") to get the list of children. [[Get]] calls [[Append]] on the resulting XMLList for each child in the element, which sets the list's [[TargetProperty]] to the current kid's name, which, at the end of the function, is <br>. Then, we call into [[Put]], which creates a kid with the same name of targetProp, and appends to that. It seems to me that [[Get]] needs to ensure that when it returns, the returned list has a [[TargetProperty]] of whatever Get was asked for.

Does this break anything else?

Comment on attachment 230691 [details] [diff] [review] patch v1 Wow, this is a big bad erratum in the ECMA-357 spec (not that there haven't been others such before it!). Thanks for finding it! >+++ jsxml.c 26 Jul 2006 03:59:37 -0000 >@@ -4092,16 +4092,18 @@ retry: > } > } > > /* Common tail code for list and non-list cases. */ > js_LeaveLocalRootScopeWithResult(cx, (jsval) listobj); > if (!ok) > return JS_FALSE; > >+ JS_ASSERT(list); You don't need this, and it does no good since list is initialized non-null in all paths. >+ list->xml_targetprop = nameqn; This is the fix, but why didn't you remove the two redundant assignments identical this one that happen before the Append loops? Comment this erratum too, citing the bug #. /be

I was attempting to protect against uses of the list's targetProp before it was set early, but code inspection shows that I was being overly cautious.

Sorry, the comment in this patch is slightly better.

Comment on attachment 230742 [details] [diff] [review] patch v2.1 Yeah, the problem is that the spec sets [[TargetProperty]] too early too (in the ToXMLList call from [[Get]]). Kill that with a set just before returning and the bug is fixed, in spec and in code. So this looks great, nice comment. /be

Comment on attachment 230742 [details] [diff] [review] patch v2.1 I'm trying for 1.8.0.6 even though this is not a stability or security fix. Tell me if that's wrong and I won't do it again. This should be fixed in 1.8.1/Firefox 2. /be

Fix checked into trunk.

Note that having this in 1.8.0.x would be very nice for Lightning, as this bug currently bites its printing. Since Lightning needs to work in TB1.5, simply landing the fix in 1.8.1 isn't sufficient for that case.

Checking in regress-336921.js; /cvsroot/mozilla/js/tests/e4x/XML/regress-336921.js,v <-- regress-336921.js initial revision: 1.1

not going to block on this, but will take the patch for 1.8.1

Comment on attachment 230742 [details] [diff] [review] patch v2.1 a=dbaron on behalf of drivers for 1.8.1. Please land on MOZILLA_1_8_BRANCH and mark fixed1.8.1 once you have done so.

Fix checked into the 1.8 branch.

verified fixed 1.8.1, 1.9 windows/mac(ppc|tel)/linux 20060728

ditto the 1.8.1 triage team (comment 12)

Comment on attachment 230742 [details] [diff] [review] patch v2.1 approved for 1.8.0 branch, a=dveditz for drivers

Fixed on the 1.8.0 branch.

verified fixed 1.8.0.7 20060818 windows/mac*/linux

note to self: e4x/XML/regress-336921.js, result: FAILED TIMED OUT, firefox_1.8.1b2_2006082806_dbg, windows/prune, browser