Closed Bug 313799 Opened 16 years ago Closed 16 years ago

E4X: Assertion failure: !JSVAL_IS_PRIMITIVE(v), at jsxml.c:5558

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.8rc1

People

(Reporter: Seno.Aiko, Assigned: brendan)

Details

(Keywords: fixed1.8, js1.6)

Attachments

(4 files, 1 obsolete file)

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:
  <><t/></>.child(0)

Reproducible: Always

Steps to Reproduce:




Just in case you want a talkback id: TB11087432Y
Assignee: general → brendan
Keywords: js1.6
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.8rc1
Assignee: brendan → mrbkap
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch Partial fix (obsolete) — Splinter Review
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).
Shaver pointed out that using an if-continue was cleaner.
Attachment #200804 - Attachment is obsolete: true
D'oh!  I must have been typing while asleep.  I'll try for the full fix.

/be
Assignee: mrbkap → brendan
Checking in regress-313799.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-313799.js,v  <--  regress-313799.js
initial revision: 1.1
done
Flags: testcase+
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 (13.4.4.6 1(a-b)).

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

/be
Attachment #200813 - Flags: review+
Attachment #200813 - Flags: approval1.8rc1?
Invalid (but known value, JSVAL_VOID) pointer defense, no risk.

/be
Flags: blocking1.8rc1?
Fixed on trunk.

/be
Status: NEW → ASSIGNED
(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 =| ;-)
(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
done
not going to lose a day with this but could become a ride-along if we're forced to respin by some other bug.
See also bug 313949.

/be
Looking good on trunk, closing but tracking for rc2 ridealong.  Thanks,

/be
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Attached file XMLList testcase
The assertion is certainly gone but the results of XMLList.prototype.child still look pretty broken to me.
Attached patch proposed fixSplinter Review
I'll let mrbkap mark r+ after he runs the testsuite ;-).

/be
Attachment #201037 - Flags: review?(mrbkap)
Attachment #201037 - Flags: review?(mrbkap) → review+
Comment on attachment 201037 [details] [diff] [review]
proposed fix

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

/be
Attachment #200813 - Flags: approval1.8rc1?
This is the new "ride-along" for rc2.

/be
Attachment #201054 - Flags: review+
Attachment #201054 - Flags: approval1.8rc1?
Status: RESOLVED → VERIFIED
moving out to the 1.8 rc2 ride-along candidate list. We'll consider taking this if we do an RC2.
Flags: blocking1.8rc1? → blocking1.8rc2?
Attachment #201054 - Flags: approval1.8rc1? → approval1.8rc1+
Attachment #201054 - Flags: approval1.8rc1+ → approval1.8rc2+
Fixed on the 1.8 branch.

/be
Keywords: fixed1.8
Flags: blocking1.8rc2?
You need to log in before you can comment on or make changes to this bug.