Bug Tracker

Opened 12 years ago

Closed 12 years ago

#7752 closed bug (invalid)

Passing 1 function to toggle-event should be treated like .click(). Currently there are side effects.

Reported by: patrickwhalen Owned by:
Priority: low Milestone: 1.6
Component: event Version: 1.4.4
Keywords: Cc:
Blocked by: Blocking:


This will likely not be considered a bug, since it involves an improper usage of the API. As such, I'm filing this as an "enhancement". I thought it merited consideration since it involves possible unexpected code execution and a potential infinite loop.

If you incorrectly pass 1 function to the event version of .toggle(), it seems to fire the handler automatically during setup. In other words, no actual event is required.

$('div').toggle(function() {
    console.log( 'the handler fired' ); // Fires once on load.


In addition, if an Error is thrown in the handler, the handler will continue to fire infinitely.

$('div').toggle(function() {
    throw( 'some error' ); // Fires infinitely


Again, passing 1 function is not a supported usage of the method. But given the potential outcome, I thought it may be appropriate to treat a toggle() with only one function as a normal click() handler.

In the toggle() method:


...its implementation may be as simple as:

if( args.length === 1 && typeof fn === 'function' ) {
     return this.click( fn );

Change History (5)

comment:1 Changed 12 years ago by patrickwhalen

Now I see that the issue is the double duty of .toggle(). As such, the enhancement is invalid, though the infinite loop aspect may be of interest.

comment:2 Changed 12 years ago by ajpiano

Component: unfiledevent
Priority: undecidedlow

I don't know, jQuery, like a lot of other things, breaks when you pass invalid arguments to methods. This is one way that libraries and languages tell you that you are doing something wrong, so I don't think this is particularly urgent. By the same token, it's a pretty simple and reasonable fix - just use jQuery.isFunction instead. Do other folks think this is worth doing?

comment:3 Changed 12 years ago by ajpiano

Type: enhancementbug

comment:4 in reply to:  2 Changed 12 years ago by patrickwhalen

Replying to ajpiano:

I don't know, jQuery, like a lot of other things, breaks when you pass invalid arguments to methods.

True. Taking another look, I would bet that some folks out there pass a single function as the sole parameter when using toggle() as its "toggle effect" version (not the click handler version). Making it default to click() would break code unnecessarily.

When I filed the original report, I had forgotten about the effect version of toggle(), but what really prompted it was the infinite loop.

Doing another test, I see that the actual cause of the infinite loop is throwing an error in the animate() callback.

    throw('e');  // Fires infinitely


Depending on what is taking place in the callback, it may be difficult to anticipate an error being thrown. Given an unanticipated error, the outcome could be devastating to the application.

But then, I don't know if there's a simple fix for this outside of a try/catch around the callback, which may not be desired.

Last edited 12 years ago by patrickwhalen (previous) (diff)

comment:5 Changed 12 years ago by ajpiano

Resolution: invalid
Status: newclosed

At this point, this ticket certainly no longer pertains to the original proposed enhancement, which we seem to have determined is probably not worth it, so I'm gonna go ahead and close it...

The issue about the uncaught exceptions in animation callbacks can be tracked in #5684.

Last edited 12 years ago by ajpiano (previous) (diff)
Note: See TracTickets for help on using tickets.