The default bug view has changed. See this FAQ.

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

VERIFIED FIXED in mozilla1.8rc1

Status

()

Core
JavaScript Engine
P1
normal
VERIFIED FIXED
12 years ago
12 years ago

People

(Reporter: Aiko, Assigned: brendan)

Tracking

({fixed1.8, js1.6})

Trunk
mozilla1.8rc1
fixed1.8, js1.6
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

12 years ago
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)

Updated

12 years ago
Assignee: general → brendan
Keywords: js1.6
OS: Windows XP → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.8rc1

Updated

12 years ago
Assignee: brendan → mrbkap
Status: UNCONFIRMED → NEW
Ever confirmed: true
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).
Created attachment 200813 [details] [diff] [review]
Better partial patch

Shaver pointed out that using an if-continue was cleaner.
Attachment #200804 - Attachment is obsolete: true
(Assignee)

Comment 3

12 years ago
D'oh!  I must have been typing while asleep.  I'll try for the full fix.

/be
Assignee: mrbkap → brendan

Comment 4

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

Comment 5

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

Comment 6

12 years ago
Invalid (but known value, JSVAL_VOID) pointer defense, no risk.

/be
Flags: blocking1.8rc1?
(Assignee)

Comment 7

12 years ago
Fixed on trunk.

/be
Status: NEW → ASSIGNED
(Reporter)

Comment 8

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

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

Comment 10

12 years ago
not going to lose a day with this but could become a ride-along if we're forced to respin by some other bug.
(Assignee)

Comment 11

12 years ago
See also bug 313949.

/be
(Assignee)

Comment 12

12 years ago
Looking good on trunk, closing but tracking for rc2 ridealong.  Thanks,

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

12 years ago
Created attachment 200998 [details]
XMLList testcase

The assertion is certainly gone but the results of XMLList.prototype.child still look pretty broken to me.
(Assignee)

Comment 14

12 years ago
Created attachment 201037 [details] [diff] [review]
proposed fix

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

/be
Attachment #201037 - Flags: review?(mrbkap)

Updated

12 years ago
Attachment #201037 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 15

12 years ago
Comment on attachment 201037 [details] [diff] [review]
proposed fix

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

/be
(Assignee)

Updated

12 years ago
Attachment #200813 - Flags: approval1.8rc1?
(Assignee)

Comment 16

12 years ago
Created attachment 201054 [details] [diff] [review]
rollup patch for 1.8 branch

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

/be
Attachment #201054 - Flags: review+
Attachment #201054 - Flags: approval1.8rc1?
(Reporter)

Updated

12 years ago
Status: RESOLVED → VERIFIED

Comment 17

12 years ago
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?

Updated

12 years ago
Attachment #201054 - Flags: approval1.8rc1? → approval1.8rc1+

Updated

12 years ago
Attachment #201054 - Flags: approval1.8rc1+ → approval1.8rc2+
(Assignee)

Comment 18

12 years ago
Fixed on the 1.8 branch.

/be
Keywords: fixed1.8

Updated

12 years ago
Flags: blocking1.8rc2?
You need to log in before you can comment on or make changes to this bug.