Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

refactor($http) Use onload/onerror/onabort instead of onreadystatechange #9329

Closed
wants to merge 2 commits into from
Closed

refactor($http) Use onload/onerror/onabort instead of onreadystatechange #9329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fdd2659
refactor($http) Simplify code by removing workarounds for older versi…
realityking Sep 26, 2014
fa2634c
refactor($http) Use onload/onerror/onabort instead of onreadystatechange
realityking Sep 27, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions docs/content/error/$httpBackend/noxhr.ngdoc
View file
Open in desktop

This file was deleted.

92 changes: 31 additions & 61 deletions src/ng/httpBackend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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();
}

/**
Expand All @@ -37,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();

Expand All @@ -59,7 +46,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) {
Expand All @@ -68,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;

// normalize IE9 bug (http://bugs.jquery.com/ticket/1450)
var status = xhr.status === 1223 ? 204 : xhr.status;

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;
}
Expand Down Expand Up @@ -138,7 +120,6 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc


function timeoutRequest() {
status = ABORTED;
jsonpDone && jsonpDone();
xhr && xhr.abort();
}
Expand All @@ -148,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);
}
Expand Down
94 changes: 11 additions & 83 deletions test/ng/httpBackendSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

Expand All @@ -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();
});

Expand All @@ -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();
});
Expand Down Expand Up @@ -195,8 +158,7 @@ describe('$httpBackend', function() {
expect(xhr.abort).toHaveBeenCalledOnce();

xhr.status = 0;
xhr.readyState = 4;
xhr.onreadystatechange();
xhr.onabort();
expect(callback).toHaveBeenCalledOnce();
});

Expand All @@ -215,8 +177,7 @@ describe('$httpBackend', function() {
expect(xhr.abort).toHaveBeenCalledOnce();

xhr.status = 0;
xhr.readyState = 4;
xhr.onreadystatechange();
xhr.onabort();
expect(callback).toHaveBeenCalledOnce();
});

Expand All @@ -234,8 +195,7 @@ describe('$httpBackend', function() {
expect(xhr.abort).toHaveBeenCalledOnce();

xhr.status = 0;
xhr.readyState = 4;
xhr.onreadystatechange();
xhr.onabort();
expect(callback).toHaveBeenCalledOnce();
}));

Expand All @@ -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();
Expand All @@ -271,42 +230,14 @@ 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);
expect(xhr.abort).not.toHaveBeenCalled();
});


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);
Expand All @@ -326,8 +257,7 @@ describe('$httpBackend', function() {
});

xhrInstance.response = {some: 'object'};
xhrInstance.readyState = 4;
xhrInstance.onreadystatechange();
xhrInstance.onload();

expect(callback).toHaveBeenCalledOnce();
});
Expand All @@ -347,8 +277,7 @@ describe('$httpBackend', function() {
});

xhrInstance.responseText = responseText;
xhrInstance.readyState = 4;
xhrInstance.onreadystatechange();
xhrInstance.onload();

expect(callback).toHaveBeenCalledOnce();
});
Expand Down Expand Up @@ -436,8 +365,7 @@ describe('$httpBackend', function() {
xhr = MockXhr.$$lastInstance;
xhr.status = status;
xhr.responseText = content;
xhr.readyState = 4;
xhr.onreadystatechange();
xhr.onload();
}


Expand Down