Opened 10 years ago
Closed 10 years ago
#12383 closed bug (fixed)
jQuery.on() selector should only apply to descendants of the element
Reported by: | 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
Change History (10)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
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
comment:3 follow-up: 9 Changed 10 years ago by
It may be working as designed, but isn't this just as bad as qSA?
comment:4 Changed 10 years ago by
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 10 years ago by
Cc: | timmywil added |
---|---|
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
comment:6 Changed 10 years ago by
Status: | reopened → open |
---|
comment:7 Changed 10 years ago by
Replying to dmethvin:
What I wonder is, should
$(selector, context).is(selector)
always be rooted atcontext
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 10 years ago by
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.
comment:9 Changed 10 years ago by
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 10 years ago by
Resolution: | → fixed |
---|---|
Status: | open → closed |
Fix #12383. All selectors should be delegateTarget-relative
Changeset: afd717df9e189d3e374ce938f03e2712f31c29e3
Github Pull Request : https://github.com/jquery/jquery/pull/906