- Notifications
You must be signed in to change notification settings - Fork20.6k
Event: Only attach events to objects that accept data - for real#4558
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Wow, this does go back a ways! |
test/unit/event.js Outdated
| QUnit.test( ".on('focus', fn) on a text node doesn't throw", function( assert ) { | ||
| assert.expect( 1 ); | ||
| jQuery( "<div></div>text<span></span>" ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'd prefer something more obvious here such asjQuery( document.createTextNode("text") ), but this works as long as the behavior doesn't change (do we test that, though?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@gibson042 Your suggestion looks good to me. What do you mean by "as long as the behavior doesn't change", though? What behavior?
There was a check in jQuery.event.add that was supposed to make it a noopfor objects that don't accept data like text or comment nodes. The problem wasthe check was incorrect: it assumed `dataPriv.get( elem )` returns a falsyvalue for an `elem` that doesn't accept data but that's not the case - we getan empty object then. The check was changed to use `acceptData` directly.Fixesjquerygh-4397
There was a check in jQuery.event.add that was supposed to make it a noopfor objects that don't accept data like text or comment nodes. The problem wasthe check was incorrect: it assumed `dataPriv.get( elem )` returns a falsyvalue for an `elem` that doesn't accept data but that's not the case - we getan empty object then. The check was changed to use `acceptData` directly.(cherry picked fromd5c505e)Fixesgh-4397Closesgh-4558
Uh oh!
There was an error while loading.Please reload this page.
Summary
There was a check in jQuery.event.add that was supposed to make it a noop
for objects that don't accept data like text or comment nodes. The problem was
the check was incorrect: it assumed
dataPriv.get( elem )returns a falsyvalue for an
elemthat doesn't accept data but that's not the case - we getan empty object then. The check was changed to use
acceptDatadirectly.Fixesgh-4397
It's funny that this has been the behavior for at least the past few years so this code has been wrong for a long time. 😱
We'll also need to CP that to
3.x-stable+2 bytes
Checklist
If needed, a docs issue/PR was created athttps://github.com/jquery/api.jquery.com