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

Update httpBackend, check window.XMLHttpRequest #5679

Closed
wants to merge 2 commits into from
Closed

Update httpBackend, check window.XMLHttpRequest #5679

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac4e9a7
Update httpBackend, check window.XMLHttpRequest
jorgt Jan 8, 2014
5638bf5
Update httpBackend, check window.XMLHttpRequest
jorgt Jan 23, 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
2 changes: 1 addition & 1 deletion src/ng/httpBackend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
function createXhr(method) {
// IE8 doesn't support PATCH method, but the ActiveX object does
/* global ActiveXObject */
return (msie <= 8 && lowercase(method) === 'patch')
return ((msie <= 8 && lowercase(method) === 'patch') || isUndefined(window.XMLHttpRequest))
Copy link
Contributor

Choose a reason for hiding this comment

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

this change means that we should use activex even for non-ie browsers if window.xhr is not defined. that's not right

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest it's only IE that needs a backfill (see Browser Compatibility). My test case is admittedly limited. Besides, right now on browsers that do not define window.xhr, the entire $http module fails which is worse.

Copy link

Choose a reason for hiding this comment

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

Why not use a try...catch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Assuming you mean this method (simplified):

var xhr;
try {
  xhr = new ActiveXObject('Microsoft.XMLHTTP');
} catch(e) {
  xhr = new window.XMLHttpRequest();
}
return xhr;

This always uses ActiveX in IE instead of XMLHttpRequest which is not really in the spirit of the original code of using ActiveX only when necessary. This was the simplest change I could make without rewriting the method. There are try / catch blocks higher up in the tree and I'm not sure what removing an exception here will do.

Copy link

Choose a reason for hiding this comment

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

Not quite.

var xhr;

if( msie <= 8 && lowercase(method) === 'patch' )
{
   xhr = new ActiveXObject('Microsoft.XMLHTTP');
} else {
  try {
    xhr = new window.XMLHttpRequest();
  } catch( e ) {
    xhr = new ActiveXObject('Microsoft.XMLHTTP');
  }
}
return xhr;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will that not only throw an error when XMLHttpRequest is undefined, which is what was checked already?

I don't mind changing that though, if it's considered more sturdy.

? new ActiveXObject('Microsoft.XMLHTTP')
: new window.XMLHttpRequest();
}
Expand Down