Check if selector is not typeof string _and_ not null before reassigning arg values when on(event-map [, selector][, data]). Fixes #11130 #652

Closed
wants to merge 2 commits into from

2 participants

@rwaldron
jQuery Foundation member

For review.

See: http://bugs.jquery.com/ticket/11130

Building ./dist/jquery.js
Minifying jQuery ./dist/jquery.min.js
Checking jQuery against JSHint...
JSHint check passed.
jQuery Size - compared to last make
  249648    (+19) jquery.js
   94059     (+9) jquery.min.js
   33388     (+3) jquery.min.js.gz
jQuery build complete.
@rwaldron rwaldron Check if selector is not typeof string _and_ not null before reassign…
…ing arg values when on(event-map [, selector][, data]). fixes #11130
2cbe58c
@staabm staabm and 1 other commented on an outdated diff Jan 5, 2012
@@ -867,7 +867,7 @@ jQuery.fn.extend({
// Types can be a map of types/handlers
if ( typeof types === "object" ) {
// ( types-Object, selector, data )
- if ( typeof selector !== "string" ) {
+ if ( typeof selector !== "string" && selector != null ) {
@staabm
staabm added a note Jan 5, 2012

intended non-strict check here?

don't know if performance matters, but shouldn't it be faster to check for null first?

@rwaldron
jQuery Foundation member
rwaldron added a note Jan 5, 2012

non-strict b/c strict === null breaks bind/unbind/one with data behaviour

The non-coerced typeof check is probably faster, but likely not to a statistically significant degree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@gibson042 gibson042 and 1 other commented on an outdated diff Jan 5, 2012
@@ -867,7 +867,7 @@ jQuery.fn.extend({
// Types can be a map of types/handlers
if ( typeof types === "object" ) {
// ( types-Object, selector, data )
- if ( typeof selector !== "string" ) {
+ if ( typeof selector !== "string" && selector != null ) {
// ( types-Object, data )
data = selector;
@gibson042
jQuery Foundation member

The size increase can be even smaller (+8/3/2) by making this assignment data = data || selector; instead of changing the if condition.

@rwaldron
jQuery Foundation member
rwaldron added a note Jan 5, 2012

Nice one - updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dmethvin
jQuery Foundation member

Landed. c0da49f

@dmethvin dmethvin closed this Jan 13, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment