[patch][cla] IE9 xhr.responseXML is insufficient for lite selector engine (the default for IE9 standards mode)

[Brandon Payton]

This work is covered under my personal CLA.

In IE9 standards mode, the default selector engine is 'lite'. This works correctly the page DOM and with the result of DOMParser.parseFromString() but not with the DOM implementation provided by XHR.responseXML.

XHR.responseXML does not provide a querySelectorAll implementation as the page's DOM and the result of DOMParser.parseFromString() do. Any query requiring use of querySelectorAll causes an exception in the lite engine because querySelectorAll is undefined.

I can think of a few options to address this within Dojo:

1. Always use acme query engine for IE9. This would accomodate the variation in DOM implementation, but we lose all access to a native querySelectorAll implementation.
2. Modify the lite engine to fall back to acme when querySelectorAll isn't available. This seems safest, but it adds an unnecessary dependency for those not using the selector engine w/ XHR XML.
3. Modify dojo/_base/xhr to manually create XML DOM from xhr.responseText when configured to handleAs 'xml'. This appears to give us a consistent DOM implementation, access to a native querySelectorAll implementation, and access to other properties available in the core IE9 DOM implementation (e.g., element.textContent).

Any other possibilities?

After some thought, I decided to create a patch for option 3, targeting IE9 specifically. In the event of an error, the xml response is null, but the patch does nothing to verify an XML compatible MIME type before trying to parse xhr.responseText as XML. I don't know whether or not that is important, but I omitted it in favor of a simpler patch.

I have also attached an example.html page and an example.xml file that together demonstrate querySelectorAll availability in IE9 standards mode.

NOTE: Just before submitting this bug, I noticed that dojo/_base/xhr is deprecated in favor of dojo/request. I'm not sure how this applies to that. At the very least, this is a conversation starter.

[Brandon Payton]

example.html dependency

[Brandon Payton]

ATTENTION: My patch as-is likely introduces a nasty bug. I was eager to make a first contribution and forgot to test in older versions of IE. The patch assumes that the assignment (result = xhr.responseXML) never yields undefined. Since IE6 doesn't support the responseXML property, this is a poor assumption.

I'm up for creating a new patch, but since there are multiple ways to address this issue, I'll wait for feedback.

[bill]

Does the same issue occur with dojo/request?

I'm not sure if this should be classified as a dojo/query bug, or an XHR bug, or just a wontfix:

• It doesn't seem like we should require (aka bloat) dojo/_base/xhr or even dojo/request's result to have a querySelectorAll() method.
• It initially seems like the feature testing in the lite engine, or the dojo/query module itself, should handle this, but that would require branching not just on the browser capabilities but on the document being queried. Not sure if that's practical or not.

[Brandon Payton]

Yes, the same issue occurs with dojo/request. I've attached an updated example (dojo-request-example.html).

It does seem like the fix should be in the query-related modules, but I didn't see a practical solution there. I'm interested to see what Kris thinks.

[Brandon Payton]

Coincidentally, IEBlog featured a post about this issue 11 days after this bug was posted: ​http://blogs.msdn.com/b/ie/archive/2012/07/19/xmlhttprequest-responsexml-in-ie10-release-preview.aspx

The responseXML in IE9 is an MSXML document. To get a native XML document in IE, you have to use DOMParser.

In IE9, dojo/query selects an engine based on the IE9 native DOM, but when handleAs=="xml", dojo/_base/xhr and dojo/request return MSXML documents that fail to deliver the native selector implementations expected by dojo/query.

[dylan]

I think we should consider this fix for 1.9... Bryan and Kris, any thoughts on this?

Brandon, can you test your patch against 1.9 and update accordingly?

[Brandon Payton]

a patch to manually parse xhr.responseText as XML in IE9 standards mode

[Brandon Payton]

HTML example demonstrating the same issue with dojo/request/xhr

[Brandon Payton]

HTML example demonstrating the issue with dojo/_base/xhr

[Brandon Payton]

I've fixed and updated the dojo/_base/xhr patch against trunk because it doesn't look like there is a 1.9 branch yet. If there is, let me know, and I will update accordingly.

Two comments about this fix:

1. It seems like there should be has() test for DOMParser. Where should that go?
2. Ideally, this fix would examine xhr.responseXML.querySelectorAll to observe its absence, but if the DOM behind xhr.responseXML is generated on demand, that would cause this fix to generate two DOM documents for the same XML instead of just one.

Doing the same fix for dojo/request/xhr appears to be more difficult due to the separation of response handlers from the request providers, so I'd like to hear from Bryan and Kris before pursuing that.

[Brandon Payton]

Adds a new test case for dojo/_base/xhr

[bill]

Bumping this ticket since we are past the deadline for the 1.9RC. The fix can be put into 1.9.1 too, if desired.

[Bryan Forbes]

This should really be fixed since this is something that MS has talked about and shown a solution to. I'll have this fixed by tomorrow.

[bill]

Let's put it in 1.9.1, since we already cut the RC for 1.9.0. Unless we want to cut another 1.9.0 RC for this ticket (and/or for other reasons).

[Colin Snover]

No, let’s put it in 1.9.

[Brandon Payton]

This is an embarrassing bug involving a huge loss of dojo/query functionality in a commonly used browser. Why not put this in 1.9 since we're so close to a fix?

[cjolif]

Let me raise a (rhetorical) question about the urgency (i.e. can't wait a 1.x.1 which is usually release relatively shortly after a 1.x release) of something when suddenly it becomes urgent after a deadline especially when the release cycle lasts during several months and was very clear about the deadlines from the start.

Now about putting this in 1.9, the problem is that the more we put post-freeze deadline the more the risk we introduce a regression is increasing and thus the risk we have to delay the release. While you might not care about the release date, some others do care and actually 1.9 was mostly built around some contributions that have to be released early May (see 1.9 document) for both users benefit but also Dojo itself benefit (wrt to competition). I understand this is not easing your life, but until Dojo 2.0 and a separate release cycle for each sub projects I think what mostly drove a release should be valued more when determining a release cut than a bug that is here for months.

That said if everyone in copy of this bug is very confident with this fix from a regression perspective and if we do another RC because of a real regression (which might happen because of #16683) there might be an opportunity to get it in. I will not object to it being committed if those two conditions are filled even though I really hope this will not put us in trouble.

[Bryan Forbes]

In [31326]:

When there's a querySelectorAll in the DOM, but no qSA in responseXml, parse responseText with DOMParser. refs #15631 !strict

[Bryan Forbes]

In [31327]:

When there's a querySelectorAll in the DOM, but no qSA in responseXml, parse responseText with DOMParser. (1.8 backport) refs #15631 !strict

[bill]

This test case doesn't work on my machine. xmlXhr.php gets an error:

Parse error: parse error in /ws/trunk/dojo/tests/request/xhrXml.php on line 8

which is:

<?xml version="1.0" encoding="UTF-8" ?>

I think we had this problem before and you had to adjust the file. Probably depends on the version of PHP.

[Colin Snover]

In [31340]:

Fix dojo/request XML test for people that still have short_open_tag turned on in PHP. Fixes #15631.

[Colin Snover]

In [31341]:

Fix dojo/request XML test for people that still have short_open_tag turned on in PHP. Fixes #15631.
Backport to 1.8

[bill]

This change broke dojo/request on node, see #17138.