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

Already on GitHub? Sign in to your account

Using "hasOwnProperty" to check for direct properties "type" and #1153

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

andrewplummer commented Jan 31, 2013

This is the same change as #1140, minus a few back and forth commits that were reverted, and with a simpler unit test, as per @gnarf37's request.

@rwaldron rwaldron commented on the diff Jan 31, 2013

src/event.js
@@ -208,8 +208,8 @@ jQuery.event = {
var i, cur, tmp, bubbleType, ontype, handle, special,
eventPath = [ elem || document ],
- type = event.type || event,
- namespaces = event.namespace ? event.namespace.split(".") : [];
+ type = core_hasOwn.call(event, 'type') ? event.type : event,
@rwaldron

rwaldron Jan 31, 2013

Member

Double quotes

@rwaldron rwaldron commented on the diff Jan 31, 2013

src/event.js
@@ -208,8 +208,8 @@ jQuery.event = {
var i, cur, tmp, bubbleType, ontype, handle, special,
eventPath = [ elem || document ],
- type = event.type || event,
- namespaces = event.namespace ? event.namespace.split(".") : [];
+ type = core_hasOwn.call(event, 'type') ? event.type : event,
+ namespaces = core_hasOwn.call(event, 'namespace') ? event.namespace.split(".") : [];
@rwaldron

rwaldron Jan 31, 2013

Member

Double quotes

@rwaldron rwaldron commented on the diff Jan 31, 2013

test/unit/event.js
@@ -2666,3 +2666,17 @@ test( "Check order of focusin/focusout events", 2, function() {
input.off();
});
+test("make sure defining 'namespace' on String.prototype does not cause trigger() to error", function() {
+ expect(1);
+ var errored = false;
+ String.prototype.namespace = function() {
+ return "test";
+ };
+ try {
+ jQuery("<p>").trigger('foo.bar');
@rwaldron

rwaldron Jan 31, 2013

Member

Double quotes

@rwaldron rwaldron commented on the diff Jan 31, 2013

test/unit/event.js
@@ -2666,3 +2666,17 @@ test( "Check order of focusin/focusout events", 2, function() {
input.off();
});
+test("make sure defining 'namespace' on String.prototype does not cause trigger() to error", function() {
+ expect(1);
+ var errored = false;
+ String.prototype.namespace = function() {
+ return "test";
+ };
+ try {
+ jQuery("<p>").trigger('foo.bar');
+ } catch( e ) {
+ errored = true;
+ }
+ equal(errored, false, 'trigger() should not have errored');
@rwaldron

rwaldron Jan 31, 2013

Member

Fix whitespace and use double quotes

Owner

gnarf commented Jan 31, 2013

@rwldrn I just took care of the whitespace/quotes myself and made a couple of tweaks. Thanks for pointing em out tho. @andrewplummer http://contribute.jquery.org/style-guide/js/

@gnarf gnarf added a commit that referenced this pull request Jan 31, 2013

@andrewplummer @gnarf andrewplummer + gnarf event: Stricter type checking in trigger - Fixes #13360 - Closes gh-1153


Squashed commit of the following:

commit 5935a36
Author: Andrew Plummer <plummer.andrew@gmail.com>
Date:   Fri Feb 1 02:40:42 2013 +0900

    Using "hasOwnProperty" to check for direct properties "type" and
    "namespace" on events before triggering.
f005af5

@gnarf gnarf closed this in d654914 Jan 31, 2013

Owner

gnarf commented Jan 31, 2013

Landed in 1.9-stable and master.

Contributor

andrewplummer commented Jan 31, 2013

@gnarf37 thank you!

Sorry, force of habit.

Owner

gnarf commented Jan 31, 2013

⭐️ Thanks for being persistant, and listening, and above all the contribution. ⭐️

👍

Contributor

andrewplummer commented Jan 31, 2013

Of course! In a sense I'm asking for a lot, I realize that.
Here's hoping there's no more.

@mescoda mescoda pushed a commit to mescoda/jquery that referenced this pull request Nov 4, 2014

@andrewplummer @gnarf andrewplummer + gnarf event: Stricter type checking in trigger - Fixes #13360 - Closes gh-1153


Squashed commit of the following:

commit 5935a36
Author: Andrew Plummer <plummer.andrew@gmail.com>
Date:   Fri Feb 1 02:40:42 2013 +0900

    Using "hasOwnProperty" to check for direct properties "type" and
    "namespace" on events before triggering.
4d784db
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment