Make XPConnect refuse to wrap E4X (was: HTMLSelectElement.add hangs if second parameter is E4X)

VERIFIED FIXED in mozilla1.9alpha1

Status

()

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

People

(Reporter: Jesse Ruderman, Assigned: mrbkap)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla1.9alpha1
hang, testcase, verified1.8.0.2, verified1.8.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 +
blocking1.8.0.2 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [rft-dl])

Attachments

(3 attachments)

(Reporter)

Description

11 years ago
 
(Reporter)

Updated

11 years ago
Blocks: 326633
(Reporter)

Comment 1

11 years ago
Created attachment 212294 [details]
testcase
(Assignee)

Comment 2

11 years ago
E4X interacts badly with XPConnect and the way it does wrapping. In this case we're trying to walk up the parent chain of an E4X object, which (similarly to bug     326615) is causing us to think there's always a parent element. The best way to fix this for now is to make XPConnect simply refuse to wrap E4X objects.
OS: MacOS X → All
Priority: -- → P1
Hardware: Macintosh → All
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Comment 3

11 years ago
Created attachment 212688 [details] [diff] [review]
Hack-fix

We should be able to remove this hack when the DOM <-> E4X bindings are in place.
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #212688 - Flags: superreview?(shaver)
Attachment #212688 - Flags: review?(brendan)
(Assignee)

Updated

11 years ago
Blocks: 326615
Comment on attachment 212688 [details] [diff] [review]
Hack-fix

Throw an exception, please!

/be
Attachment #212688 - Flags: review?(brendan) → review-
Comment on attachment 212688 [details] [diff] [review]
Hack-fix

Curse me for not reading the (missing) context!

All's well, this method is supposed to return true or false and its caller is responsible for throwing in the latter case -- mrbkap kindly pointed that out in person.

/be
Attachment #212688 - Flags: review- → review+
And file a followup (depending on whatever bugs it depends on) to undo this?
(Assignee)

Comment 7

11 years ago
I've added a reference to bug 270553 in the comment above the early return.
Comment on attachment 212688 [details] [diff] [review]
Hack-fix

sr=shaver
Attachment #212688 - Flags: superreview?(shaver) → superreview+
(Assignee)

Comment 9

11 years ago
Fix checked into trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Reporter)

Comment 10

11 years ago
Verified fixed.  The error message I now see looks reasonable:

Error: [Exception... "Could not convert JavaScript argument arg 1 [nsIDOMHTMLSelectElement.add]"  nsresult: "0x80570009 (NS_ERROR_XPC_BAD_CONVERT_JS)"  location: "JS frame :: https://bugzilla.mozilla.org/attachment.cgi?id=212294 :: init :: line 9"  data: no]
Status: RESOLVED → VERIFIED
(Reporter)

Updated

11 years ago
Summary: HTMLSelectElement.add hangs if second parameter is E4X → Make XPConnect refuse to wrap E4X (was: HTMLSelectElement.add hangs if second parameter is E4X)

Comment 11

11 years ago
Checking in regress-327697.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-327697.js,v  <--  regress-327697.js
initial revision: 1.1
done
Flags: testcase+
approving for 1.8.0 and 1.8 branches to fix hang after fix for bug 326615, a=dveditz
Flags: blocking1.8.1+
Flags: blocking1.8.0.2+
Attachment #212688 - Flags: approval1.8.0.2+
Attachment #212688 - Flags: approval-branch-1.8.1+
(Assignee)

Comment 13

11 years ago
Created attachment 214340 [details] [diff] [review]
What I checked into the 1.8 branches
(Assignee)

Comment 14

11 years ago
Checked into the 1.8 branches.
Keywords: fixed1.8.0.2, fixed1.8.1

Updated

11 years ago
Whiteboard: [rft-dl]

Comment 15

11 years ago
v 1.8.0.2, 1.8, 1.9 20060308 win/linux/mac
Keywords: fixed1.8.0.2, fixed1.8.1 → verified1.8.0.2, verified1.8.1

Updated

11 years ago
Keywords: fixed1.8.1

Updated

11 years ago
Keywords: fixed1.8.1
You need to log in before you can comment on or make changes to this bug.