Bug Tracker

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#11269 closed bug (invalid)

Event delegation with .on() will not work when a jQuery object is supplied for the Selector parameter

Reported by: warrenparsons Owned by:
Priority: undecided Milestone: None
Component: unfiled Version: 1.7.1
Keywords: Cc:
Blocked by: Blocking:

Description

Event delegation with .on() will not work when a jQuery object is supplied for the Selector parameter.

It seems to ignore the parameter in this case, and skips delegation altogether. The result is that 'this' refers to the object to which .on() is attached, rather than the element(s) specified in the Selector param.

This may be intended behavior, but it is somewhat unexpected.

Test cases: http://jsfiddle.net/tUkwZ/1/

Tested with: 1.7.1 (included with test case), 1.7.2b1 (tested locally)

Verified in: Chrome (current) and Firefox 9.0.1 - both Mac

Change History (7)

comment:1 Changed 8 years ago by dmethvin

Resolution: invalid
Status: newclosed

Yep, as documented it must be a selector string. If you are going to select the elements in advance you might as well bind them.

comment:2 Changed 8 years ago by warrenparsons

Hi Dave,

I think the test case didn't make the problem I'm seeing clear enough.

In my real-world use case, I need to delegate because the elements in question are sometimes added to the document after page load, and it would be useful to pass in a jQuery object, as my selector is rather complex.

Here is the selector I'm using:

var $links = $('a[href^="http"], a[rel="new-window"]').not('.colorbox, .addthis-button');

This would be pretty ugly to write as a pure CSS selector. Also, the API docs for the :not() selector (http://api.jquery.com/not-selector/) include the following:

The .not() method will end up providing you with more readable selections than pushing complex selectors or variables into a :not() selector filter. In most cases, it is a better choice.

Being able to pass in a jQuery object rather than only a string can, in cases like this, make code more readable. It also has potential benefits when selectors are dynamically constructed, since they can be built with jQuery objects rather than put together as a string.

Thanks for listening.

Version 1, edited 8 years ago by warrenparsons (previous) (next) (diff)

comment:3 in reply to:  1 Changed 8 years ago by warrenparsons

Replying to dmethvin:

Yep, as documented it must be a selector string. If you are going to select the elements in advance you might as well bind them.

Hi Dave,

I think the test case didn't make the problem I'm seeing clear enough.

In my real-world use case, I need to delegate because the elements in question are sometimes added to the document after page load, and it would be useful to pass in a jQuery object, as my selector is rather complex.

Here is the selector I'm using:

var $links = $('a[href^="http"], a[rel="new-window"]').not('.colorbox, .addthis-button');

This would be pretty ugly to write as a pure CSS selector. Also, the API docs for the :not() selector (http://api.jquery.com/not-selector/) include the following:

The .not() method will end up providing you with more readable selections than pushing complex selectors or variables into a :not() selector filter. In most cases, it is a better choice.

Being able to pass in a jQuery object rather than only a string can, in cases like this, make code more readable. It also has potential benefits when selectors are dynamically constructed, since they can be built with jQuery objects rather than put together as a string.

Thanks for listening.

comment:4 Changed 8 years ago by dmethvin

You're misunderstanding the point at which a selector selects. Once your $links variable is set, it contains specific DOM elements. If you add more to the document later, they will not be in $links. The selector string was only used to obtain the DOM elements that are now in $links. It does not represent the set of elements that currently exist in the document matching the selector string originally passed to $() at some point in the past. The reason a selector string is required is so that we can determine if the element *currently* matches the selector when an event occurs. If you do not want to repeat the long selector, just assign the selector string to a variable.

comment:5 in reply to:  4 Changed 8 years ago by warrenparsons

Replying to dmethvin:

Crap. You're right of course.

Sorry for the false alarm.

comment:6 Changed 8 years ago by dmethvin

And actually, for a complex case like that I would just do the delegation manually. Directly bind an event to a parent and manually check whether the complex criteria are met, e.g.,

var $elem = $(event.target);
if ( event.target.nodeName.toLowerCase() != "a" )
  return;
if ( $elem.hasClass("colorbox") || $elem.hasClass("addthis-button") )
  return;
... etc ...
Last edited 8 years ago by dmethvin (previous) (diff)

comment:7 in reply to:  6 Changed 8 years ago by warrenparsons

Replying to dmethvin:

And actually, for a complex case like that I would just do the delegation manually. Directly bind an event to a parent and manually check whether the complex criteria are met, e.g.,

var $elem = $(event.target);
if ( event.target.nodeName.toLowerCase() != "a" )
  return;
if ( $elem.hasClass("colorbox") || $elem.hasClass("addthis-button") )
  return;
... etc ...

That's a good call.

In my (somewhat sheepish) defense, I wrote the core of this function three years ago, without any attempt at delegation. I've let it evolve over time, allowing me to overlook its incompatibility with delegation as currently written, when I should instead have just rewritten it from scratch.

Thanks.

Note: See TracTickets for help on using tickets.