Skip to content

Conversation

@timmywil
Copy link
Member

@timmywil timmywil commented Nov 2, 2015

Fixes gh-2079

Copy link
Member

Choose a reason for hiding this comment

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

Bad indentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, this converts spaces to tabs.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, but it's still indented one tab too many, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call.

@timmywil timmywil force-pushed the abort-2079 branch 2 times, most recently from b7ab67d to 892b57b Compare November 2, 2015 17:31
@timmywil
Copy link
Member Author

timmywil commented Nov 2, 2015

Bah, IE9 doesn't have onabort. The only other solution I see is to use .onreadystatechange, but that would change other stuff as well.

@dmethvin
Copy link
Member

dmethvin commented Nov 2, 2015

Could we just document that IE9 doesn't support this? It would be a shame to let IE9 prevent us from doing the right thing for everyone else.

@timmywil
Copy link
Member Author

timmywil commented Nov 2, 2015

That's probably better than switching everything back to onreadystatechange. Anyone against this?

@gibson042
Copy link
Member

Looking at the compat code, I don't think use of onreadystatechange would be that cumbersome.

@timmywil
Copy link
Member Author

timmywil commented Nov 2, 2015

Afaict, it does require determining success/error manually again, which has historically been cumbersome.

@timmywil
Copy link
Member Author

timmywil commented Nov 2, 2015

Yea, we end up losing a lot of advantages.

@gibson042
Copy link
Member

No, we can keep the current structure and only use onreadystatechange for the abort case.

@timmywil
Copy link
Member Author

timmywil commented Nov 2, 2015

Ah, well I tried that first, but onreadystatechange is called before other handlers, and brings up some IE9 quirks, such as not being to access any properties on a manual, native abort – which makes it difficult to test – and not having a solid way to tell the difference between abort and other errors.

@gibson042
Copy link
Member

For example, verify readyState === 4 and then delay its body (inside of which is an if ( callback ) guard) by a tick so that it only executes if none of onload/onerror/onabort have.

@timmywil
Copy link
Member Author

timmywil commented Nov 2, 2015

Ah, a setTimeout could do the trick to allow other callbacks to run first.

@timmywil
Copy link
Member Author

timmywil commented Nov 2, 2015

@dmethvin @gibson042 This works. Let me know if you have ideas to get it smaller.

- IE9 does not have onabort. Use onreadystatechange instead.

Fixes jquerygh-2079
@jaubourg
Copy link
Member

jaubourg commented Nov 2, 2015

Ah, a setTimeout could do the trick to allow other callbacks to run first.

Except for synchronous requests, right?

@timmywil
Copy link
Member Author

timmywil commented Nov 2, 2015

Except for synchronous requests, right?

Right. That's quite an edge case, tho. IE9, native abort, sync only.

@timmywil timmywil closed this in 76e9a95 Nov 3, 2015
@timmywil timmywil deleted the abort-2079 branch November 3, 2015 17:34
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 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.

7 participants