fix($httpbackend): ie11 api changes for jsonp #4527
Conversation
|
Thanks for the PR!
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
|
Before we accept this, we need to get IE11 up and running on our CI server. IE11 has graduated from "preview" to actually being released, so we really should add it. |
| @@ -132,7 +132,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.
is the msie part actually still required?
|
@eddiemonge @vra @btford |
|
@allaud You are right. I incorrectly assumed the existence of the property would allow the if statement. However, I forgot to check what the property was actually set to. If the property is set, its initial value is null. Checking for the typeof was better then checking the value. IE11 doesn't have the property at all so it is undefined where other IE's have it as null, which is an object, not undefined. After checking IE 7-11 (7 didnt like my angular test case at all) I have verified the updated code now runs correctly. Non-IE browsers still won't see it because of the MSIE part. |
|
Now with a live demo: http://jsbin.com/EYuCUxi/1 |
| @@ -132,7 +132,7 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument, | |||
| script.type = 'text/javascript'; | |||
| script.src = url; | |||
|
|
|||
| if (msie) { | |||
| if (msie && typeof script.onreadystatechange !== 'undefined') { | |||
There was a problem hiding this comment.
How about:
if ( msie && isDefined(script.onreadystatechange) ) {
|
Apart from using |
|
I can confirm that the problem is apparent for me on IE11 @ Angular 1.2-rc.3 and is fixed by this patch. Moreover @btford - IE11 causes numerous other errors on |
IE11 changed their script api and no longer uses onreadystatechange. It now follows other browsers with onload and onerror Fixes #4523
|
#4922 adopts a similar approach --- In the commit discussions, @mzgol references jQuery's approach' to the same problem, which circumvents the issue in a more robust manner without relying on browser sniffing. As I mention there, I don't see a reason a similar approach couldn't be leveraged here |
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
IE11 changed their script api and no longer uses onreadystatechange. It
now follows other browsers with onload and onerror
Fixes #4523