`data` attribute disappears from settings for ajax GET requests

[steve@…]

This is specifically for v1.6.3, everything worked prior to that, from at least 1.4 onward.

If you create an ajax request with jQuery.ajax(type: 'GET', data: someData), and then bind handler to the beforeSubmit event, the data attribute disappears from the settings object. This only happens for GET requests in 1.6.3. The data attribute does not disappear for POST requests, or for any request types in 1.6.2 and earlier.

I don't know if this was intensional or not (since GET requests also happen to serialize the data attribute into the URL), but I didn't see it in the changelog, and it messed up the jquery-ujs in Rails.

See  http://jsfiddle.net/HnVjX/1/

If you change the JS in the above example to be a non-GET type it works, or alternatively if you change it to jquery 1.6.2, it works.

[rwaldron]

Works for me...  http://jsfiddle.net/rwaldron/HnVjX/2/

[anonymous]

Please read the ticket description. You changed the type to "POST". From the original ticket description:

This only happens for GET requests in 1.6.3.

Please re-open this ticket. It is not fixed, and it broke our well-tested jquery-ujs library in core rails.

[rwaldron]

Whoops! Thanks for noting and thanks for your patience and persistence - greatly appreciated.

[steve@…]

No prob, I know how it goes ;-) If I can get some time, and if nobody else can get to it, I could probably come up with a test case and fix in the next week or so.

[rwaldron]

I'll cc jaubourg on this, maybe he has some insight, in the meantime, yes a test case would be awesome.

[jaubourg]

http://bugs.jquery.com/ticket/9682

So, yes, data is removed on purpose for requests with no body.

[steve@…]

Hmm, so it looks like data was purposely removed from the settings object, so that if you retry the ajax request with $.ajax(this), it won't re-append the data to the url.

I can definitely see that use-case, but it seems at least equally likely (if not more) that we have some handler bound to *any* of the event hooks that needs access to settings.data. I'm not sure how breaking one situation to fix the other is a good solution.

Also, in order to workaround the bug in that other scenario, all we had to do is: delete this.data; $.ajax(this). But now, in order to workaround *this* bug, and access the data, we need an entire script to deserialize the url?

[steve@…]

For that other bug, if we're going to alter the settings object anyway, wouldn't have made way more sense to change the processData attribute to false rather than delete the data attribute?

[Steve Schwartz <steve@…>]

Replying to steve@…:

For that other bug, if we're going to alter the settings object anyway, wouldn't have made way more sense to change the processData attribute to false rather than delete the data attribute?

Nope, nevermind, that just affects translating the data from an array to a string, it doesn't actually change whether or not it gets appended to the query string.

[timmywil]

I'll mark this as a valid regression.

[jaubourg]

Replying to timmywil:

I'll mark this as a valid regression.

I completly disagree. This is not a regression.

{
    url: "mypath",
    data: {
        a: 1
    }
}

and

{
    url: "mypath?a=1"
}

are stricly equivalent from an ajax point of view.

If one need to keep track of the original data then:

1. use the original options map (never changed)
2. create a prefilter that "saves" the data in the complete options map

We're just paying the price for giving access to the internal options map in the callbacks, knowing that the exact behaviour of such and such options in that scenario has never ever been documented.

[Steve Schwartz <steve@…>]

It seems easy to avoid this problem in the first place. Right now, jQuery always appends the serialized data attribute to url. So, to fix that other ticket, jQuery was modified to remove the data attribute after the first time through, so that there would not be any data left to append on subsequent attempts.

Rather than deleting the data attribute (which seems destructive), why not just check to see if it's already been done?

// If data is available, append data to url
if ( s.data && s.url.indexOf(s.data) === -1 ) {
	s.url += ( rquery.test( s.url ) ? "&" : "?" ) + s.data;
}

[jaubourg]

Replying to Steve Schwartz <steve@…>:

It seems easy to avoid this problem in the first place. Right now, jQuery always appends the serialized data attribute to url. So, to fix that other ticket, jQuery was modified to remove the data attribute after the first time through, so that there would not be any data left to append on subsequent attempts.

Rather than deleting the data attribute (which seems destructive), why not just check to see if it's already been done?

// If data is available, append data to url
if ( s.data && s.url.indexOf(s.data) === -1 ) {
	s.url += ( rquery.test( s.url ) ? "&" : "?" ) + s.data;
}

Because I could very well do something like this:

$.ajax({
    url: "myPath?hello=world",
    data: {
        field: value
    }
});

[Steve Schwartz <steve@…>]

I don't understand. That would result in url: myPath?hello=world&field=value the first time around, and then it wouldn't change on subsequent tries. This is exactly as jQuery currently does, except that the code I posted doesn't delete the data attribute.

[niyazpk]

Replying to Steve Schwartz <steve@…>:

I don't understand. That would result in url: myPath?hello=world&field=value the first time around, and then it wouldn't change on subsequent tries. This is exactly as jQuery currently does, except that the code I posted doesn't delete the data attribute.

What about:

$.ajax({
    url: "myPath?hello=world",
    data: {
        ello: worl
    }
});

?

[Steve Schwartz <steve@…>]

Ah, that's a good point. Replace my original code example with this:

var dataAppendedToUrl = (new RegExp("(&|\\?)" + s.data + "(&|$)")).test(s.url);
if ( s.data && !dataAppendedToUrl ) {
	s.url += ( rquery.test( s.url ) ? "&" : "?" ) + s.data;
}

I have this coded up, complete with test, and all tests passing. See pull request 544:  https://github.com/jquery/jquery/pull/544.

[jaubourg]

This is not a regression. Rationale and possible adaptation to the now correct behaviour ajax exhibits are given in http://bugs.jquery.com/ticket/10202#comment:11 (I'd go with a prefilter personally).

[Steve Schwartz <steve@…>]

I posted this on the github ticket, but I'll repost here, in response to comment 11 and why {p: 6} and "p=6" are not technically equivalent, and thus why this is still needed.

I don't think it's quite as simple as saying they're equivalent and thus it's not a regression. The problem is this real-world scenario: we have a handler that does something with xhr.data, expecting the object that we *just* passed to .ajax() in the first place. This has been the case since forever, up until 1.6.3. Do {p: 6} and "p=6" contain the same data? Yes. However, jQuery makes it incredibly easy to convert the former to the latter, but *no way* to convert the latter to the former. We'd have to use our own function to convert back (I've done this and it's about 30 lines of code).

Since conversion process between the two forms ({p: 6} and "p=6") is trivial one way and difficult the other way, then they're not equivalent in practice.

If adding 30 lines of code to every plugin and app that needs this can be easily avoided by changing one already-existing line of code in here, why not do it? I understand not wanting to use this solution, rightfully so; but there are a number of other ways to go about it, and I'm more than willing to figure it out.