New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ajaxError and ajaxComplete firing twice upon abort #1775

Closed
mgol opened this Issue Oct 21, 2014 · 5 comments

Comments

Projects
None yet
4 participants
@mgol
Member

mgol commented Oct 21, 2014

Originally reported by Dwelle at: http://bugs.jquery.com/ticket/15160

Aborting jqXHR objects inside ajaxSend results in ajaxError and ajaxComplete callbacks being called twice. On first execution, jqXHR's status is "abort", on second execution, it throws TypeError: Cannot read property 'send' of undefined .

JSFiddle:  http://jsfiddle.net/LK2g3/

tested on Chrome 35, FF 30, jQuery 1.9.1, 1.11.1, 2.1.1

Issue reported for jQuery 1.11.1

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 21, 2014

Member

Comment author: dmethvin

Looks like this has been around for a while. Aborting happens synchronously and cleans up the request to avoid memory bloat. Then when it gets back from the ajaxSend it actually tries to send the request and the XHR object was blown out from under it.

Aborting the request doesn't make a lot of sense at that point, you can see from the XHR spec that it's  pretty much a no-op if called before a .send(). So it seems more appropriate to have ajaxSend indicate that it doesn't want the request to proceed in some other way. The concern I'd have with making a change there would be compat. At least we know aborting doesn't currently work.

I'll mark this open for discussion.

Member

mgol commented Oct 21, 2014

Comment author: dmethvin

Looks like this has been around for a while. Aborting happens synchronously and cleans up the request to avoid memory bloat. Then when it gets back from the ajaxSend it actually tries to send the request and the XHR object was blown out from under it.

Aborting the request doesn't make a lot of sense at that point, you can see from the XHR spec that it's  pretty much a no-op if called before a .send(). So it seems more appropriate to have ajaxSend indicate that it doesn't want the request to proceed in some other way. The concern I'd have with making a change there would be compat. At least we know aborting doesn't currently work.

I'll mark this open for discussion.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 21, 2014

Member

Comment author: Azriel

I've opened a PR with a suggested solution to this here:  https://github.com/jquery/jquery/pull/1619

Member

mgol commented Oct 21, 2014

Comment author: Azriel

I've opened a PR with a suggested solution to this here:  https://github.com/jquery/jquery/pull/1619

@jabelman

This comment has been minimized.

Show comment
Hide comment
@jabelman

jabelman Oct 31, 2014

Hey, I am relatively new to open source/github. I would like to start contributing to projects soon. Out of curiosity and a desire to learn how to make effective contributions in the future, I want to ask why the PR hasn't been merged yet?

jabelman commented Oct 31, 2014

Hey, I am relatively new to open source/github. I would like to start contributing to projects soon. Out of curiosity and a desire to learn how to make effective contributions in the future, I want to ask why the PR hasn't been merged yet?

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Oct 31, 2014

Member

Hey @jabelman I took a look at the earlier and think it's fine. @jaubourg or others, any comments on gh-1619?

Quite often we batch these things up and don't get serious about landing PRs until we start the push for a new version. If you've already signed our CLA you're in good shape and we will land it soon. Thanks for contributing!

Member

dmethvin commented Oct 31, 2014

Hey @jabelman I took a look at the earlier and think it's fine. @jaubourg or others, any comments on gh-1619?

Quite often we batch these things up and don't get serious about landing PRs until we start the push for a new version. If you've already signed our CLA you're in good shape and we will land it soon. Thanks for contributing!

@jaubourg

This comment has been minimized.

Show comment
Hide comment
@jaubourg

jaubourg Nov 1, 2014

Member

The PR looks good to me 👍

Member

jaubourg commented Nov 1, 2014

The PR looks good to me 👍

@timmywil timmywil closed this in 598ed05 Nov 1, 2014

timmywil added a commit that referenced this issue Nov 1, 2014

mescoda pushed a commit to mescoda/jquery that referenced this issue Nov 4, 2014

@dmethvin dmethvin added this to the 3.0.0 milestone Dec 8, 2014

@dmethvin dmethvin added Bug Ajax labels Dec 8, 2014

@dmethvin dmethvin modified the milestones: 1.12/2.2, 3.0.0 Jan 7, 2016

@cssmagic cssmagic referenced this issue May 18, 2016

Open

jQuery #5

@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.