Fixes #11151, #13388. Minor refactor of response conversion and when/where

 responseXXX fields are set on the jqXHR. Close gh-1164.

(Cherry-picked from 69b3d5ce0f081d3f113b2917495f35df160f8522)

diff --git a/src/ajax.js b/src/ajax.js
index 55250494a0d3cd37714a972d8ec524f032a8b937..f12b231a89b52471b425a6353aa74c0c83f1e199 100644 (file)
--- a/src/ajax.js
+++ b/src/ajax.js
@@ -258,7 +258,8 @@ jQuery.extend({
 
                responseFields: {
                        xml: "responseXML",
-                       text: "responseText"
+                       text: "responseText",
+                       json: "responseJSON"
                },
 
                // Data converters
@@ -604,13 +605,19 @@ jQuery.extend({
                        // Set readyState
                        jqXHR.readyState = status > 0 ? 4 : 0;
 
+                       // Determine if successful
+                       isSuccess = status >= 200 && status < 300 || status === 304;
+
                        // Get response data
                        if ( responses ) {
                                response = ajaxHandleResponses( s, jqXHR, responses );
                        }
 
+                       // Convert no matter what (that way responseXXX fields are always set)
+                       response = ajaxConvert( s, response, jqXHR, isSuccess );
+
                        // If successful, handle type chaining
-                       if ( status >= 200 && status < 300 || status === 304 ) {
+                       if ( isSuccess ) {
 
                                // Set the If-Modified-Since and/or If-None-Match header, if in ifModified mode.
                                if ( s.ifModified ) {
@@ -626,20 +633,17 @@ jQuery.extend({
 
                                // if no content
                                if ( status === 204 ) {
-                                       isSuccess = true;
                                        statusText = "nocontent";
 
                                // if not modified
                                } else if ( status === 304 ) {
-                                       isSuccess = true;
                                        statusText = "notmodified";
 
                                // If we have data, let's convert it
                                } else {
-                                       isSuccess = ajaxConvert( s, response );
-                                       statusText = isSuccess.state;
-                                       success = isSuccess.data;
-                                       error = isSuccess.error;
+                                       statusText = response.state;
+                                       success = response.data;
+                                       error = response.error;
                                        isSuccess = !error;
                                }
                        } else {
@@ -699,7 +703,6 @@ jQuery.extend({
 });
 
 /* Handles responses to an ajax request:
- * - sets all responseXXX fields accordingly
  * - finds the right dataType (mediates between content-type and expected dataType)
  * - returns the corresponding response
  */
@@ -707,15 +710,7 @@ function ajaxHandleResponses( s, jqXHR, responses ) {
 
        var ct, type, finalDataType, firstDataType,
                contents = s.contents,
-               dataTypes = s.dataTypes,
-               responseFields = s.responseFields;
-
-       // Fill responseXXX fields
-       for ( type in responseFields ) {
-               if ( type in responses ) {
-                       jqXHR[ responseFields[type] ] = responses[ type ];
-               }
-       }
+               dataTypes = s.dataTypes;
 
        // Remove auto dataType and get content-type in the process
        while( dataTypes[ 0 ] === "*" ) {
@@ -764,20 +759,14 @@ function ajaxHandleResponses( s, jqXHR, responses ) {
        }
 }
 
-// Chain conversions given the request and the original response
-function ajaxConvert( s, response ) {
-
-       var conv, conv2, current, tmp,
+/* Chain conversions given the request and the original response
+ * Also sets the responseXXX fields on the jqXHR instance
+ */
+function ajaxConvert( s, response, jqXHR, isSuccess ) {
+       var conv2, current, conv, tmp, prev,
                converters = {},
-               i = 0,
                // Work with a copy of dataTypes in case we need to modify it for conversion
-               dataTypes = s.dataTypes.slice(),
-               prev = dataTypes[ 0 ];
-
-       // Apply the dataFilter if provided
-       if ( s.dataFilter ) {
-               response = s.dataFilter( response, s.dataType );
-       }
+               dataTypes = s.dataTypes.slice();
 
        // Create converters map with lowercased keys
        if ( dataTypes[ 1 ] ) {
@@ -786,14 +775,32 @@ function ajaxConvert( s, response ) {
                }
        }
 
-       // Convert to each sequential dataType, tolerating list modification
-       for ( ; (current = dataTypes[++i]); ) {
+       current = dataTypes.shift();
+
+       // Convert to each sequential dataType
+       while ( current ) {
+
+               if ( s.responseFields[ current ] ) {
+                       jqXHR[ s.responseFields[ current ] ] = response;
+               }
+
+               // Apply the dataFilter if provided
+               if ( !prev && isSuccess && s.dataFilter ) {
+                       response = s.dataFilter( response, s.dataType );
+               }
+
+               prev = current;
+               current = dataTypes.shift();
+
+               if ( current ) {
 
                // There's only work to do if current dataType is non-auto
-               if ( current !== "*" ) {
+                       if ( current === "*" ) {
+
+                               current = prev;
 
                        // Convert response if prev dataType is non-auto and differs from current
-                       if ( prev !== "*" && prev !== current ) {
+                       } else if ( prev !== "*" && prev !== current ) {
 
                                // Seek a direct converter
                                conv = converters[ prev + " " + current ] || converters[ "* " + current ];
@@ -803,7 +810,7 @@ function ajaxConvert( s, response ) {
                                        for ( conv2 in converters ) {
 
                                                // If conv2 outputs current
-                                               tmp = conv2.split(" ");
+                                               tmp = conv2.split( " " );
                                                if ( tmp[ 1 ] === current ) {
 
                                                        // If prev can be converted to accepted input
@@ -817,9 +824,8 @@ function ajaxConvert( s, response ) {
                                                                // Otherwise, insert the intermediate dataType
                                                                } else if ( converters[ conv2 ] !== true ) {
                                                                        current = tmp[ 0 ];
-                                                                       dataTypes.splice( i--, 0, current );
+                                                                       dataTypes.unshift( tmp[ 1 ] );
                                                                }
-
                                                                break;
                                                        }
                                                }
@@ -830,7 +836,7 @@ function ajaxConvert( s, response ) {
                                if ( conv !== true ) {
 
                                        // Unless errors are allowed to bubble, catch and return them
-                                       if ( conv && s["throws"] ) {
+                                       if ( conv && s[ "throws" ] ) {
                                                response = conv( response );
                                        } else {
                                                try {
@@ -841,9 +847,6 @@ function ajaxConvert( s, response ) {
                                        }
                                }
                        }
-
-                       // Update prev for next iteration
-                       prev = current;
                }
        }
 

diff --git a/test/data/errorWithJSON.php b/test/data/errorWithJSON.php
new file mode 100644 (file)
index 0000000..62b187e
--- /dev/null
+++ b/test/data/errorWithJSON.php
@@ -0,0 +1,6 @@
+<?php
+
+header("HTTP/1.0 400 Bad Request");
+header("Content-Type: application/json");
+
+echo '{ "code": 40, "message": "Bad Request" }';
\ No newline at end of file

diff --git a/test/unit/ajax.js b/test/unit/ajax.js
index e5e82f97795c51c355c15b64473fb02ec516ee5f..e2335e50725db88494d0cda3f8b6b1475b506749 100644 (file)
--- a/test/unit/ajax.js
+++ b/test/unit/ajax.js
@@ -1378,6 +1378,18 @@ module( "ajax", {
 
        });
 
+       ajaxTest( "#11151 - jQuery.ajax() - parse error body", 2, {
+               url: url("data/errorWithJSON.php"),
+               dataFilter: function( string ) {
+                       ok( false, "dataFilter called" );
+                       return string;
+               },
+               error: function( jqXHR ) {
+                       strictEqual( jqXHR.responseText, "{ \"code\": 40, \"message\": \"Bad Request\" }", "Error body properly set" );
+                       deepEqual( jqXHR.responseJSON, { code: 40, message: "Bad Request" }, "Error body properly parsed" );
+               }
+       });
+
        ajaxTest( "#11426 - jQuery.ajax() - loading binary data shouldn't throw an exception in IE", 1, {
                url: url("data/1x1.jpg"),
                success: function( data ) {
@@ -1477,6 +1489,16 @@ module( "ajax", {
                }
        });
 
+       ajaxTest( "#13388 - jQuery.ajax() - responseXML", 3, {
+               url: url("data/with_fries.xml"),
+               dataType: "xml",
+               success: function( resp, _, jqXHR ) {
+                       notStrictEqual( resp, undefined, "XML document exists" );
+                       ok( "responseXML" in jqXHR, "jqXHR.responseXML exists" );
+                       strictEqual( resp, jqXHR.responseXML, "jqXHR.responseXML is set correctly" );
+               }
+       });
+
 //----------- jQuery.ajaxPrefilter()
 
        ajaxTest( "jQuery.ajaxPrefilter() - abort", 1, {