Opened 14 years ago

Closed 12 years ago

#353 closed defect (invalid)

[patch][cla] event.connect breaks with undefined first argument

Reported by: david Owned by: sjmiles
Priority: high Milestone: 0.9
Component: Core Version: 0.2
Keywords: event Cc: dojo-contributors@…, alex
Blocked By: Blocking:

Description

This attaches to window:

dojo.event.connect(undefined, "onclick", dojo, "debug");

Outputs DEBUG: [object MouseEvent] when I click anywhere on the page.

If you have:

dojo.event.connect(undefined, "onclick", dojo.debug);

It doesn't throw any errors but I don't know where that gets connected as it doesn't fire.

Attachments (3)

t353.patch (719 bytes) - added by guest 13 years ago.
event.js.353.patch (1.4 KB) - added by bookstack 13 years ago.
Another patch regarding the feedback from the community, esspecially elazutkin
test_event.html (1.8 KB) - added by bookstack 13 years ago.
The test cases for #353

Download all attachments as: .zip

Change History (14)

comment:1 Changed 13 years ago by dylan

Milestone: 0.4

comment:2 Changed 13 years ago by guest

Owner: changed from alex to bookstack@…

Since undefined is not an object or string, current interpolateArgs implementation does not cover this corner situation.

Changed 13 years ago by guest

Attachment: t353.patch added

comment:3 Changed 13 years ago by Eugene Lazutkin

1st case: most probably we should guard against "undefined" and throw an exception for such connects. Or ignore it. I would prefer the 1st option because a programmer made a mistake during calculation of parameters.

2nd case: typeof(dojo.debug) is "function", and dojo.debug instanceof Function is True. :-( You can even call it but this is not going to be set. So my guess is true methods will blow up on event but I don't see how we can distinguish this (clearly wrong) case from legitimate function parameters.

comment:4 Changed 13 years ago by Eugene Lazutkin

Priority: highnormal
severity: majornormal

Changed 13 years ago by bookstack

Attachment: event.js.353.patch added

Another patch regarding the feedback from the community, esspecially elazutkin

comment:5 Changed 13 years ago by bookstack

Cc: dojo-contributors@… added

The new attachment event.js.353.patch would not mimic the default behavior if an undefined object is used, it would throw exceptions in all corner cases. Here is a test case for it as well.

Changed 13 years ago by bookstack

Attachment: test_event.html added

The test cases for #353

comment:6 Changed 13 years ago by dylan

Summary: event.connect breaks with undefined first argument[patch][cla] event.connect breaks with undefined first argument

comment:7 Changed 13 years ago by dylan

Milestone: 0.40.4.1

comment:8 Changed 13 years ago by bill

Milestone: 0.4.10.5

seems like a lot of code to add to catch a user error; i don't think it's worth it. anyway, i'll defer to 0.5 and we can think about it then.

comment:9 Changed 12 years ago by tk

Cc: alex added

Maybe we should get Alex involved in this discussion, I would imagine this has been resolved (or maybe its an edge case that David found) since I havent seen any buzz on the ML or forums regarding this type of bug...

Trying to clean up the trac DB...

-Karl

comment:10 Changed 12 years ago by Adam Peller

Owner: changed from bookstack@… to sjmiles

comment:11 Changed 12 years ago by sjmiles

Resolution: invalid
Status: newclosed

0.9 analog dojo.connect requires a non-null first argument and will throw an exception on detectably bad input.

Note: See TracTickets for help on using tickets.