Opened 12 years ago

Closed 12 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 Adam Peller)

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)

dojo._base.xhr_20071206.patch (732 bytes) - added by Jared Jurkiewicz 12 years ago.
Patch to xhr in base to fix IE bug.
dojo._base.xhr_20071206.2.patch (851 bytes) - added by Jared Jurkiewicz 12 years ago.
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

Download all attachments as: .zip

Change History (10)

Changed 12 years ago by Jared Jurkiewicz

Patch to xhr in base to fix IE bug.

comment:1 Changed 12 years ago by Jared Jurkiewicz

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 12 years ago by Jared Jurkiewicz

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 12 years ago by Jared Jurkiewicz

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 12 years ago by Adam Peller

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 12 years ago by Adam Peller

Description: modified (diff)

comment:5 Changed 12 years ago by Adam Peller

Resolution: fixed
Status: newclosed

(In [11732]) xml response handler - can't return out of a dojo.forEach, and prefixes was undefined. Fixes #5320 Also, make objectToQuery shorter and faster.

comment:6 Changed 12 years ago by Adam Peller

Resolution: fixed
Status: closedreopened

need to apply to 1.0 branch

comment:7 Changed 12 years ago by Adam Peller

Cc: alex added
Owner: changed from James Burke to Adam Peller
Status: reopenednew

comment:8 Changed 12 years ago by Adam Peller

Milestone: 1.0.21.1
Resolution: fixed
Status: newclosed

upon further consideration, this isn't all that critical since it only seems to impact the file: case. Leaving as trunk-only, for now.

Note: See TracTickets for help on using tickets.