Last Comment Bug 313799 - E4X: Assertion failure: !JSVAL_IS_PRIMITIVE(v), at jsxml.c:5558
: E4X: Assertion failure: !JSVAL_IS_PRIMITIVE(v), at jsxml.c:5558
: fixed1.8, js1.6
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: P1 normal (vote)
: mozilla1.8rc1
Assigned To: Brendan Eich [:brendan]
: Jason Orendorff [:jorendorff]
Depends on:
  Show dependency treegraph
Reported: 2005-10-25 14:12 PDT by Aiko
Modified: 2005-11-01 13:35 PST (History)
5 users (show)
bob: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Partial fix (1.35 KB, patch)
2005-10-25 16:00 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Better partial patch (1.18 KB, patch)
2005-10-25 16:59 PDT, Blake Kaplan (:mrbkap)
brendan: review+
Details | Diff | Splinter Review
XMLList testcase (780 bytes, text/html)
2005-10-27 08:15 PDT, Aiko
no flags Details
proposed fix (1.10 KB, patch)
2005-10-27 13:30 PDT, Brendan Eich [:brendan]
mrbkap: review+
Details | Diff | Splinter Review
rollup patch for 1.8 branch (1.26 KB, patch)
2005-10-27 14:52 PDT, Brendan Eich [:brendan]
brendan: review+
asa: approval1.8rc2+
Details | Diff | Splinter Review

Description Aiko 2005-10-25 14:12:42 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051025 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051025 Firefox/1.6a1

Calling a XMLList's child method causes this assertion:

Reproducible: Always

Steps to Reproduce:

Just in case you want a talkback id: TB11087432Y
Comment 1 Blake Kaplan (:mrbkap) 2005-10-25 16:00:22 PDT
Created attachment 200804 [details] [diff] [review]
Partial fix

This fixes the assertion botching and does something like the right thing, but by my reading of the spec. this is not compliant, since it seems that |<xml/>.child(0)| should return an empty XMLList, not void (or undefined) as we currently return.

I'm trying to figure out how to return a true empty XML list (which would fix the assertion, as well as the compliance issue).
Comment 2 Blake Kaplan (:mrbkap) 2005-10-25 16:59:15 PDT
Created attachment 200813 [details] [diff] [review]
Better partial patch

Shaver pointed out that using an if-continue was cleaner.
Comment 3 Brendan Eich [:brendan] 2005-10-25 18:04:51 PDT
D'oh!  I must have been typing while asleep.  I'll try for the full fix.

Comment 4 Bob Clary [:bc:] 2005-10-26 01:00:40 PDT
Checking in regress-313799.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-313799.js,v  <--  regress-313799.js
initial revision: 1.1
Comment 5 Brendan Eich [:brendan] 2005-10-26 01:17:43 PDT
Comment on attachment 200813 [details] [diff] [review]
Better partial patch

Note that this is the full fix.  There's nothing wrong with xml_child_helper, it is an inlined specialization of the spec ( 1(a-b)).

This is a nice safe crash fix, good for 1.8.

Comment 6 Brendan Eich [:brendan] 2005-10-26 01:18:52 PDT
Invalid (but known value, JSVAL_VOID) pointer defense, no risk.

Comment 7 Brendan Eich [:brendan] 2005-10-26 01:34:35 PDT
Fixed on trunk.

Comment 8 Aiko 2005-10-26 03:15:18 PDT
(In reply to comment #4)
> /cvsroot/mozilla/js/tests/e4x/Regress/regress-313799.js,v  <-- 

48           var val = <><t/></>.child(0);
51           TEST(2, 'xml',  typeof x);

|typeof val| or |var x =| ;-)
Comment 9 Bob Clary [:bc:] 2005-10-26 03:30:52 PDT
(In reply to comment #8)

doh! /me was testing in the shell using x but... Thanks Seno!

Checking in regress-313799.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-313799.js,v  <--  regress-313799.js
new revision: 1.2; previous revision: 1.1
Comment 10 Asa Dotzler [:asa] 2005-10-26 11:19:58 PDT
not going to lose a day with this but could become a ride-along if we're forced to respin by some other bug.
Comment 11 Brendan Eich [:brendan] 2005-10-26 15:48:29 PDT
See also bug 313949.

Comment 12 Brendan Eich [:brendan] 2005-10-26 19:57:17 PDT
Looking good on trunk, closing but tracking for rc2 ridealong.  Thanks,

Comment 13 Aiko 2005-10-27 08:15:10 PDT
Created attachment 200998 [details]
XMLList testcase

The assertion is certainly gone but the results of XMLList.prototype.child still look pretty broken to me.
Comment 14 Brendan Eich [:brendan] 2005-10-27 13:30:45 PDT
Created attachment 201037 [details] [diff] [review]
proposed fix

I'll let mrbkap mark r+ after he runs the testsuite ;-).

Comment 15 Brendan Eich [:brendan] 2005-10-27 14:48:59 PDT
Comment on attachment 201037 [details] [diff] [review]
proposed fix

Fixed on trunk.  I'll attach a roll-up branch patch.

Comment 16 Brendan Eich [:brendan] 2005-10-27 14:52:54 PDT
Created attachment 201054 [details] [diff] [review]
rollup patch for 1.8 branch

This is the new "ride-along" for rc2.

Comment 17 Asa Dotzler [:asa] 2005-10-31 14:47:08 PST
moving out to the 1.8 rc2 ride-along candidate list. We'll consider taking this if we do an RC2.
Comment 18 Brendan Eich [:brendan] 2005-10-31 17:18:51 PST
Fixed on the 1.8 branch.


Note You need to log in before you can comment on or make changes to this bug.