Bug Tracker

Modify

Ticket #7752 (closed bug: invalid)

Opened 3 years ago

Last modified 3 years ago

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:
Blocking: Blocked by:

Description

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.
});

 http://jsfiddle.net/patrick_dw/jGwQA/

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
});

 http://jsfiddle.net/patrick_dw/jGwQA/1/

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:

 https://github.com/jquery/jquery/blob/master/src/event.js#L971

...its implementation may be as simple as:

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

Change History

comment:1 Changed 3 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 follow-up: ↓ 4 Changed 3 years ago by ajpiano

  • Priority changed from undecided to low
  • Component changed from unfiled to event

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 3 years ago by ajpiano

  • Type changed from enhancement to bug

comment:4 in reply to: ↑ 2 Changed 3 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.

$('div').animate({width:100},function(){
    throw('e');  // Fires infinitely
});

 http://jsfiddle.net/patrick_dw/jGwQA/2/

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 3 years ago by patrickwhalen (previous) (diff)

comment:5 Changed 3 years ago by ajpiano

  • Status changed from new to closed
  • Resolution set to invalid

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 3 years ago by ajpiano (previous) (diff)

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.