New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CustomEvent `detail` property not copied to Event object #1867

Closed
remko opened this Issue Nov 13, 2014 · 10 comments

Comments

Projects
None yet
6 participants
@remko

remko commented Nov 13, 2014

According to the documentation (or at least how I interpret it), the detail property is copied to the event object when it is present (i.e. when the event is a CustomEvent). However, this does not seem to be the case (I tried accessing it, and i don't see any reference to detail in the code either, it's not listed in any props list in event.js).

My workaround is to go through originalEvent.detail, but it would be nice if jQuery copied this property.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Nov 13, 2014

Member

Thanks for the report! Could you create a minimal example on
http://jsbin.com?

Michał Gołębiowski

Member

mgol commented Nov 13, 2014

Thanks for the report! Could you create a minimal example on
http://jsbin.com?

Michał Gołębiowski

@remko

This comment has been minimized.

Show comment
Hide comment

@markelog markelog added the Event label Nov 13, 2014

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Nov 14, 2014

Member

There seem to be two documented properties that are not copied, detail and prevValue. I have no idea why prevValue is listed there and I removed it.

Because of the overhead of copying properties, I'm hesitant to add detail and as you say it's available through originalEvent. So we can either update the docs to remove it (my preferred route) or add it. Note that adding it won't help anyone until the next release anyway so the docs wouldn't be correct as-is unless we put in some note like "this was added in jQuery 3.0".

Member

dmethvin commented Nov 14, 2014

There seem to be two documented properties that are not copied, detail and prevValue. I have no idea why prevValue is listed there and I removed it.

Because of the overhead of copying properties, I'm hesitant to add detail and as you say it's available through originalEvent. So we can either update the docs to remove it (my preferred route) or add it. Note that adding it won't help anyone until the next release anyway so the docs wouldn't be correct as-is unless we put in some note like "this was added in jQuery 3.0".

@FagnerMartinsBrack

This comment has been minimized.

Show comment
Hide comment
@FagnerMartinsBrack

FagnerMartinsBrack Nov 14, 2014

I had this same problem recently in a project, where I initially assumed the property was present.

After some thought I realized jQuery already have a custom way to handle this using the data property.
Since the detail property is a standard way to pass data to the triggered callback without using jQuery, it makes sense to be available in the original event.

In other hand, the callback is not portable between native and jQuery-based environments.
Is portability something to be considered in this case when dealing with an event triggered by a third party that doesn't use jQuery?

I had this same problem recently in a project, where I initially assumed the property was present.

After some thought I realized jQuery already have a custom way to handle this using the data property.
Since the detail property is a standard way to pass data to the triggered callback without using jQuery, it makes sense to be available in the original event.

In other hand, the callback is not portable between native and jQuery-based environments.
Is portability something to be considered in this case when dealing with an event triggered by a third party that doesn't use jQuery?

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Nov 14, 2014

Member

Any property not normalized by jQuery can be accessed via originalEvent, and there are plenty of special ones that are only used by one or two events. The copying to jQuery's event is something we want to keep to a minimum since it has real costs and is in a time-critical section of code. Accessing the value through orginalEvent doesn't cause any problem, does it?

Member

dmethvin commented Nov 14, 2014

Any property not normalized by jQuery can be accessed via originalEvent, and there are plenty of special ones that are only used by one or two events. The copying to jQuery's event is something we want to keep to a minimum since it has real costs and is in a time-critical section of code. Accessing the value through orginalEvent doesn't cause any problem, does it?

@FagnerMartinsBrack

This comment has been minimized.

Show comment
Hide comment
@FagnerMartinsBrack

FagnerMartinsBrack Nov 14, 2014

As I mentioned, in my case in particular the only problem was the code portability.

When I ported the event handling callback from another codebase I initially expected it would work without changes in a jQuery environment using the ui widget factory. What happened instead is that I had to change event.detail to event.originalEvent.detail.

It is not a big deal though if performance is an issue. Updating the docs seems reasonable.

As I mentioned, in my case in particular the only problem was the code portability.

When I ported the event handling callback from another codebase I initially expected it would work without changes in a jQuery environment using the ui widget factory. What happened instead is that I had to change event.detail to event.originalEvent.detail.

It is not a big deal though if performance is an issue. Updating the docs seems reasonable.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Dec 1, 2014

Member

Per discussion in the meeting today, let's add .detail to match the current docs.

Member

dmethvin commented Dec 1, 2014

Per discussion in the meeting today, let's add .detail to match the current docs.

@dmethvin dmethvin added this to the 3.0.0 milestone Dec 1, 2014

@dmethvin dmethvin self-assigned this Dec 1, 2014

dmethvin added a commit that referenced this issue Dec 3, 2014

Event: Copy detail property to jQuery.Event on native events
Fixes gh-1867
(cherry picked from commit d9ed166)

Conflicts:
   test/unit/event.js

@dmethvin dmethvin closed this in d9ed166 Dec 3, 2014

@amakhrov

This comment has been minimized.

Show comment
Hide comment
@amakhrov

amakhrov May 28, 2015

Any chance to get this fix in the stable branches?

Any chance to get this fix in the stable branches?

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin May 28, 2015

Member

Is there a problem using event.originalEvent.detail instead?

Member

dmethvin commented May 28, 2015

Is there a problem using event.originalEvent.detail instead?

@amakhrov

This comment has been minimized.

Show comment
Hide comment
@amakhrov

amakhrov May 28, 2015

Not really - just have to make sure we always use jquery for attaching events.

Not really - just have to make sure we always use jquery for attaching events.

@dmethvin dmethvin modified the milestones: 1.12/2.2, 3.0.0 Jan 7, 2016

@cssmagic cssmagic referenced this issue May 18, 2016

Open

jQuery #5

@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.