742156 - Stringifying EventTarget throws

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Testcase:

  javascript:alert(EventTarget)

the issue is we're calling Function.prototype.toString.  We never added an own toString on our interface objects....

Comment 1

[Peter Van der Beken [:peterv]]

Attachment: v1

This is causing orange on the Jetpack tests.

Comment 2

[Alex Keybl [:akeybl]]

(In reply to Peter Van der Beken [:peterv] from comment #1)
> Created attachment 612126 [details] [diff] [review]
> v1
> 
> This is causing orange on the Jetpack tests.

Tracking for release to prevent issues hiding behind these orange tests. Let's go with the simplest fix (test or code change) to get this working again.

If that's not the only reason to track this bug for release, please clarify. Thanks!

Comment 3

[Peter Van der Beken [:peterv]]

Attachment: v1.1

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 620287 [details] [diff] [review]
v1.1

>+++ b/dom/bindings/Utils.cpp
>+// passed a non-Function object we als need to provide our own toString method

s/als/also/

>+InterfaceObjectToString(JSContext* cx, unsigned argc, JS::Value *vp)

>+  JS::Value thisv = JS_THIS(cx, vp);

You could use JS_THIS_OBJECT and then just null-check it; then no need for thisv.

Note that JS_THIS always returns either JSVAL_NULL or an ObjectValue(), so the "undefined" case can't even be hit here.

r=me with those fixed.

Comment 5

[Peter Van der Beken [:peterv]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/efa51cd286ee

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/efa51cd286ee

Comment 7

[Peter Van der Beken [:peterv]]

Comment on attachment 620287 [details] [diff] [review]
v1.1

[Approval Request Comment]
Regression caused by (bug #): bug 740069
User impact if declined: seems to break Jetpack tests, not sure if it affects addons. Could cause website breakage due to unexpected exceptions.
Testing completed (on m-c, etc.): landed on m-c (and has a testcase)
Risk to taking this patch (and alternatives if risky): low-risk
String changes made by this patch: None

Comment 8

[Alex Keybl [:akeybl]]

Comment on attachment 620287 [details] [diff] [review]
v1.1

[Triage Comment]
Approving for Aurora 14.

Comment 9

[Peter Van der Beken [:peterv]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b486a6c98abe