sstrigler / JSJaC

JavaScript Jabber Client Library
Other
295 stars 86 forks source link

Explicitly assign window reference to XMLHttpRequest. #71

Closed jon-armstrong-zz closed 8 years ago

jon-armstrong-zz commented 10 years ago

In some installations of IE8 on Windows XP, XMLHttpRequest and window.XMLHttpRequest do not reference the same object after window.XMLHttpRequest is reassigned.

rraptorr commented 10 years ago

Can you explain on what installations? How to test and verify this?

rraptorr commented 10 years ago

Also, you have not used 'var' which means that you will pollute whatever 'this' refers to. Please either prefix all of the uses with 'window' or use 'var'.

jon-armstrong-zz commented 10 years ago

Sorry, I should have been more specific. I am running XP Pro 2002 SP3 with IE: 8.0.6001.18702. This is one of the VMs downloaded from modern.ie, so no exotic setup beyond that. However, coworkers with the same setup are not able to reproduce.

Basically, under "normal" conditions (verified with debugger):

window.XMLHttpRequest == XMLHttpRequest // true
window.XMLHttpRequest = function() {
...
}
window.XMLHttpRequest == XMLHttpRequest // true

But with this IE VM, the latter assertion evaluates to false. Global pollution without var should not normally be an issue because these should both be references to the same global object. However, for consistency what you suggest is a good idea, so I will change the other references to window.XMLHttpRequest and see if that still resolves the issue. I will also see if I can come up with a more isolated test case. For me at least, anything that references XMLHttpRequest after JSJaC is loaded in this browser fails.

jon-armstrong-zz commented 10 years ago

Try as I might, I was unable to figure out a reduced test case for this issue. It somehow involves something specific to my application, although nothing else alters the XMLHttpRequest object that I can find.

However, using window.XMLHttpRequest exclusively does resolve that issue for me, so I've pushed that fix and removed my previous commit.