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

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Jesse Ruderman]

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

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.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Hack-fix

We should be able to remove this hack when the DOM <-> E4X bindings are in place.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 212688 [details] [diff] [review]
Hack-fix

Throw an exception, please!

/be

Comment 5

[Brendan Eich [:brendan]]

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

Comment 6

[Boris Zbarsky [:bzbarsky]]

And file a followup (depending on whatever bugs it depends on) to undo this?

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

I've added a reference to bug 270553 in the comment above the early return.

Comment 8

[Mike Shaver (:shaver emeritus)]

Comment on attachment 212688 [details] [diff] [review]
Hack-fix

sr=shaver

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 10

[Jesse Ruderman]

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]

Comment 11

[Bob Clary [:bc] (inactive)]

Checking in regress-327697.js;
/cvsroot/mozilla/js/tests/e4x/Regress/regress-327697.js,v  <--  regress-327697.js
initial revision: 1.1
done

Comment 12

[Daniel Veditz [:dveditz]]

approving for 1.8.0 and 1.8 branches to fix hang after fix for bug 326615, a=dveditz

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: What I checked into the 1.8 branches

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Checked into the 1.8 branches.

Comment 15

[Bob Clary [:bc] (inactive)]

v 1.8.0.2, 1.8, 1.9 20060308 win/linux/mac