Skip to content
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

PR for #3586 (Safari xhr timeout) #3590

Closed
wants to merge 6 commits into from
Closed

Conversation

eriklax
Copy link
Contributor

@eriklax eriklax commented Mar 23, 2017

This PR fixed the ontimeout callback being trigged by Safari on timeout discussed in #3586.

@jsf-clabot
Copy link

jsf-clabot commented Mar 23, 2017

CLA assistant check
All committers have signed the CLA.

@mention-bot
Copy link

@eriklax, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jaubourg, @markelog and @dmethvin to be potential reviewers.

@@ -527,6 +527,23 @@ QUnit.module( "ajax", {
};
} );

ajaxTest( "jQuery.ajax() - timeout", 2, function( assert ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timeout -> native timeout, similarly to how the native abort test case is named.

src/ajax/xhr.js Outdated
@@ -75,7 +75,8 @@ jQuery.ajaxTransport( function( options ) {
return function() {
if ( callback ) {
callback = errorCallback = xhr.onload =
xhr.onerror = xhr.onabort = xhr.onreadystatechange = null;
xhr.onerror = xhr.onabort = xhr.ontimeout =
xhr.onreadystatechange = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be indented by an additional tab.

Copy link
Contributor Author

@eriklax eriklax Mar 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the last line?

@mgol
Copy link
Member

mgol commented Mar 23, 2017

A few nits but otherwise looks good to me!

@timmywil timmywil added this to the 3.3.0 milestone Mar 27, 2017
@timmywil timmywil closed this in 262acc6 Jul 24, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants