[patch] [cla] Dojo.query on XML in IE 7 fails for nth-child selectors

[Jared Jurkiewicz]

Dojo.query on XML in IE 7 fails for nth-child selectors

I was trying to do a basic nth-child query across an XML dom in IE7 and discovered it fails. The same testcase works great in Firefox/Safari?.

Error in firebug lite: [ XmlDoc? ] FAILURE IN QUERY! [ Error: Object doesn't support this property or method ]

Testcase will be attached.

[Jared Jurkiewicz]

Testcase demonstrating the issue.

[nic]

I have almost fixed the bug, using setAttribute/getAttribute instead of node___cachedIndex? = index in the getNodeIndex function - it seems IE doesn't allow that, when node is an xml node. But there is another problem: both FF and IE don't allow setAttribute and getAttribute on the Document Node (nodeType = 9). Thoughts?

Nicola

[nic]

[cla][patch] fixes the "nth-child" selector (xml docs on IE7)

[Douglas Hays]

This test in query.html

doh.is(dojo.byId('_foo'), dojo.query('.foo:nth-child(2)')[0]);
[ Error: assertEqual() failed: expected [object HTMLDivElement] but got undefined ]

is failing consistently for me on Safari 4 public beta (528.16)/winxp - but works using dojo 1.2.3 so this is a regression. Not sure its related to this ticket. I tested using trunk [16936].

[Douglas Hays]

The nth-child regression in query.html using Safari 4 was caused by [16928] and apparently is not related to this ticket.

[bill]

1.3rc1 has been release; bumping remaining tickets to 1.4 (except for documentation/testing tickets)

[bill]

(sadly) punting seemingly abandoned ticket and meta tickets to future

[dylan]

Kris is inheriting query tickets for now.

[bill]

@nic - unfortunately this patch was never applied, and got stale. There's a getTag() function now to get the case sensitive tag. And the __cachedIndex etc. were renamed in [16532] to _cidx, _clen, and then in [16870] they were renamed to _i and _l. The pnc[] change was also affected by [16532].

I redid the patch against the latest code, as best I could, but the test case still fails. Can you take a look? I'll attach the updated patch.

[bill]

updated patch to apply to [27976], but doesn't fix test failure anymore

[bill]

Nic - see above.

[nic]

Replying to bill:

@nic - unfortunately this patch was never applied, and got stale. There's a getTag() function now to get the case sensitive tag. And the __cachedIndex etc. were renamed in [16532] to _cidx, _clen, and then in [16870] they were renamed to _i and _l. The pnc[] change was also affected by [16532].

I redid the patch against the latest code, as best I could, but the test case still fails. Can you take a look? I'll attach the updated patch.

Sure, I'll take a look. Thank you!

[nic]

Replying to bill:

@nic - unfortunately this patch was never applied, and got stale. There's a getTag() function now to get the case sensitive tag. And the __cachedIndex etc. were renamed in [16532] to _cidx, _clen, and then in [16870] they were renamed to _i and _l. The pnc[] change was also affected by [16532].

I redid the patch against the latest code, as best I could, but the test case still fails. Can you take a look? I'll attach the updated patch.

@bill

I changed lines 550-554 in your patch to:

root = root.nodeType != 7 ? root : root.nextSibling; // PROCESSING_INSTRUCTION_NODE[[BR]]

and it works well; I tested on IE9 (compatibility and standard mode), this weekend I'll setup two VMs to test IE8 and IE7
I used the test case with this selector "book:nth-child(4) > isbn" (the old selector returned an empty result); also, I had to remove isDebug:true to avoid runtime errors on IE.

[nic]

@bill
ok, the patch works on IE7/IE8 too.
Now there is that problem related to isDebug:true; on IE7 it triggers firebug lite and it works, but on IE8 and 9 a console.log(data) in a xhr success callback (with handleAs: "xml") causes an error

[nic]

patch to dojo/tests/query/query.html

[bill]

Hmm, well I tried the change you suggested, but now I get two test failures instead of 1. In the new test you added, and LOG: debug: FAILED test: doh.is(query('style')[0], query(':nth-child(2)')[0]); 0 ms. This is on IE8.

I'm attaching the combined patch. What results are you getting w/this patch?

(I tried removing the isDebug:true but that didn't change anything.

[bill]

patch to acme.js and test file, still not working for me on IE8

[nic]

This patch should work, tested on IE7/8 on windows XP and IE9/win7

[bill]

I tested it on IE8 and it fails for me for every selector setting. Does ​http://bill.dojotoolkit.org/trunk/util/doh/runner.html?testModule=tests.query really work on your IE8?

[nic]

Does this work for you? ​http://bill.dojotoolkit.org/trunk/dojo/tests/query/query.html

[bill]

Yes, ​http://bill.dojotoolkit.org/trunk/dojo/tests/query/query.html works on IE8, as does ​http://bill.dojotoolkit.org/trunk/dojo/tests/query/query.html?selector=acme, but not (for example) ​http://bill.dojotoolkit.org/trunk/dojo/tests/query/query.html?selector=lite.

So I guess you've fixed it in acme, but it's broken in the lite engine? I could check in your fix but leave the ticket open, and for the time being modify the test to only check acme. What do you think?

[nic]

Sounds good to me, in the meantime I'll try to fix the lite engine too. Thanks!

[bill]

In [28117]:

Fixes XML on IE in acme engine, but not lite engine, thanks for the patch nic, refs #7869 !strict.

As the comment in the parser.html test file says, when #7869 is completely fixed please move the xml_nthchild() test from the acme section to the css2 section.

[Colin Snover]

1.8 has been tagged; moving all outstanding tickets to next major release milestone.

[Kitson Kelly]

Won't fix, since move to 2.0 milestone and 2.0 won't support IE7.