Opened 13 years ago
Closed 13 years ago
#5320 closed defect (fixed)
[patch][ccla]xhrGet fails on IE 6/7 in some cases (most notable in local file load)
Reported by: | Jared Jurkiewicz | Owned by: | Adam Peller |
---|---|---|---|
Priority: | high | Milestone: | 1.1 |
Component: | IO | Version: | 1.0 |
Keywords: | Cc: | Jared Jurkiewicz, alex | |
Blocked By: | Blocking: |
Description (last modified by )
xhrGet fails on IE 6/7 in some cases (most notable in local file load).
This issue was reported internally at my workplace. They were having problems getting xhrGet to work correctly on IE when loading files locally. The problem is actually in the xml handler. This is what they determined from investigation:
This one is related to #3294 and #3294. Although these two bugs marked fix, the code in 0.9 seems to be broken. The following is from xhr.js (or dojo.js.uncompressed.js), which parses XML from responseXML due to IE bug, if needed:
dojo._contentHandlers = { ...some code... "xml": function(xhr){ if(dojo.isIE && !xhr.responseXML){ dojo.forEach(["MSXML2", "Microsoft", "MSXML", "MSXML3"], function(i){ try{ var doc = new ActiveXObject(prefixes[i]+".XMLDOM"); doc.async = false; doc.loadXML(xhr.responseText); return doc; // DOMDocument }catch(e){ /* squelch */ }; }); }else{ return xhr.responseXML; } }
There is not a prefixes[] array defined, so you get an error at prefixes[i]+".XMLDOM". When changed to i+".XMLDOM", it fails again. The return call after loading of doc returns the anonymous function in dojo.forEach, not the function dojo._contentHandlers.xml and the loop continues and returns the wrong value.
This can be readily seen to fail just bu loading some dojox testcases locally. Specifically:
dojox/data/tests/runTests.html. The OpmlStore? and XmlStore? tests fail because they are XML based stores (and as such use handleAs: xml).
A patch is coming shortly for this issue. I just wanted to go ahead and get it opened now.
Attachments (2)
Change History (10)
Changed 13 years ago by
Attachment: | dojo._base.xhr_20071206.patch added |
---|
comment:1 Changed 13 years ago by
Double-checking the patch now to be sure it fixes it. It seemed to after one run I did, but I want to double-check...
Changed 13 years ago by
Attachment: | dojo._base.xhr_20071206.2.patch added |
---|
Updated patch for this. Have to check both if it's IE ... and if it's file, force the load from responseText to get it to work properly on IE and local file loads
comment:2 Changed 13 years ago by
I tested this on: IE 6 IE 7
Local file loads as well as browser loads.
I don't understand why IE behaves this way ... but response.responseXML is unusable if it was loaded from file: protocol. That's why it has to check for either response.XML being null/undefined OR if it's a local file load. Otherwise, it just doesn't work. Gotta love the old active X XHR object... Very strange. In any event, this patch does seem to resolve the issue.
comment:3 Changed 13 years ago by
Milestone: | → 1.0.2 |
---|---|
Summary: | xhrGet fails on IE 6/7 in some cases (most notable in local file load) → [patch][ccla]xhrGet fails on IE 6/7 in some cases (most notable in local file load) |
looks like a regression caused by [9899] perhaps we should fix in the 1.0 branch?
Rather than replace the forEach with a traditional for loop, why can't we just reference 'i'?
comment:4 Changed 13 years ago by
Description: | modified (diff) |
---|
comment:5 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:6 Changed 13 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
need to apply to 1.0 branch
comment:7 Changed 13 years ago by
Cc: | alex added |
---|---|
Owner: | changed from James Burke to Adam Peller |
Status: | reopened → new |
comment:8 Changed 13 years ago by
Milestone: | 1.0.2 → 1.1 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
upon further consideration, this isn't all that critical since it only seems to impact the file: case. Leaving as trunk-only, for now.
Patch to xhr in base to fix IE bug.