jQuery.on() selector should only apply to descendants of the element

Reported by:	contact@julienhuang.com	Owned by:
Priority:	blocker	Milestone:	1.8.1
Component:	event	Version:	1.8.0
Keywords:		Cc:	timmywil
Blocked by:		Blocking:

Description

The $.on() method accepts a CSS selector parameter in order to setup delegated event handlers.

According to the documentation, this selector only applies to descendants of the element :

A selector string to filter the descendants of the selected elements that will call the handler. If the selector is null or omitted, the handler is always called when it reaches the selected element.

The current implementation when the event is triggered via jQuery.event.dispatch is to call $.is() in order to match the event.target and its parents. The issue with $.is() is that it will not only test for descendants, but also for parents of the element. Considering the documentation, I think this is unintuitive.

Below an example of the issue :

JSBin link : http://jsbin.com/ocijif/1/embed?live

Attachments (0)

Change History (10)

Changed August 23, 2012 04:49PM UTC by Julien Huang <contact@julienhuang.com> comment:1

Github Pull Request : https://github.com/jquery/jquery/pull/906

Changed August 23, 2012 04:58PM UTC by gibson042 comment:2

component:	unfiled → event
resolution:	→ wontfix
status:	new → closed

This is working as designed. If you want the thead in your filtering selector to be a descendant of the context element, specify it accordingly: http://jsbin.com/ocijif/2/edit

Changed August 23, 2012 05:19PM UTC by scottgonzalez comment:3

It may be working as designed, but isn't this just as bad as qSA?

Changed August 23, 2012 05:25PM UTC by dmethvin comment:4

Actually I thought I had fixed this in 1.8.0 so it's worth another look. I'm mobile ATM so I will do this later.

Changed August 24, 2012 12:49AM UTC by dmethvin comment:5

cc:	→ timmywil
milestone:	None
priority:	undecided → high
resolution:	wontfix
status:	closed → reopened

I did fix the issue in 1.8.0, but only for positional selectors like :first where .is() already uses the exact same code being proposed by the pull request.

What I wonder is, should $(selector, context).is(selector) always be rooted at context as well? At the moment, it is not unless the selector is a positional one.

@timmywil I know we discussed this when the change for positionals was made but I don't recall all the details.

https://github.com/jquery/jquery/commit/70e2e32e0eb03607ad0c8b7752dbd7747da47164

Changed August 24, 2012 12:50AM UTC by dmethvin comment:6

status:	reopened → open

Changed August 24, 2012 02:15AM UTC by gibson042 comment:7

Replying to [comment:5 dmethvin]:

What I wonder is, should $(selector, context).is(selector) always be rooted at context as well? At the moment, it is not unless the selector is a positional one.

I would vote against that. There is an argument to be made for the restricted context when it is obvious (as in the case of .on(events, selector, handler)) or necessary (as in the case of positionals). But if we make it universal, then functions accepting jQuery object parameters won't be able to trust .is("ancestor *") because the objects ''might'' have been constructed with a context descended from ancestor.

Changed August 24, 2012 02:24AM UTC by dmethvin comment:8

milestone:	→ 1.8.1
priority:	high → blocker

It *does* seem rather obscure if the context gets too far away from the .is(). It seemed worth making the exception for positionals because people are constantly confused by the case mentioned in the code.

Changed August 24, 2012 02:27PM UTC by timmywil comment:9

I agree with @gibson042. Makes sense for .on(), but not .is(). .is() shouldn't be context dependent. The set is already limited and may contain elements from all over the page. Limiting matching to a particular context would produce unexpected results.

Changed August 28, 2012 02:12PM UTC by Dave Methvin comment:10

resolution:	→ fixed
status:	open → closed

Fix #12383. All selectors should be delegateTarget-relative

Changeset: afd717df9e189d3e374ce938f03e2712f31c29e3

Side navigation

#12383 closed bug (fixed)