Skip to content

CustomEvent detail property not copied to Event object #1867

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

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

CustomEvent detail property not copied to Event object #1867

remko opened this issue Nov 13, 2014 · 10 comments
Assignees
Labels
Milestone

Comments

@remko
Copy link

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
Copy link
Member

mgol commented Nov 13, 2014

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

Michał Gołębiowski

@remko
Copy link
Author

remko commented Nov 13, 2014

@markelog markelog added the Event label Nov 13, 2014
@dmethvin
Copy link
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".

@FagnerMartinsBrack
Copy link

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
Copy link
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?

@FagnerMartinsBrack
Copy link

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
Copy link
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
Fixes gh-1867
(cherry picked from commit d9ed166)

Conflicts:
   test/unit/event.js
@amakhrov
Copy link

Any chance to get this fix in the stable branches?

@dmethvin
Copy link
Member

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

@amakhrov
Copy link

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
@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.
Labels
Development

No branches or pull requests

6 participants