Side navigation
#11756 closed bug (worksforme)
Opened May 11, 2012 06:39PM UTC
Closed May 18, 2012 03:39PM UTC
Last modified May 21, 2012 05:58PM UTC
event.currentTarget documentation inaccurate for delegated events
Reported by: | jmm | Owned by: | jmm |
---|---|---|---|
Priority: | undecided | Milestone: | None |
Component: | unfiled | Version: | 1.7.2 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
Summary
The event.currentTarget documentation doesn't seem to be quite accurate for delegated events. It says:
Description: The current DOM element within the event bubbling phase.
But it seems that for delegated events, currentTarget
is hijacked to mean "the element that matches the selector" (the selector part of the .on( events [, selector] [, data] , handler(eventObject) )
signature). In other words, it simulates the currentTarget
that would exist if the listener was directly bound to the element matching the selector.
I get the impression that has been the case since delegate()
was added in 1.4.2, for example from ticket #10563
There's now event.delegateTarget
, which is really the current element within the event bubbling phase, isn't it?
Testcase
The situation I'm describing is illustrated by the following evaluating to true
(in FF 7 Win, Chrome 18 Win) in a delegated handler:
event.currentTarget !== event.originalEvent.currentTarget &&
event.delegateTarget === event.originalEvent.currentTarget
Notes
My interpretation of this is:
- A long time ago (1.4.2), a useful piece of data (the DOM element that matches the selector in a delegation ) was added to
jQuery.Event
, which was good.
- The data was added by co-opting the existing DOM
Event.currentTarget
property, changing its meaning. I'm wondering if that was as good of an idea.
- 1.7 added
event.delegateTarget
to provide (restore) access to the data that is lost as a result ofcurrentTarget
being co-opted to store other data.
- Perhaps if this were being created from scratch today
currentTarget
anddelegateTarget
would be reversed, so thatcurrentTarget
corresponds to the element where the listener is registered (the actual current element within the bubbling phase) anddelegateTarget
corresponds to the element matching the selector.
I speculate that because, if there's going to be an additional property (delegateTarget
) for these events to convey data above and beyond what the DOM includes, it seems to me like it would make sense to preserve the meaning of the existing property, currentTarget
, and embed the new information in the new, non-standard property.
But, I imagine it's either intentional (?), or too late to change the meaning of currentTarget
for delegated events anyway. In any case, it would be good for the currentTarget
documentation and other relevant parts to accurately describe the situation.
As an aside, where the .on()
documentation says:
jQuery passes the handler an event object it can use to analyze and change the status of the event. This object is a *normalized subset* of data provided by the browser;
With the addition of things like delegateTarget
(and in some cases other things, like currentTarget
), it's not strictly true that the event object is a subset of data provided by the browser. I guess to be really accurate it could say something like "includes a *normalized subset* of data included in the W3C DOM Event interface, as well as additional data provided by the browser and / or jQuery".
Attachments (0)
Change History (10)
Changed May 11, 2012 06:54PM UTC by comment:1
owner: | → jmm |
---|---|
status: | new → pending |
Changed May 11, 2012 08:54PM UTC by comment:2
status: | pending → new |
---|
I think the reasonevent.currentTarget
was munged was so that.live()
and.bind()
would be equivalent from the view of the event handler.
Ok, thanks for the background.
Really it hardly matters since I bet 98% of people usethis
instead ofevent.currentTarget
.
>
If we were starting again then yes, it would make sense to leave currentTarget alone and make people look at delegateTarget.
Perhaps most people do mostly use this
. If it's that prevalent, then it does sound like there would have been little downside to requiring people to use delegateTarget
in that case, unless it's considered essential that currentTarget === this
when this
is a DOM object.
I often write in an OO fashion and use $.proxy()
or some other mechanism for making this
in my listeners refer to my own objects. This is beside your point, just an aside about my personal usage.
In general, using this
for this purpose seems like kind of a waste since the data it would refer to is accessible via the other variables (target
, currentTarget
, delegateTarget
) and this
can be put to a more useful purpose. Not that I expect that to change, I'm just saying.
The amount of this
usage may decline if things like Backbone catch on, which encourages setting up event handling in a way that binds this
to its view objects in the listeners.
But given that several years have gone by we are likely to break someone's code. There isn't a lot of reason to do that.
Yeah, I figured there's no way it can be changed now, unfortunately.
For most people's view it's a normalized subset, the importance of saying subset there is that many people seem to expect every property of the native event object to be present in the jQuery one.
Right. Well maybe it would make sense to just change it to "This object includes a normalized subset...". I guess it's a technicality though.
Given that information, what action do you think needs to be taken to close this ticket?
I think the event.currentTarget
documentation should be updated to explain exactly what data it will contain in various situations (directly bound and delegated events) and clarify that it isn't necessarily the current element in the bubbling phase and not necessarily the same as the currentTarget
value of the native event object. Also, explain that delegateTarget
may contain what you might expect currentTarget
to, and when it will.
I don't know what's involved in submitting a patch for the documentation or if I'll have time to figure it out, but if I wrote some new copy, might that be accepted and added to the docs by someone who already knows how?
I could submit a new ticket if I notice other areas of the documentation that could use clarification on this issue and this ticket is closed by then.
Thanks
Changed May 11, 2012 09:05PM UTC by comment:3
jmm - We've begun tracking documentation bugs in Github, https://github.com/jquery/api.jquery.com/issues! You can file this there, and you can submit a patch by forking the repository and submitting a pull request on the docs for event.currentTarget (https://github.com/jquery/api.jquery.com/blob/master/entries/event.currentTarget.xml). Right now those changes have to be rolled into our Wordpress install manually, but it will soon be automated. In the meantime, if you get the PR in, someone will be able to assist you with getting it in to the live docs. Thanks for your interest in helping out!
Changed May 15, 2012 12:16PM UTC by comment:4
Considering the following, is it definitely too late to swap the meaning of currentTarget
and delegateTarget
, restoring currentTarget
to its standard meaning?:
on()
is still relatively new.
- jQuery's treatment of
currentTarget
has not even been documented up to now.
- It's been speculated that a massive majority of people use
this
and don't even usecurrentTarget
.
I'm suggesting:
- Restore the standard meaning of
currentTarget
when usingon()
.
- Leave the existing treatment of
currentTarget
for older methods likelive()
anddelegate()
.
- Store the element that matches the selector in a delegation in
delegateTarget
. Or, if there's more peril in changing that thancurrentTarget
, create a new property likevirtualCurrentTarget
/effectiveCurrentTarget
/ ?
- Accurately document the treatment of
currentTarget
according to registration method / jQuery version.
Changed May 15, 2012 12:19PM UTC by comment:5
status: | new → pending |
---|
I just don't see any need to change any behavior or add new variables. This has not been a point of confusion. Are there situations where the current variables cannot provide the information you need?
Changed May 16, 2012 04:34PM UTC by comment:6
status: | pending → new |
---|
Are there situations where the current variables cannot provide the information you need?
No, the data is available with current variables in 1.7+ now that delegateTarget
has been added. To me it just seems like an undesirable outcome that the new, non-standard property (delegateTarget
) -- and not currentTarget
-- is what corresponds to the standard currentTarget
.
In hindsight it seems like the decision to use currentTarget
like this maybe wasn't ideal, and perhaps there was a missed opportunity to straighten it out when on()
and delegateTarget
were introduced. If it's too late to straighten out now, that's that. If it's not too late, would it not make sense to seize the opportunity to get currentTarget
back on track, mirroring the standard property of the same name, once and for all, and have delegateTarget
contain the data that's overlaid on the DOM?
As I mentioned, the treatment of currentTarget
, diverging from the DOM, has not actually been documented, at least not in the currentTarget
entry.
This has not been a point of confusion.
I can't speak for anyone else, but it was a source of confusion for me when the documentation says currentTarget
, which has the same name as a standard DOM property, is "the current DOM element within the event bubbling phase" and it's not really. But if 98% of people use this
and not currentTarget
, then I'm not too surprised it hasn't been on the radar.
Changed May 18, 2012 03:39PM UTC by comment:7
resolution: | → worksforme |
---|---|
status: | new → closed |
I think we're good with what we currently have so I'll close this ticket. If you have specific wording suggestions for the docs you can post them here.
Changed May 18, 2012 06:51PM UTC by comment:8
I actually submitted a pull request for a documentation patch and then closed it when I got the idea that maybe it's actually not too late to change the treatment of currentTarget
(as I talked about in comment 4).
I can't help but think that if there is still enough leeway to make a change to this behavior, due to the newness of on()
and lack of documentation of this treatment of currentTarget
, then documenting it will be the last nail in the coffin of opportunity to change it.
If there's any chance that changing the behavior will be considered, then it wouldn't make sense to use my documentation patch. If there's zero chance it will be changed, as it seems you're saying, then I'll re-open the pull request.
Changed May 21, 2012 01:30AM UTC by comment:9
I should reference #10563 here, which indicates the compat impact of changing this is not hypothetical.
Changed May 21, 2012 05:58PM UTC by comment:10
Understood, which is why I was only floating the idea of changing it for listeners registered with on()
. However, as a developer it's dangerous to rely on undocumented behavior. The current docs describe the standard meaning of currentTarget
, not the actual treatment in jQuery:
The current DOM element within the event bubbling phase.
It makes no distinction between direct and delegated registrations.
If the issue is backward compatibility, that seems relevant to me. If it's intentional that currentTarget
be coerced this way because on balance it's considered more useful than preserving the standard meaning, that's a different story.
Anyway, if the official position of jQuery is that it's not changing, the last paragraph of my previous post sums it up for me.
I think you've got it about right.
I think the reason
event.currentTarget
was munged was so that.live()
and.bind()
would be equivalent from the view of the event handler. Since we're trying to abstract the delegation perhaps that made sense. Really it hardly matters since I bet 98% of people usethis
instead ofevent.currentTarget
.If we were starting again then yes, it would make sense to leave
currentTarget
alone and make people look atdelegateTarget
. But given that several years have gone by we are likely to break someone's code. There isn't a lot of reason to do that.For most people's view it's a normalized subset, the importance of saying subset there is that many people seem to expect every property of the native event object to be present in the jQuery one.
Given that information, what action do you think needs to be taken to close this ticket?