Bug Tracker

Modify

Ticket #12383 (closed bug: fixed)

Opened 20 months ago

Last modified 20 months ago

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

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

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

Change History

comment:1 Changed 20 months ago by Julien Huang <contact@…>

comment:2 Changed 20 months ago by gibson042

  • Status changed from new to closed
  • Resolution set to wontfix
  • Component changed from unfiled to event

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

comment:3 follow-up: ↓ 9 Changed 20 months ago by scott.gonzalez

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

comment:4 Changed 20 months ago by dmethvin

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.

comment:5 follow-up: ↓ 7 Changed 20 months ago by dmethvin

  • Cc timmywil added
  • Priority changed from undecided to high
  • Status changed from closed to reopened
  • Resolution wontfix deleted
  • Milestone None deleted

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

comment:6 Changed 20 months ago by dmethvin

  • Status changed from reopened to open

comment:7 in reply to: ↑ 5 Changed 20 months ago by gibson042

Replying to 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.

comment:8 Changed 20 months ago by dmethvin

  • Priority changed from high to blocker
  • Milestone set to 1.8.1

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.

comment:9 in reply to: ↑ 3 Changed 20 months ago by timmywil

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.

comment:10 Changed 20 months ago by Dave Methvin

  • Status changed from open to closed
  • Resolution set to fixed

Fix #12383. All selectors should be delegateTarget-relative

Changeset: afd717df9e189d3e374ce938f03e2712f31c29e3

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.