321831 - Bad arguments to addEventListener cause: Error: uncaught exception: null

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P3)

Description

[thermworm]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051229 Firefox/1.6a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051229 Firefox/1.6a1

A variety of different forms of incorrect argument will cause addEventListener
to report "Error: uncaught exception: null". This is especially annoying
because it doesn't report a line number.

See testcase for examples.


 

Reproducible: Always

Comment 1

[thermworm]

Attachment: Some incorrect addEventListener invocations.

Comment 2

[Jesse Ruderman]

I ran into this too.  In addition, window.addEventListener.arity is 0 and I think it should be 3.

Comment 3

[Jeremy Kitchen]

> See testcase for examples.

I ran into this problem as well, and I created a test scenario for this:
http://test.jscentral.org/addeventlistener.html

t'would be nice if it at least said "addEventListener invocation incorrect" or some sort of clue. :)

-Jeremy

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The one here that I find particularly painful is forgetting the true/false third argument for useCapture.

Comment 5

[Boris Zbarsky [:bzbarsky]]

This is a regression from bug 289940.  The basic problem is that nsDOMClassInfo::ThrowJSException only works for exceptions that have a provider registered.  There's no provider for XPConnect exceptions, so it produces a null exception object.

Blake and I feel that the answer is to implement an XPConnect exception provider.    It wouldn't be very much code to implement this, but we probably need a new object to handle this, unless we want to just hang it off nsXPConnect or something.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Patch v1

This fixes the error by allowing the DOM script object creator translate XPConnect exceptions. It also fixes nsDOMClassInfo::ThrowJSException to never throw null -- instead we'll throw a hard-coded string instead, which can only fail in ways that JS will report.

I also fixed XPCException::ToString to get the message associated with the nsresult. This works for all strings currently, but if anybody ever adds a string with a %-format then we'll end up reading random memory :-(, so we should keep that in mind when describing XPConnect exceptions in the future.

I'll attach a diff -w for reviewing to help with the nsDOMClassInfo changes.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Patch v1 -w

jst wouldn't let me use goto :-/.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Is there any way we can catch the %-format thing?  Say an assert at startup or something?  So if someone adds something like that they'll get an assert as soon as they start?

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 218481 [details] [diff] [review]
Patch v1 -w

r+sr=jst

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

wrt the % problem: it turned out that I was incorrect. While the last parameter should probably be a format string, in actual practice it is used as a flat string, so we don't have to worry about someone adding %s to the strings.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 218481 [details] [diff] [review]
Patch v1 -w

I think we should take this on the 1.8 branch.

Comment 12

[Jesse Ruderman]

(In reply to comment #2)
> I ran into this too.  In addition, window.addEventListener.arity is 0 and I
> think it should be 3.

Filed bug 334245 for the ".arity" issue.

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into MOZILLA_1_8_BRANCH.