Side navigation
#10202 closed bug (invalid)
Opened September 04, 2011 09:41PM UTC
Closed October 12, 2011 12:47AM UTC
Last modified March 14, 2012 09:22AM UTC
`data` attribute disappears from settings for ajax GET requests
Reported by: | steve@alfajango.com | Owned by: | |
---|---|---|---|
Priority: | low | Milestone: | 1.next |
Component: | ajax | Version: | 1.6.3 |
Keywords: | Cc: | jaubourg | |
Blocked by: | Blocking: |
Description
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.
Attachments (0)
Change History (18)
Changed September 05, 2011 12:35AM UTC by comment:1
component: | unfiled → ajax |
---|---|
priority: | undecided → low |
resolution: | → worksforme |
status: | new → closed |
Changed September 05, 2011 12:54AM UTC by comment:2
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.
Changed September 05, 2011 01:02AM UTC by comment:3
resolution: | worksforme |
---|---|
status: | closed → reopened |
Whoops! Thanks for noting and thanks for your patience and persistence - greatly appreciated.
Changed September 05, 2011 01:06AM UTC by comment:4
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.
Changed September 05, 2011 01:14AM UTC by comment:5
cc: | → jaubourg |
---|
I'll cc jaubourg on this, maybe he has some insight, in the meantime, yes a test case would be awesome.
Changed September 05, 2011 10:42AM UTC by comment:6
http://bugs.jquery.com/ticket/9682
So, yes, data is removed on purpose for requests with no body.
Changed September 05, 2011 02:26PM UTC by comment:7
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?
Changed September 05, 2011 02:30PM UTC by comment:8
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?
Changed September 05, 2011 02:33PM UTC by comment:9
Replying to [comment:8 steve@…]:
For that other bug, if we're going to alter thesettings
object anyway, wouldn't have made way more sense to change theprocessData
attribute tofalse
rather than delete thedata
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.
Changed September 21, 2011 12:32AM UTC by comment:10
milestone: | None → 1.next |
---|---|
status: | reopened → open |
I'll mark this as a valid regression.
Changed September 21, 2011 07:42AM UTC by comment:11
Replying to [comment:10 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.
Changed September 21, 2011 02:20PM UTC by comment:12
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; }
Changed September 21, 2011 02:54PM UTC by comment:13
Replying to [comment:12 Steve Schwartz <steve@…>]:
It seems easy to avoid this problem in the first place. Right now, jQuery ''always'' appends the serializeddata
attribute tourl
. So, to fix that other ticket, jQuery was modified to remove thedata
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 } });
Changed September 21, 2011 04:07PM UTC by comment:14
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.
Changed October 11, 2011 10:55AM UTC by comment:15
_comment0: | Replying to [comment:14 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 \ } \ }); \ }}} \ \ → 1318330547330610 |
---|
Replying to [comment:14 Steve Schwartz <steve@…>]:
I don't understand. That would result inurl: 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 thedata
attribute.
What about:
$.ajax({ url: "myPath?hello=world", data: { ello: worl } });
?
Changed October 11, 2011 08:32PM UTC by comment:16
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.
Changed October 12, 2011 12:47AM UTC by comment:17
resolution: | → invalid |
---|---|
status: | open → closed |
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).
Changed October 12, 2011 01:55AM UTC by comment:18
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 withxhr.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.
Works for me... http://jsfiddle.net/rwaldron/HnVjX/2/