From fdd26598c1a09e9aeb8423e9a6ce3ecb0afe914f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rouven=20We=C3=9Fling?= Date: Fri, 26 Sep 2014 16:01:54 +0200 Subject: [PATCH 1/2] refactor($http) Simplify code by removing workarounds for older versions of Internet Explorer This removes a workaround for IE 8 and and error handling for IE6. --- docs/content/error/$httpBackend/noxhr.ngdoc | 10 ---------- src/ng/httpBackend.js | 16 +++------------- 2 files changed, 3 insertions(+), 23 deletions(-) delete mode 100644 docs/content/error/$httpBackend/noxhr.ngdoc diff --git a/docs/content/error/$httpBackend/noxhr.ngdoc b/docs/content/error/$httpBackend/noxhr.ngdoc deleted file mode 100644 index e2d5901d72ea..000000000000 --- a/docs/content/error/$httpBackend/noxhr.ngdoc +++ /dev/null @@ -1,10 +0,0 @@ -@ngdoc error -@name $httpBackend:noxhr -@fullName Unsupported XHR -@description - -This error occurs in browsers that do not support XmlHttpRequest. AngularJS -supports Safari, Chrome, Firefox, Opera, IE8 and higher, and mobile browsers -(Android, Chrome Mobile, iOS Safari). To avoid this error, use an officially -supported browser. - diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js index 78d04cfa24b9..c29e5bfc23e8 100644 --- a/src/ng/httpBackend.js +++ b/src/ng/httpBackend.js @@ -1,17 +1,7 @@ 'use strict'; -function createXhr(method) { - //if IE and the method is not RFC2616 compliant, or if XMLHttpRequest - //is not available, try getting an ActiveXObject. Otherwise, use XMLHttpRequest - //if it is available - if (msie <= 8 && (!method.match(/^(get|post|head|put|delete|options)$/i) || - !window.XMLHttpRequest)) { - return new window.ActiveXObject("Microsoft.XMLHTTP"); - } else if (window.XMLHttpRequest) { - return new window.XMLHttpRequest(); - } - - throw minErr('$httpBackend')('noxhr', "This browser does not support XMLHttpRequest."); +function createXhr() { + return new window.XMLHttpRequest(); } /** @@ -59,7 +49,7 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc }); } else { - var xhr = createXhr(method); + var xhr = createXhr(); xhr.open(method, url, true); forEach(headers, function(value, key) { From fa2634c83a8dad31516109e4ea98f1506701068c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rouven=20We=C3=9Fling?= Date: Sat, 27 Sep 2014 19:42:48 +0200 Subject: [PATCH 2/2] refactor($http) Use onload/onerror/onabort instead of onreadystatechange --- src/ng/httpBackend.js | 76 ++++++++++++------------------ test/ng/httpBackendSpec.js | 94 +++++--------------------------------- 2 files changed, 39 insertions(+), 131 deletions(-) diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js index c29e5bfc23e8..d6db4daed4de 100644 --- a/src/ng/httpBackend.js +++ b/src/ng/httpBackend.js @@ -27,11 +27,8 @@ function $HttpBackendProvider() { } function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDocument) { - var ABORTED = -1; - // TODO(vojta): fix the signature return function(method, url, post, callback, headers, timeout, withCredentials, responseType) { - var status; $browser.$$incOutstandingRequestCount(); url = url || $browser.url(); @@ -58,44 +55,39 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc } }); - // In IE6 and 7, this might be called synchronously when xhr.send below is called and the - // response is in the cache. the promise api will ensure that to the app code the api is - // always async - xhr.onreadystatechange = function() { - // onreadystatechange might get called multiple times with readyState === 4 on mobile webkit caused by - // xhrs that are resolved while the app is in the background (see #5426). - // since calling completeRequest sets the `xhr` variable to null, we just check if it's not null before - // continuing - // - // we can't set xhr.onreadystatechange to undefined or delete it because that breaks IE8 (method=PATCH) and - // Safari respectively. - if (xhr && xhr.readyState == 4) { - var responseHeaders = null, - response = null, - statusText = ''; - - if(status !== ABORTED) { - responseHeaders = xhr.getAllResponseHeaders(); - - // responseText is the old-school way of retrieving response (supported by IE8 & 9) - // response/responseType properties were introduced in XHR Level2 spec (supported by IE10) - response = ('response' in xhr) ? xhr.response : xhr.responseText; - } + xhr.onload = function requestComplete() { + var statusText = xhr.statusText || ''; - // Accessing statusText on an aborted xhr object will - // throw an 'c00c023f error' in IE9 and lower, don't touch it. - if (!(status === ABORTED && msie < 10)) { - statusText = xhr.statusText; - } + // responseText is the old-school way of retrieving response (supported by IE8 & 9) + // response/responseType properties were introduced in XHR Level2 spec (supported by IE10) + var response = ('response' in xhr) ? xhr.response : xhr.responseText; - completeRequest(callback, - status || xhr.status, - response, - responseHeaders, - statusText); + // normalize IE9 bug (http://bugs.jquery.com/ticket/1450) + var status = xhr.status === 1223 ? 204 : xhr.status; + + // fix status code when it is 0 (0 status is undocumented). + // Occurs when accessing file resources or on Android 4.1 stock browser + // while retrieving files from application cache. + if (status === 0) { + status = response ? 200 : urlResolve(url).protocol == 'file' ? 404 : 0; } + + completeRequest(callback, + status, + response, + xhr.getAllResponseHeaders(), + statusText); }; + var requestError = function () { + // The response is always empty + // See https://xhr.spec.whatwg.org/#request-error-steps and https://fetch.spec.whatwg.org/#concept-network-error + completeRequest(callback, -1, null, null, ''); + }; + + xhr.onerror = requestError; + xhr.onabort = requestError; + if (withCredentials) { xhr.withCredentials = true; } @@ -128,7 +120,6 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc function timeoutRequest() { - status = ABORTED; jsonpDone && jsonpDone(); xhr && xhr.abort(); } @@ -138,17 +129,6 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc timeoutId && $browserDefer.cancel(timeoutId); jsonpDone = xhr = null; - // fix status code when it is 0 (0 status is undocumented). - // Occurs when accessing file resources or on Android 4.1 stock browser - // while retrieving files from application cache. - if (status === 0) { - status = response ? 200 : urlResolve(url).protocol == 'file' ? 404 : 0; - } - - // normalize IE bug (http://bugs.jquery.com/ticket/1450) - status = status === 1223 ? 204 : status; - statusText = statusText || ''; - callback(status, response, headersString, statusText); $browser.$$completeOutstandingRequest(noop); } diff --git a/test/ng/httpBackendSpec.js b/test/ng/httpBackendSpec.js index c1c2ec4f0320..f2c9e5138944 100644 --- a/test/ng/httpBackendSpec.js +++ b/test/ng/httpBackendSpec.js @@ -87,27 +87,7 @@ describe('$httpBackend', function() { $backend('GET', '/some-url', null, callback); xhr = MockXhr.$$lastInstance; xhr.statusText = 'OK'; - xhr.readyState = 4; - xhr.onreadystatechange(); - expect(callback).toHaveBeenCalledOnce(); - }); - - it('should not touch xhr.statusText when request is aborted on IE9 or lower', function() { - callback.andCallFake(function(status, response, headers, statusText) { - expect(statusText).toBe((!msie || msie >= 10) ? 'OK' : ''); - }); - - $backend('GET', '/url', null, callback, {}, 2000); - xhr = MockXhr.$$lastInstance; - spyOn(xhr, 'abort'); - - fakeTimeout.flush(); - expect(xhr.abort).toHaveBeenCalledOnce(); - - xhr.status = 0; - xhr.readyState = 4; - xhr.statusText = 'OK'; - xhr.onreadystatechange(); + xhr.onload(); expect(callback).toHaveBeenCalledOnce(); }); @@ -118,8 +98,7 @@ describe('$httpBackend', function() { $backend('GET', '/some-url', null, callback); xhr = MockXhr.$$lastInstance; - xhr.readyState = 4; - xhr.onreadystatechange(); + xhr.onload(); expect(callback).toHaveBeenCalledOnce(); }); @@ -133,23 +112,7 @@ describe('$httpBackend', function() { xhr = MockXhr.$$lastInstance; xhr.status = 1223; - xhr.readyState = 4; - xhr.onreadystatechange(); - - expect(callback).toHaveBeenCalledOnce(); - }); - - // onreadystatechange might by called multiple times - // with readyState === 4 on mobile webkit caused by - // xhrs that are resolved while the app is in the background (see #5426). - it('should not process onreadystatechange callback with readyState == 4 more than once', function() { - $backend('GET', 'URL', null, callback); - xhr = MockXhr.$$lastInstance; - - xhr.status = 200; - xhr.readyState = 4; - xhr.onreadystatechange(); - xhr.onreadystatechange(); + xhr.onload(); expect(callback).toHaveBeenCalledOnce(); }); @@ -195,8 +158,7 @@ describe('$httpBackend', function() { expect(xhr.abort).toHaveBeenCalledOnce(); xhr.status = 0; - xhr.readyState = 4; - xhr.onreadystatechange(); + xhr.onabort(); expect(callback).toHaveBeenCalledOnce(); }); @@ -215,8 +177,7 @@ describe('$httpBackend', function() { expect(xhr.abort).toHaveBeenCalledOnce(); xhr.status = 0; - xhr.readyState = 4; - xhr.onreadystatechange(); + xhr.onabort(); expect(callback).toHaveBeenCalledOnce(); }); @@ -234,8 +195,7 @@ describe('$httpBackend', function() { expect(xhr.abort).toHaveBeenCalledOnce(); xhr.status = 0; - xhr.readyState = 4; - xhr.onreadystatechange(); + xhr.onabort(); expect(callback).toHaveBeenCalledOnce(); })); @@ -250,8 +210,7 @@ describe('$httpBackend', function() { spyOn(xhr, 'abort'); xhr.status = 200; - xhr.readyState = 4; - xhr.onreadystatechange(); + xhr.onload(); expect(callback).toHaveBeenCalledOnce(); $timeout.flush(); @@ -271,8 +230,7 @@ describe('$httpBackend', function() { expect(fakeTimeout.delays[0]).toBe(2000); xhr.status = 200; - xhr.readyState = 4; - xhr.onreadystatechange(); + xhr.onload(); expect(callback).toHaveBeenCalledOnce(); expect(fakeTimeout.delays.length).toBe(0); @@ -280,33 +238,6 @@ describe('$httpBackend', function() { }); - it('should register onreadystatechange callback before sending', function() { - // send() in IE6, IE7 is sync when serving from cache - function SyncXhr() { - xhr = this; - this.open = this.setRequestHeader = noop; - - this.send = function() { - this.status = 200; - this.responseText = 'response'; - this.readyState = 4; - this.onreadystatechange(); - }; - - this.getAllResponseHeaders = valueFn(''); - } - - callback.andCallFake(function(status, response) { - expect(status).toBe(200); - expect(response).toBe('response'); - }); - - $backend = createHttpBackend($browser, function() { return new SyncXhr(); }); - $backend('GET', '/url', null, callback); - expect(callback).toHaveBeenCalledOnce(); - }); - - it('should set withCredentials', function() { $backend('GET', '/some.url', null, callback, {}, null, true); expect(MockXhr.$$lastInstance.withCredentials).toBe(true); @@ -326,8 +257,7 @@ describe('$httpBackend', function() { }); xhrInstance.response = {some: 'object'}; - xhrInstance.readyState = 4; - xhrInstance.onreadystatechange(); + xhrInstance.onload(); expect(callback).toHaveBeenCalledOnce(); }); @@ -347,8 +277,7 @@ describe('$httpBackend', function() { }); xhrInstance.responseText = responseText; - xhrInstance.readyState = 4; - xhrInstance.onreadystatechange(); + xhrInstance.onload(); expect(callback).toHaveBeenCalledOnce(); }); @@ -436,8 +365,7 @@ describe('$httpBackend', function() { xhr = MockXhr.$$lastInstance; xhr.status = status; xhr.responseText = content; - xhr.readyState = 4; - xhr.onreadystatechange(); + xhr.onload(); }