Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

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

4 participants

@rwaldron
Collaborator

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
src/event.js
@@ -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

intended non-strict check here?

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

@rwaldron Collaborator
rwaldron added a note

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
src/event.js
@@ -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 Collaborator

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

@rwaldron Collaborator
rwaldron added a note

Nice one - updated.

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

Landed. c0da49f

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

    Check if selector is not typeof string _and_ not null before reassign…

    rwaldron authored
    …ing arg values when on(event-map [, selector][, data]). fixes #11130
  2. @rwaldron

    Smaller fix

    rwaldron authored
This page is out of date. Refresh to see the latest.
Showing with 22 additions and 6 deletions.
  1. +2 −2 src/event.js
  2. +20 −4 test/unit/event.js
View
4 src/event.js
@@ -867,9 +867,9 @@ 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;
+ data = data || selector;
selector = undefined;
}
for ( type in types ) {
View
24 test/unit/event.js
@@ -1197,7 +1197,7 @@ test("Delegated events in SVG (#10791)", function() {
'<rect id="svg-by-id" x="10" y="20" width="100" height="60" r="10" rx="10" ry="10"></rect>'+
'</svg>'
).appendTo( "body" );
-
+
jQuery( "body" )
.on( "click", "#svg-by-id", function() {
ok( true, "delegated id selector" );
@@ -1222,7 +1222,7 @@ test("Delegated events in forms (#10844)", function() {
'<input type="text" name="id" value="secret agent man" />'+
'</form>'
).appendTo( "body" );
-
+
jQuery( "body" )
.on( "submit", "#myform", function() {
ok( true, "delegated id selector with aliased name" );
@@ -2588,7 +2588,7 @@ test("special bind/delegate name mapping", function() {
// Ensure .one() events are removed after their maiden voyage
jQuery( '<p>Gut Feeling</p>' )
.one( "gutfeeling", jQuery.noop )
- .trigger( "gutfeeling" ) // This one should
+ .trigger( "gutfeeling" ) // This one should
.trigger( "gutfeeling" ) // This one should not
.remove();
@@ -2603,7 +2603,7 @@ test(".on and .off, selective mixed removal (#10705)", function() {
ok( true, "triggered " + e.type );
};
- jQuery( '<p>Strange Pursuit</p>' )
+ jQuery( "<p>Strange Pursuit</p>" )
.on( "click", timingx )
.on( "click.duty", timingx )
.on( "click.now", timingx )
@@ -2618,6 +2618,22 @@ test(".on and .off, selective mixed removal (#10705)", function() {
.trigger( "click" ); // 0
});
+test(".on( event-map, null-selector, data ) #11130", function() {
+
+ expect( 1 );
+
+ var $p = jQuery("<p>Strange Pursuit</p>"),
+ data = "bar",
+ map = {
+ "foo": function( event ) {
+ equal( event.data, "bar", "event.data correctly relayed with null selector" );
+ $p.remove();
+ }
+ };
+
+ $p.on( map, null, data ).trigger("foo");
+});
+
test("delegated events quickIs", function() {
expect(14);
var markup = jQuery(
Something went wrong with that request. Please try again.