Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix #12989. Remove ajax "global" event behavior.

  • Loading branch information...
commit c2d6847de09a52496f78baebc04f317e11ece6d2 1 parent b382af6
@dmethvin dmethvin authored
Showing with 43 additions and 143 deletions.
  1. +11 −27 src/event.js
  2. +32 −116 test/unit/ajax.js
View
38 src/event.js
@@ -198,33 +198,30 @@ jQuery.event = {
},
trigger: function( event, data, elem, onlyHandlers ) {
- // Don't do events on text and comment nodes
- if ( elem && (elem.nodeType === 3 || elem.nodeType === 8) ) {
- return;
- }
- // Event object or event type
- var cache, i, cur, old, ontype, special, handle, eventPath, bubbleType,
+ var i, cur, old, ontype, special, handle, eventPath, bubbleType,
type = event.type || event,
namespaces = event.namespace ? event.namespace.split(".") : [];
+ elem = elem || document;
+
+ // Don't do events on text and comment nodes
+ if ( elem.nodeType === 3 || elem.nodeType === 8 ) {
+ return;
+ }
+
// focus/blur morphs to focusin/out; ensure we're not firing them right now
if ( rfocusMorph.test( type + jQuery.event.triggered ) ) {
return;
}
- if ( type.indexOf( "." ) >= 0 ) {
+ if ( type.indexOf(".") >= 0 ) {
// Namespaced trigger; create a regexp to match event type in handle()
namespaces = type.split(".");
type = namespaces.shift();
namespaces.sort();
}
- if ( !elem && !jQuery.event.global[ type ] ) {
- // No jQuery handlers for this event type, and it can't have inline handlers
- return;
- }
-
// Caller can pass in an Event, Object, or just an event type string
event = typeof event === "object" ?
// jQuery.Event object
@@ -236,22 +233,9 @@ jQuery.event = {
event.type = type;
event.isTrigger = true;
- event.namespace = namespaces.join( "." );
+ event.namespace = namespaces.join(".");
event.namespace_re = event.namespace? new RegExp("(^|\\.)" + namespaces.join("\\.(?:.*\\.|)") + "(\\.|$)") : null;
- ontype = type.indexOf( ":" ) < 0 ? "on" + type : "";
-
- // Handle a global trigger
- if ( !elem ) {
-
- // TODO: Stop taunting the data cache; remove global events and always attach to document
- cache = jQuery.cache;
- for ( i in cache ) {
- if ( cache[ i ].events && cache[ i ].events[ type ] ) {
- jQuery.event.trigger( event, data, cache[ i ].handle.elem, true );
- }
- }
- return;
- }
+ ontype = type.indexOf(":") < 0 ? "on" + type : "";
// Clean up the event in case it is being reused
event.result = undefined;
View
148 test/unit/ajax.js
@@ -7,7 +7,10 @@ module( "ajax", {
return callback;
};
},
- teardown: moduleTeardown
+ teardown: function() {
+ jQuery( document ).off( "ajaxStart ajaxStop ajaxSend ajaxComplete ajaxError ajaxSuccess" );
+ moduleTeardown();
@jaubourg Collaborator
jaubourg added a note

moduleTeardown.apply( this, arguments ); to be safe?

@dmethvin Owner
dmethvin added a note

Yep, good point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
});
(function() {
@@ -16,24 +19,16 @@ module( "ajax", {
return;
}
+ function addGlobalEvents() {
@jaubourg Collaborator
jaubourg added a note

It's a bit too DRY for my taste. Misfires will only be caught in tests because the number of assertions is wrong, supposedly... for moderately complex tests, regressions may be missed entirely.

I'd really prefer if we would give the expected events explicitely:

function addGlobalEvents( expected ) {
    expected = expected || "";
    jQuery( document ).on( "ajaxStart ajaxStop ajaxSend ajaxComplete ajaxError ajaxSuccess", function( e ) {
        ok( expected.indexOf(e.type) !== -1, e.type );
    });
}
@dmethvin Owner
dmethvin added a note

There was only one case that didn't have all of them, but that's a good suggestion to be explicit about the expected events. For the abort test it actually improves the test since it now will fail on unanticipated global events. I'll update these today.

@jaubourg Collaborator
jaubourg added a note

FYI, I'm still working on the ajax unit tests so that things are more coherent and less dependent on server-side logic. So I'm all for better tests, some stuff in unit/ajax.js is just very wrong and easily breakable ;)

@dmethvin Owner
dmethvin added a note

Do you want to make these changes then? The fewer merge conficts the better.

@jaubourg Collaborator
jaubourg added a note

No, go ahead, I'll have gibson's changes to integrate later on anyway. It'll just remind me of the ajax rewrite days :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ jQuery( document ).on( "ajaxStart ajaxStop ajaxSend ajaxComplete ajaxError ajaxSuccess", function( e ) {
+ ok( true, e.type );
+ });
+ }
+
//----------- jQuery.ajax()
ajaxTest( "jQuery.ajax() - success callbacks", 8, {
- setup: function() {
- jQuery("#foo").ajaxStart(function() {
- ok( true, "ajaxStart" );
- }).ajaxStop(function() {
- ok( true, "ajaxStop" );
- }).ajaxSend(function() {
- ok( true, "ajaxSend" );
- }).ajaxComplete(function() {
- ok( true, "ajaxComplete" );
- }).ajaxError(function() {
- ok( false, "ajaxError" );
- }).ajaxSuccess(function() {
- ok( true, "ajaxSuccess" );
- });
- },
+ setup: addGlobalEvents,
@rwaldron Collaborator
rwaldron added a note

This makes me so happy :)

(as well as all the others that follow)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
url: url("data/name.html"),
beforeSend: function() {
ok( true, "beforeSend" );
@@ -47,21 +42,7 @@ module( "ajax", {
});
ajaxTest( "jQuery.ajax() - success callbacks - (url, options) syntax", 8, {
- setup: function() {
- jQuery("#foo").ajaxStart(function() {
- ok( true, "ajaxStart" );
- }).ajaxStop(function() {
- ok( true, "ajaxStop" );
- }).ajaxSend(function() {
- ok( true, "ajaxSend" );
- }).ajaxComplete(function() {
- ok( true, "ajaxComplete" );
- }).ajaxError(function() {
- ok( false, "ajaxError" );
- }).ajaxSuccess(function() {
- ok( true, "ajaxSuccess" );
- });
- },
+ setup: addGlobalEvents,
create: function( options ) {
return jQuery.ajax( url("data/name.html"), options );
},
@@ -77,21 +58,7 @@ module( "ajax", {
});
ajaxTest( "jQuery.ajax() - success callbacks (late binding)", 8, {
- setup: function() {
- jQuery("#foo").ajaxStart(function() {
- ok( true, "ajaxStart" );
- }).ajaxStop(function() {
- ok( true, "ajaxStop" );
- }).ajaxSend(function() {
- ok( true, "ajaxSend" );
- }).ajaxComplete(function() {
- ok( true, "ajaxComplete" );
- }).ajaxError(function() {
- ok( false, "ajaxError" );
- }).ajaxSuccess(function() {
- ok( true, "ajaxSuccess" );
- });
- },
+ setup: addGlobalEvents,
url: url("data/name.html"),
beforeSend: function() {
ok( true, "beforeSend" );
@@ -109,21 +76,7 @@ module( "ajax", {
});
ajaxTest( "jQuery.ajax() - success callbacks (oncomplete binding)", 8, {
- setup: function() {
- jQuery("#foo").ajaxStart(function() {
- ok( true, "ajaxStart" );
- }).ajaxStop(function() {
- ok( true, "ajaxStop" );
- }).ajaxSend(function() {
- ok( true, "ajaxSend" );
- }).ajaxComplete(function() {
- ok( true, "ajaxComplete" );
- }).ajaxError(function() {
- ok( false, "ajaxError" );
- }).ajaxSuccess(function() {
- ok( true, "ajaxSuccess" );
- });
- },
+ setup: addGlobalEvents,
url: url("data/name.html"),
beforeSend: function() {
ok( true, "beforeSend" );
@@ -141,21 +94,7 @@ module( "ajax", {
});
ajaxTest( "jQuery.ajax() - error callbacks", 8, {
- setup: function() {
- jQuery("#foo").ajaxStart(function() {
- ok( true, "ajaxStart" );
- }).ajaxStop(function() {
- ok( true, "ajaxStop" );
- }).ajaxSend(function() {
- ok( true, "ajaxSend" );
- }).ajaxComplete(function() {
- ok( true, "ajaxComplete" );
- }).ajaxError(function() {
- ok( true, "ajaxError" );
- }).ajaxSuccess(function() {
- ok( false, "ajaxSuccess" );
- });
- },
+ setup: addGlobalEvents,
url: url("data/name.php?wait=5"),
beforeSend: function() {
ok( true, "beforeSend" );
@@ -237,7 +176,7 @@ module( "ajax", {
ajaxTest( "jQuery.ajax() - headers", 4, {
setup: function() {
- jQuery("#foo").ajaxSend(function( evt, xhr ) {
+ jQuery( document ).ajaxSend(function( evt, xhr ) {
xhr.setRequestHeader( "ajax-send", "test" );
});
},
@@ -394,18 +333,8 @@ module( "ajax", {
];
});
- ajaxTest( "jQuery.ajax() - abort", 8, {
- setup: function() {
- jQuery("#foo").ajaxStart(function() {
- ok( true, "ajaxStart" );
- }).ajaxStop(function() {
- ok( true, "ajaxStop" );
- }).ajaxSend(function() {
- ok( true, "ajaxSend" );
- }).ajaxComplete(function() {
- ok( true, "ajaxComplete" );
- });
- },
+ ajaxTest( "jQuery.ajax() - abort", 9, {
+ setup: addGlobalEvents,
url: url("data/name.php?wait=5"),
beforeSend: function() {
ok( true, "beforeSend" );
@@ -660,14 +589,14 @@ module( "ajax", {
var success = function() {
successCount++;
};
- jQuery("#foo").ajaxError(function( e, xml, s, ex ) {
+ jQuery( document ).on( "ajaxError.passthru", function( e, xml, s, ex ) {
errorCount++;
errorEx += ": " + xml.status;
});
- jQuery("#foo").one( "ajaxStop", function() {
+ jQuery( document ).one( "ajaxStop", function() {
equal( successCount, 5, "Check all ajax calls successful" );
equal( errorCount, 0, "Check no ajax errors (status" + errorEx + ")" );
- jQuery("#foo").unbind("ajaxError");
+ jQuery( document ).off("ajaxError.passthru");
start();
});
@@ -1592,7 +1521,7 @@ module( "ajax", {
pass = function() {
ok( passed++ < 2, "Error callback executed" );
if ( passed == 2 ) {
- jQuery("#qunit-fixture").unbind("ajaxError");
+ jQuery( document ).off("ajaxError.setupTest");
start();
}
},
@@ -1601,7 +1530,7 @@ module( "ajax", {
start();
};
- jQuery("#qunit-fixture").ajaxError( pass );
+ jQuery( document ).on( "ajaxError.setupTest", pass );
jQuery.ajaxSetup({
timeout: 1000
@@ -1767,20 +1696,8 @@ module( "ajax", {
});
asyncTest( "jQuery.fn.load() - 404 error callbacks", 6, function() {
- jQuery("#foo").ajaxStart(function() {
- ok( true, "ajaxStart" );
- }).ajaxStop(function() {
- ok( true, "ajaxStop" );
- start();
- }).ajaxSend(function() {
- ok( true, "ajaxSend" );
- }).ajaxComplete(function() {
- ok( true, "ajaxComplete" );
- }).ajaxError(function() {
- ok( true, "ajaxError" );
- }).ajaxSuccess(function() {
- ok( false, "ajaxSuccess" );
- });
+ addGlobalEvents();
+ jQuery( document ).ajaxStop( start );
jQuery("<div/>").load( "data/404.html", function() {
ok( true, "complete" );
});
@@ -1921,9 +1838,9 @@ module( "ajax", {
jQuery.ajaxSetup({
dataType: "json"
});
- jQuery("#first").ajaxComplete(function( e, xml, s ) {
+ jQuery( document ).ajaxComplete(function( e, xml, s ) {
strictEqual( s.dataType, "html", "Verify the load() dataType was html" );
- jQuery("#first").unbind("ajaxComplete");
+ jQuery( document ).unbind("ajaxComplete");
start();
});
jQuery("#first").load("data/test3.html");
@@ -1938,12 +1855,11 @@ module( "ajax", {
"foo": "bar"
}
});
- jQuery("#foo")
- .load( "data/echoQuery.php", data )
- .ajaxComplete(function( event, jqXHR, options ) {
- ok( ~options.data.indexOf("foo=bar"), "Data from ajaxSettings was used" );
- start();
- });
+ jQuery("#foo").load( "data/echoQuery.php", data );
+ jQuery( document ).ajaxComplete(function( event, jqXHR, options ) {
+ ok( ~options.data.indexOf("foo=bar"), "Data from ajaxSettings was used" );
+ start();
+ });
});
//----------- jQuery.post()
@jaubourg
Collaborator

It's a bit too DRY for my taste. Misfires will only be caught in tests because the number of assertions is wrong, supposedly... for moderately complex tests, regressions may be missed entirely.

I'd really prefer if we would give the expected events explicitely:

function addGlobalEvents( expected ) {
    expected = expected || "";
    jQuery( document ).on( "ajaxStart ajaxStop ajaxSend ajaxComplete ajaxError ajaxSuccess", function( e ) {
        ok( expected.indexOf(e.type) !== -1, e.type );
    });
}
@jaubourg
Collaborator

moduleTeardown.apply( this, arguments ); to be safe?

@dmethvin

Yep, good point.

@dmethvin

There was only one case that didn't have all of them, but that's a good suggestion to be explicit about the expected events. For the abort test it actually improves the test since it now will fail on unanticipated global events. I'll update these today.

@jaubourg
Collaborator

FYI, I'm still working on the ajax unit tests so that things are more coherent and less dependent on server-side logic. So I'm all for better tests, some stuff in unit/ajax.js is just very wrong and easily breakable ;)

@dmethvin

Do you want to make these changes then? The fewer merge conficts the better.

@jaubourg
Collaborator

No, go ahead, I'll have gibson's changes to integrate later on anyway. It'll just remind me of the ajax rewrite days :P

@rwaldron
Collaborator

This makes me so happy :)

(as well as all the others that follow)

Please sign in to comment.
Something went wrong with that request. Please try again.