Last Comment Bug 742156 - Stringifying EventTarget throws
: Stringifying EventTarget throws
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla15
Assigned To: Peter Van der Beken [:peterv]
:
:
Mentors:
Depends on:
Blocks: 740069 741390
  Show dependency treegraph
 
Reported: 2012-04-03 20:11 PDT by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2012-05-10 05:12 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed


Attachments
v1 (4.04 KB, patch)
2012-04-04 00:34 PDT, Peter Van der Beken [:peterv]
no flags Details | Diff | Splinter Review
v1.1 (5.89 KB, patch)
2012-05-02 06:29 PDT, Peter Van der Beken [:peterv]
bzbarsky: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2012-04-03 20:11:04 PDT
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] 2012-04-04 00:34:52 PDT
Created attachment 612126 [details] [diff] [review]
v1

This is causing orange on the Jetpack tests.
Comment 2 Alex Keybl [:akeybl] 2012-04-09 15:23:27 PDT
(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] 2012-05-02 06:29:56 PDT
Created attachment 620287 [details] [diff] [review]
v1.1
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2012-05-02 11:51:15 PDT
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] 2012-05-03 06:56:25 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/efa51cd286ee
Comment 6 Ed Morley [:emorley] 2012-05-04 04:01:49 PDT
https://hg.mozilla.org/mozilla-central/rev/efa51cd286ee
Comment 7 Peter Van der Beken [:peterv] 2012-05-04 04:26:54 PDT
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] 2012-05-06 19:22:05 PDT
Comment on attachment 620287 [details] [diff] [review]
v1.1

[Triage Comment]
Approving for Aurora 14.
Comment 9 Peter Van der Beken [:peterv] 2012-05-10 05:12:18 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/b486a6c98abe

Note You need to log in before you can comment on or make changes to this bug.