From 40d8da80ce773dd82166428f481e33df51883340 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] refactor($http) Use onload/onerror/onabort instead of onreadystatechange Closes #9329 --- src/ng/httpBackend.js | 74 +++++++++++------------------- test/ng/httpBackendSpec.js | 94 +++++--------------------------------- 2 files changed, 38 insertions(+), 130 deletions(-) diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js index c29e5bfc..3026b1e8 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 = ''; + xhr.onload = function requestLoaded() { + var statusText = xhr.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) + var response = ('response' in xhr) ? xhr.response : xhr.responseText; - // 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; - } + // normalize IE9 bug (http://bugs.jquery.com/ticket/1450) + var status = xhr.status === 1223 ? 204 : xhr.status; - // 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; - } - - completeRequest(callback, - status || xhr.status, - response, - responseHeaders, - statusText); + // 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 c1c2ec4f..f2c9e513 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(); }