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

RESOLVED FIXED in mozilla1.9alpha1

Status

()

defect
P3
normal
RESOLVED FIXED
14 years ago
13 years ago

People

(Reporter: thermworm, Assigned: mrbkap)

Tracking

(Blocks 1 bug, {fixed1.8.1, testcase})

Trunk
mozilla1.9alpha1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(3 attachments)

(Reporter)

Description

14 years ago
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

Updated

13 years ago
Assignee: general → events
Status: UNCONFIRMED → NEW
Component: DOM: Core → DOM: Events
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All

Comment 2

13 years ago
I ran into this too.  In addition, window.addEventListener.arity is 0 and I think it should be 3.
Blocks: 326633

Updated

13 years ago
Keywords: testcase

Comment 3

13 years ago
> 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
The one here that I find particularly painful is forgetting the true/false third argument for useCapture.
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.
Blocks: 289940
Assignee: events → mrbkap
Priority: -- → P3
Target Milestone: --- → mozilla1.9alpha
Status: NEW → ASSIGNED
Whiteboard: [patch]
Posted patch Patch v1Splinter Review
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.
Posted patch Patch v1 -wSplinter Review
jst wouldn't let me use goto :-/.
Attachment #218481 - Flags: review?(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 on attachment 218481 [details] [diff] [review]
Patch v1 -w

r+sr=jst
Attachment #218481 - Flags: superreview+
Attachment #218481 - Flags: review?(bzbarsky)
Attachment #218481 - Flags: review+
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Comment on attachment 218481 [details] [diff] [review]
Patch v1 -w

I think we should take this on the 1.8 branch.
Attachment #218481 - Flags: approval-branch-1.8.1?(jst)

Comment 12

13 years ago
(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.
Attachment #218481 - Flags: approval-branch-1.8.1?(jst) → approval-branch-1.8.1+
Flags: blocking1.8.1?
Fix checked into MOZILLA_1_8_BRANCH.
Keywords: fixed1.8.1
Flags: blocking1.8.1?
You need to log in before you can comment on or make changes to this bug.