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

fix($httpBackend): don't error when JSONP callback called with no parameter #6735

Closed
wants to merge 1 commit into from
Closed

fix($httpBackend): don't error when JSONP callback called with no parameter #6735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
59 changes: 31 additions & 28 deletions src/ng/httpBackend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,13 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
var callbackId = '_' + (callbacks.counter++).toString(36);
callbacks[callbackId] = function(data) {
callbacks[callbackId].data = data;
callbacks[callbackId].called = true;
};

var jsonpDone = jsonpReq(url.replace('JSON_CALLBACK', 'angular.callbacks.' + callbackId),
function() {
if (callbacks[callbackId].data) {
completeRequest(callback, 200, callbacks[callbackId].data);
} else {
completeRequest(callback, status || -2);
}
callbacks[callbackId] = angular.noop;
callbackId, function(status, text) {
completeRequest(callback, status, callbacks[callbackId].data, "", text);
callbacks[callbackId] = noop;
});
} else {

Expand Down Expand Up @@ -158,33 +155,39 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
}
};

function jsonpReq(url, done) {
function jsonpReq(url, callbackId, done) {
// we can't use jQuery/jqLite here because jQuery does crazy shit with script elements, e.g.:
// - fetches local scripts via XHR and evals them
// - adds and immediately removes script elements from the document
var script = rawDocument.createElement('script'),
doneWrapper = function() {
script.onreadystatechange = script.onload = script.onerror = null;
rawDocument.body.removeChild(script);
if (done) done();
};

script.type = 'text/javascript';
var script = rawDocument.createElement('script'), callback = null;
script.type = "text/javascript";
script.src = url;

if (msie && msie <= 8) {
script.onreadystatechange = function() {
if (/loaded|complete/.test(script.readyState)) {
doneWrapper();
script.async = true;

callback = function(event) {
removeEventListenerFn(script, "load", callback);
removeEventListenerFn(script, "error", callback);
rawDocument.body.removeChild(script);
script = null;
var status = -1;
var text = "unknown";

if (event) {
if (event.type === "load" && !callbacks[callbackId].called) {
event = { type: "error" };
}
};
} else {
script.onload = script.onerror = function() {
doneWrapper();
};
}
text = event.type;
status = event.type === "error" ? 404 : 200;
}

if (done) {
done(status, text);
}
};

addEventListenerFn(script, "load", callback);
addEventListenerFn(script, "error", callback);
rawDocument.body.appendChild(script);
return doneWrapper;
return callback;
}
}
71 changes: 4 additions & 67 deletions test/ng/httpBackendSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ describe('$httpBackend', function() {
fakeDocument = {
$$scripts: [],
createElement: jasmine.createSpy('createElement').andCallFake(function() {
return {};
// Return a proper script element...
return document.createElement(arguments[0]);
}),
body: {
appendChild: jasmine.createSpy('body.appendChild').andCallFake(function(script) {
Expand Down Expand Up @@ -325,13 +326,7 @@ describe('$httpBackend', function() {

expect(url[1]).toBe('http://example.org/path');
callbacks[url[2]]('some-data');

if (script.onreadystatechange) {
script.readyState = 'complete';
script.onreadystatechange();
} else {
script.onload();
}
browserTrigger(script, "load");

expect(callback).toHaveBeenCalledOnce();
});
Expand All @@ -346,71 +341,13 @@ describe('$httpBackend', function() {
callbackId = script.src.match(SCRIPT_URL)[2];

callbacks[callbackId]('some-data');

if (script.onreadystatechange) {
script.readyState = 'complete';
script.onreadystatechange();
} else {
script.onload();
}
browserTrigger(script, "load");

expect(callbacks[callbackId]).toBe(angular.noop);
expect(fakeDocument.body.removeChild).toHaveBeenCalledOnceWith(script);
});


Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IE8 is no longer supported, onreadystate is not being used, this is no longer useful

if(msie<=8) {

it('should attach onreadystatechange handler to the script object', function() {
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, noop);

expect(fakeDocument.$$scripts[0].onreadystatechange).toEqual(jasmine.any(Function));

var script = fakeDocument.$$scripts[0];

script.readyState = 'complete';
script.onreadystatechange();

expect(script.onreadystatechange).toBe(null);
});

} else {

it('should attach onload and onerror handlers to the script object', function() {
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, noop);

expect(fakeDocument.$$scripts[0].onload).toEqual(jasmine.any(Function));
expect(fakeDocument.$$scripts[0].onerror).toEqual(jasmine.any(Function));

var script = fakeDocument.$$scripts[0];
script.onload();

expect(script.onload).toBe(null);
expect(script.onerror).toBe(null);
});

}

it('should call callback with status -2 when script fails to load', function() {
callback.andCallFake(function(status, response) {
expect(status).toBe(-2);
expect(response).toBeUndefined();
});

$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
expect(fakeDocument.$$scripts.length).toBe(1);

var script = fakeDocument.$$scripts.shift();
if (script.onreadystatechange) {
script.readyState = 'complete';
script.onreadystatechange();
} else {
script.onload();
}
expect(callback).toHaveBeenCalledOnce();
});


it('should set url to current location if not specified or empty string', function() {
$backend('JSONP', undefined, null, callback);
expect(fakeDocument.$$scripts[0].src).toBe($browser.url());
Expand Down