Fixes #2994. Not finding a transport now fires the error callbacks an…

…d doesn't make ajax return false. Had to revise how jsonp and script prefilters & transports work (better separation of concerns). Also took the opportunity to revise jXHR getRequestHeader and abort methods and enabled early transport garbage collection when the request completes.
jquery · Jan 16, 2011 · 8ab23aec2c333834a6e442fa15b73125ba857afe · 8ab23ae
1 parent d9cb698
commit 8ab23aec2c333834a6e442fa15b73125ba857afe
Showing 4 changed files with 123 additions and 122 deletions.
diff --git a/src/ajax.js b/src/ajax.js
@@ -306,30 +306,35 @@ jQuery.extend({
 				// (match is used internally)
 				getResponseHeader: function( key , match ) {
 
-					if ( state !== 2 ) {
-						return null;
-					}
+					if ( state === 2 ) {
 
-					if ( responseHeaders === undefined ) {
+						if ( responseHeaders === undefined ) {
 
-						responseHeaders = {};
+							responseHeaders = {};
 
-						if ( typeof responseHeadersString === "string" ) {
+							if ( typeof responseHeadersString === "string" ) {
 
-							while( ( match = rheaders.exec( responseHeadersString ) ) ) {
-								responseHeaders[ match[ 1 ].toLowerCase() ] = match[ 2 ];
+								while( ( match = rheaders.exec( responseHeadersString ) ) ) {
+									responseHeaders[ match[ 1 ].toLowerCase() ] = match[ 2 ];
+								}
 							}
 						}
+						match = responseHeaders[ key.toLowerCase() ];
+
+					} else {
+
+						match = null;
 					}
-					return responseHeaders[ key.toLowerCase() ];
+
+					return match;
 				},
 
 				// Cancel the request
 				abort: function( statusText ) {
-					if ( transport && state !== 2 ) {
+					if ( transport ) {
 						transport.abort( statusText || "abort" );
-						done( 0 , statusText );
 					}
+					done( 0 , statusText );
 					return this;
 				}
 			};
@@ -347,6 +352,10 @@ jQuery.extend({
 			// State is "done" now
 			state = 2;
 
+			// Dereference transport for early garbage collection
+			// (no matter how long the jXHR transport will be used
+			transport = 0;
+
 			// Set readyState
 			jXHR.readyState = status ? 4 : 0;
 
@@ -599,84 +608,87 @@ jQuery.extend({
 			s.data = jQuery.param( s.data , s.traditional );
 		}
 
-		// Get transport
-		transport = jQuery.ajaxPrefilter( s , options ).ajaxTransport( s );
+		// Apply prefilters
+		jQuery.ajaxPrefilter( s , options );
 
 		// Watch for a new set of requests
 		if ( s.global && jQuery.active++ === 0 ) {
 			jQuery.event.trigger( "ajaxStart" );
 		}
 
-		// If no transport, we auto-abort
-		if ( ! transport ) {
-
-			done( 0 , "transport not found" );
-			jXHR = false;
+		// More options handling for requests with no content
+		if ( ! s.hasContent ) {
 
-		} else {
+			// If data is available, append data to url
+			if ( s.data ) {
+				s.url += ( rquery.test( s.url ) ? "&" : "?" ) + s.data;
+			}
 
-			// More options handling for requests with no content
-			if ( ! s.hasContent ) {
+			// Add anti-cache in url if needed
+			if ( s.cache === false ) {
 
-				// If data is available, append data to url
-				if ( s.data ) {
-					s.url += ( rquery.test( s.url ) ? "&" : "?" ) + s.data;
-				}
+				var ts = jQuery.now(),
+					// try replacing _= if it is there
+					ret = s.url.replace( rts , "$1_=" + ts );
 
-				// Add anti-cache in url if needed
-				if ( s.cache === false ) {
+				// if nothing was replaced, add timestamp to the end
+				s.url = ret + ( (ret == s.url ) ? ( rquery.test( s.url ) ? "&" : "?" ) + "_=" + ts : "");
+			}
+		}
 
-					var ts = jQuery.now(),
-						// try replacing _= if it is there
-						ret = s.url.replace( rts , "$1_=" + ts );
+		// Set the correct header, if data is being sent
+		if ( s.data && s.hasContent && s.contentType !== false || options.contentType ) {
+			requestHeaders[ "content-type" ] = s.contentType;
+		}
 
-					// if nothing was replaced, add timestamp to the end
-					s.url = ret + ( (ret == s.url ) ? ( rquery.test( s.url ) ? "&" : "?" ) + "_=" + ts : "");
-				}
+		// Set the If-Modified-Since and/or If-None-Match header, if in ifModified mode.
+		if ( s.ifModified ) {
+			if ( jQuery_lastModified[ s.url ] ) {
+				requestHeaders[ "if-modified-since" ] = jQuery_lastModified[ s.url ];
 			}
-
-			// Set the correct header, if data is being sent
-			if ( s.data && s.hasContent && s.contentType !== false || options.contentType ) {
-				requestHeaders[ "content-type" ] = s.contentType;
+			if ( jQuery_etag[ s.url ] ) {
+				requestHeaders[ "if-none-match" ] = jQuery_etag[ s.url ];
 			}
+		}
 
-			// Set the If-Modified-Since and/or If-None-Match header, if in ifModified mode.
-			if ( s.ifModified ) {
-				if ( jQuery_lastModified[ s.url ] ) {
-					requestHeaders[ "if-modified-since" ] = jQuery_lastModified[ s.url ];
-				}
-				if ( jQuery_etag[ s.url ] ) {
-					requestHeaders[ "if-none-match" ] = jQuery_etag[ s.url ];
-				}
-			}
+		// Set the Accepts header for the server, depending on the dataType
+		requestHeaders.accept = s.dataTypes[ 0 ] && s.accepts[ s.dataTypes[ 0 ] ] ?
+			s.accepts[ s.dataTypes[ 0 ] ] + ( s.dataTypes[ 0 ] !== "*" ? ", */*; q=0.01" : "" ) :
+			s.accepts[ "*" ];
+
+		// Check for headers option
+		for ( i in s.headers ) {
+			requestHeaders[ i.toLowerCase() ] = s.headers[ i ];
+		}
 
-			// Set the Accepts header for the server, depending on the dataType
-			requestHeaders.accept = s.dataTypes[ 0 ] && s.accepts[ s.dataTypes[ 0 ] ] ?
-				s.accepts[ s.dataTypes[ 0 ] ] + ( s.dataTypes[ 0 ] !== "*" ? ", */*; q=0.01" : "" ) :
-				s.accepts[ "*" ];
+		// Allow custom headers/mimetypes and early abort
+		if ( s.beforeSend && ( s.beforeSend.call( callbackContext , jXHR , s ) === false || state === 2 ) ) {
 
-			// Check for headers option
-			for ( i in s.headers ) {
-				requestHeaders[ i.toLowerCase() ] = s.headers[ i ];
+				// Abort if not done already
+				done( 0 , "abort" );
+
+				// Return false
+				jXHR = false;
+
+		} else {
+
+			// Install callbacks on deferreds
+			for ( i in { success:1, error:1, complete:1 } ) {
+				jXHR[ i ]( s[ i ] );
 			}
 
-			// Allow custom headers/mimetypes and early abort
-			if ( s.beforeSend && ( s.beforeSend.call( callbackContext , jXHR , s ) === false || state === 2 ) ) {
+			// Get transport
+			transport = jQuery.ajaxTransport( s );
 
-					// Abort if not done already
-					done( 0 , "abort" );
-					jXHR = false;
+			// If no transport, we auto-abort
+			if ( ! transport ) {
+
+				done( 0 , "notransport" );
 
 			} else {
 
 				// Set state as sending
-				state = 1;
-				jXHR.readyState = 1;
-
-				// Install callbacks on deferreds
-				for ( i in { success:1, error:1, complete:1 } ) {
-					jXHR[ i ]( s[ i ] );
-				}
+				state = jXHR.readyState = 1;
 
 				// Send global event
 				if ( s.global ) {

diff --git a/src/ajax/jsonp.js b/src/ajax/jsonp.js
@@ -11,9 +11,7 @@ jQuery.ajaxSetup({
 		return "jsonp" + jsc++;
 	}
 
-// Normalize jsonp queries
-// 1) put callback parameter in url or data
-// 2) sneakily ensure transportDataType is always jsonp for jsonp requests
+// Detect, normalize options and install callbacks for jsonp requests
 }).ajaxPrefilter("json jsonp", function(s, originalSettings) {
 
 	if ( s.dataTypes[ 0 ] === "jsonp" ||
@@ -22,8 +20,10 @@ jQuery.ajaxSetup({
 		jsre.test(s.url) ||
 		typeof(s.data) === "string" && jsre.test(s.data) ) {
 
-		var jsonpCallback = s.jsonpCallback =
+		var responseContainer,
+			jsonpCallback = s.jsonpCallback =
 				jQuery.isFunction( s.jsonpCallback ) ? s.jsonpCallback() : s.jsonpCallback,
+			previous = window[ jsonpCallback ],
 			url = s.url.replace(jsre, "$1" + jsonpCallback + "$2"),
 			data = s.url === url && typeof(s.data) === "string" ? s.data.replace(jsre, "$1" + jsonpCallback + "$2") : s.data;
 
@@ -33,51 +33,42 @@ jQuery.ajaxSetup({
 
 		s.url = url;
 		s.data = data;
-		s.dataTypes[ 0 ] = "jsonp";
-	}
-
-// Bind transport to jsonp dataType
-}).ajaxTransport("jsonp", function(s) {
 
-	// Put callback in place
-	var responseContainer,
-		jsonpCallback = s.jsonpCallback,
-		previous = window[ jsonpCallback ];
+		window [ jsonpCallback ] = function( response ) {
+			responseContainer = [response];
+		};
 
-	window [ jsonpCallback ] = function( response ) {
-		responseContainer = [response];
-	};
+		s.complete = [function() {
 
-	s.complete = [function() {
+			// Set callback back to previous value
+			window[ jsonpCallback ] = previous;
 
-		// 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] );
+			// 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){}
 			}
-		} else {
-			// else, more memory leak avoidance
-			try{ delete window[ jsonpCallback ]; } catch(e){}
-		}
 
-	}, s.complete ];
+		}, s.complete ];
 
-	// Sneakily ensure this will be handled as json
-	s.dataTypes[ 0 ] = "json";
+		// Use data converter to retrieve json after script execution
+		s.converters["script json"] = function() {
+			if ( ! responseContainer ) {
+				jQuery.error( jsonpCallback + " was not called" );
+			}
+			return responseContainer[ 0 ];
+		};
 
-	// Use data converter to retrieve json after script execution
-	s.converters["script json"] = function() {
-		if ( ! responseContainer ) {
-			jQuery.error( jsonpCallback + " was not called" );
-		}
-		return responseContainer[ 0 ];
-	};
+		// force json dataType
+		s.dataTypes[ 0 ] = "json";
 
-	// Delegate to script transport
-	return "script";
+		// Delegate to script
+		return "script";
+	}
 });
 
 })( jQuery );
diff --git a/src/ajax/script.js b/src/ajax/script.js
@@ -15,18 +15,22 @@ jQuery.ajaxSetup({
 		"text script": jQuery.globalEval
 	}
 
-// Bind script tag hack transport
-}).ajaxTransport("script", function(s) {
+// Handle cache's special case and global
+}).ajaxPrefilter("script", function(s) {
 
-	// Handle cache special case
 	if ( s.cache === undefined ) {
 		s.cache = false;
 	}
 
-	// This transport only deals with cross domain get requests
-	if ( s.crossDomain && s.async && ( s.type === "GET" || ! s.data ) ) {
-
+	if ( s.crossDomain ) {
 		s.global = false;
+	}
+
+// Bind script tag hack transport
+}).ajaxTransport("script", function(s) {
+
+	// This transport only deals with cross domain requests
+	if ( s.crossDomain ) {
 
 		var script,
 			head = document.getElementsByTagName("head")[0] || document.documentElement;

diff --git a/test/unit/ajax.js b/test/unit/ajax.js
@@ -1865,25 +1865,19 @@ test("jQuery ajax - failing cross-domain", function() {
 
 	var i = 2;
 
-	if ( jQuery.ajax({
+	jQuery.ajax({
 		url: 'http://somewebsitethatdoesnotexist-67864863574657654.com',
 		success: function(){ ok( false , "success" ); },
 		error: function(xhr,_,e){ ok( true , "file not found: " + xhr.status + " => " + e ); },
 		complete: function() { if ( ! --i ) start(); }
-	}) === false ) {
-		ok( true , "no transport" );
-		if ( ! --i ) start();
-	}
+	});
 
-	if ( jQuery.ajax({
+	jQuery.ajax({
 		url: 'http://www.google.com',
 		success: function(){ ok( false , "success" ); },
 		error: function(xhr,_,e){ ok( true , "access denied: " + xhr.status + " => " + e ); },
 		complete: function() { if ( ! --i ) start(); }
-	}) === false ) {
-		ok( true , "no transport" );
-		if ( ! --i ) start();
-	}
+	});
 
 });
 
@@ -1937,7 +1931,7 @@ test( "jQuery.ajax - statusCode" , function() {
 			404: function() {
 				ok( ! isSuccess , name );
 			}
-		}
+		};
 	}
 
 	jQuery.each( {