-
Notifications
You must be signed in to change notification settings - Fork 27.5k
script.onreadystatechange Does not exist in IE11 #4922
Conversation
In order for Jsonp to work correctly in IE11 this is needed.
@@ -135,7 +135,7 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument, | |||
script.type = 'text/javascript'; | |||
script.src = url; | |||
|
|||
if (msie) { | |||
if (msie && script.onreadystatechange) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MSDN says IE11 should use "onload" for script tags. What about this?
if (msie) {
script[script.onreadystatechange ? 'onreadystatechange' : 'onload'] = function() {
if (/loaded|complete/.test(script.readyState)) doneWrapper();
};
}
Or is it okay to just ignore it for IE11?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO that's exactly one of those reasons for which browser sniffing is a bad idea. It's unfortunate if code can break just because a new version of a particular browser is released and the sniffing now takes incorrect assumptions.
If this code used feature detection, it would just work in IE11 without having to introduce any changes post-factum.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess prior versions of MSIE didn't fire the load event for script tags which aren't added to the DOM or something? Not really sure, MSDN doesn't really offer much useful information about past versions, and I haven't found any issues about it in previous versions from google --- so I'm curious why we're actually using onreadystatechange
at all there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@caitp It's needed for IE8 and older, the load
event is not fired there on script elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, jQuery 1.x just sets both: https://github.com/jquery/jquery/blob/1.x-master/src/ajax/script.js#L57-77
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why a similar approach couldn't be adopted here until XP disappears from the face of the earth
According to this page: http://pieisgood.org/test/script-link-events/ |
Also, the jQuery code sets the handlers to null once they have been called (an IE memory leak bug, it says). This seems like a sensible thing to do. |
IE8, IE9 and IE10 can use `script.onreadystate` so up till now we have been using this if the sniffer says we are on IE. But IE11 now does not support `script.onreadystate` and only supports the more standard `script.onload` and `script.onerror`. IE9 and IE10 do support `script.onload` and `script.onerror`. So now we only test whether we are on IE8 or earlier before using `script.onreadystate`. See http://pieisgood.org/test/script-link-events/ jQuery just uses all these handlers at once and hopes for the best, but since IE9 and IE10 support both sets of handlers, this could cause the handlers to be run more than once. jQuery also notes that there is a potential memory leak in IE unless we remove the handlers from the script object once they are run. So we are doing this too, now. Closes angular#4523 Closes angular#4527 Closes angular#4922
…SONP IE8, IE9 and IE10 can use `script.onreadystate` so up till now we have been using this if the sniffer says we are on IE. But IE11 now does not support `script.onreadystate` and only supports the more standard `script.onload` and `script.onerror`. IE9 and IE10 do support `script.onload` and `script.onerror`. So now we only test whether we are on IE8 or earlier before using `script.onreadystate`. See http://pieisgood.org/test/script-link-events/ jQuery just uses all these handlers at once and hopes for the best, but since IE9 and IE10 support both sets of handlers, this could cause the handlers to be run more than once. jQuery also notes that there is a potential memory leak in IE unless we remove the handlers from the script object once they are run. So we are doing this too, now. Closes angular#4523 Closes angular#4527 Closes angular#4922
…SONP IE8, IE9 and IE10 can use `script.onreadystate` so up till now we have been using this if the sniffer says we are on IE. But IE11 now does not support `script.onreadystate` and only supports the more standard `script.onload` and `script.onerror`. IE9 and IE10 do support `script.onload` and `script.onerror`. So now we only test whether we are on IE8 or earlier before using `script.onreadystate`. See http://pieisgood.org/test/script-link-events/ jQuery just uses all these handlers at once and hopes for the best, but since IE9 and IE10 support both sets of handlers, this could cause the handlers to be run more than once. jQuery also notes that there is a potential memory leak in IE unless we remove the handlers from the script object once they are run. So we are doing this too, now. Closes angular#4523 Closes angular#4527 Closes angular#4922
In order for Jsonp to work correctly in IE11 this is needed.