Makes sure jsonp callback is not left in the global namespace in case…

… of an early abort (beforeSend). Unit test added.
jquery · Jan 31, 2011 · f286a71 · f286a71
1 parent 7c9a5e6
commit f286a71
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 25 deletions.
diff --git a/src/ajax/jsonp.js b/src/ajax/jsonp.js
@@ -12,9 +12,9 @@ jQuery.ajaxSetup({
 });
 
 // Detect, normalize options and install callbacks for jsonp requests
-jQuery.ajaxPrefilter( "json jsonp", function( s, originalSettings, dataIsString /* internal */ ) {
+jQuery.ajaxPrefilter( "json jsonp", function( s, originalSettings, jXHR ) {
 
-	dataIsString = ( typeof s.data === "string" );
+	var dataIsString = ( typeof s.data === "string" );
 
 	if ( s.dataTypes[ 0 ] === "jsonp" ||
 		originalSettings.jsonpCallback ||
@@ -28,7 +28,15 @@ jQuery.ajaxPrefilter( "json jsonp", function( s, originalSettings, dataIsString
 			previous = window[ jsonpCallback ],
 			url = s.url,
 			data = s.data,
-			replace = "$1" + jsonpCallback + "$2";
+			replace = "$1" + jsonpCallback + "$2",
+			cleanUp = function() {
+				// Set callback back to previous value
+				window[ jsonpCallback ] = previous;
+				// Call if it was a function and we have a response
+				if ( responseContainer && jQuery.isFunction( previous ) ) {
+					window[ jsonpCallback ]( responseContainer[ 0 ] );
+				}
+			};
 
 		if ( s.jsonp !== false ) {
 			url = url.replace( jsre, replace );
@@ -46,32 +54,17 @@ jQuery.ajaxPrefilter( "json jsonp", function( s, originalSettings, dataIsString
 		s.url = url;
 		s.data = data;
 
+		// Install callback
 		window[ jsonpCallback ] = function( response ) {
 			responseContainer = [ response ];
 		};
 
-		s.complete = [ function() {
-
-			// Set callback back to previous value
-			window[ jsonpCallback ] = previous;
-
-			// Call if it was a function and we have a response
-			if ( previous) {
-				if ( responseContainer && jQuery.isFunction( previous ) ) {
-					window[ jsonpCallback ] ( responseContainer[ 0 ] );
-				}
-			} else {
-				// else, more memory leak avoidance
-				try{
-					delete window[ jsonpCallback ];
-				} catch( e ) {}
-			}
-
-		}, s.complete ];
+		// Install cleanUp function
+		jXHR.then( cleanUp, cleanUp );
 
 		// Use data converter to retrieve json after script execution
 		s.converters["script json"] = function() {
-			if ( ! responseContainer ) {
+			if ( !responseContainer ) {
 				jQuery.error( jsonpCallback + " was not called" );
 			}
 			return responseContainer[ 0 ];

diff --git a/test/unit/ajax.js b/test/unit/ajax.js
@@ -1221,10 +1221,10 @@ test("jQuery.getScript(String, Function) - no callback", function() {
 jQuery.each( [ "Same Domain", "Cross Domain" ] , function( crossDomain , label ) {
 
 	test("jQuery.ajax() - JSONP, " + label, function() {
-		expect(16);
+		expect(20);
 
 		var count = 0;
-		function plus(){ if ( ++count == 16 ) start(); }
+		function plus(){ if ( ++count == 18 ) start(); }
 
 		stop();
 
@@ -1364,6 +1364,38 @@ jQuery.each( [ "Same Domain", "Cross Domain" ] , function( crossDomain , label )
 			}
 		});
 
+		jQuery.ajax({
+			url: "data/jsonp.php",
+			dataType: "jsonp",
+			crossDomain: crossDomain,
+			jsonpCallback: "functionToCleanUp",
+			success: function(data){
+				ok( data.data, "JSON results returned (GET, custom callback name to be cleaned up)" );
+				strictEqual( window.functionToCleanUp, undefined, "Callback was removed (GET, custom callback name to be cleaned up)" );
+				plus();
+				var xhr;
+				jQuery.ajax({
+					url: "data/jsonp.php",
+					dataType: "jsonp",
+					crossDomain: crossDomain,
+					jsonpCallback: "functionToCleanUp",
+					beforeSend: function( jXHR ) {
+						xhr = jXHR;
+						return false;
+					}
+				});
+				xhr.error(function() {
+					ok( true, "Ajax error JSON (GET, custom callback name to be cleaned up)" );
+					strictEqual( window.functionToCleanUp, undefined, "Callback was removed after early abort (GET, custom callback name to be cleaned up)" );
+					plus();
+				});
+			},
+			error: function(data){
+				ok( false, "Ajax error JSON (GET, custom callback name to be cleaned up)" );
+				plus();
+			}
+		});
+
 		jQuery.ajax({
 			type: "POST",
 			url: "data/jsonp.php",
@@ -1647,7 +1679,7 @@ test("jQuery.post - data", 3, function() {
 			jQuery( 'math', xml ).each( function() {
 				equals( jQuery( 'calculation', this ).text(), '5-2', 'Check for XML' );
 				equals( jQuery( 'result', this ).text(), '3', 'Check for XML' );
-			})
+			});
 		}),
 
 		jQuery.ajax({