Opened 10 years ago

Closed 8 years ago

Last modified 7 years ago

#10320 closed defect (fixed)

changeContentTable test errors in Opera

Reported by: gsnedders Owned by: bill
Priority: low Milestone: 1.7.5
Component: Query Version: 1.3.2
Keywords: needsreview Cc:
Blocked By: Blocking:

Description

Let me start off by explaining the cause of this bug by including a bit of code from dojo/_base/query.js:

root = root||getDoc();
var od = root.ownerDocument||root.documentElement;

// throw the big case sensitivity switch

// NOTE:
// 		Opera in XHTML mode doesn't detect case-sensitivity correctly
// 		and it's not clear that there's any way to test for it
caseSensitive = (root.contentType && root.contentType=="application/xml") || 
        (d.isOpera && (root.doctype || od.toString() == "[object XMLDocument]")) ||
        (!!od) && 
        (d.isIE ? od.xml : (root.xmlVersion||od.xmlVersion));

"root" is the root of the search or Document if none is provided.

The issues start with the second line of this extract, however: it would appear that "od" is meant to contain the ownerDocument, however, in the case that the root is a Document (as it will be if no node is provided as the root for the search), then ownerDocument will always be null. Currently this leads to root.documentElement being used as od in the case of root being Document or a DOCTYPE not yet added to a document (though you can special case as no query on that will ever match anything, if it isn't already).

In the case of od being the Document everything works fine in Opera currently (though relying upon a specific stringification of od seems risky), but when it is not, it always concludes that it is case sensitive (this is the case in the changeContentTable test, and the reason for the error).

It would seem best to fix this by checking whether root is a Document (this cannot be done in a cross-browser way through instanceof, as Opera will return false in some cases, as the behaviour is different in the undefined manner in which DOM interfaces map to ECMAScript), probably by checking for some property like documentURI (which only exists on Document) and using document.ownerDocument otherwise (and special case DocumentType? nodes somehow so they fail, as it makes no sense to run a query on something that cannot have children), and then just check whether od.createElement("div").tagName === "div" (it will always return "DIV" in the case of HTML documents, which is effectively what it is testing).

Change History (13)

comment:1 Changed 10 years ago by gsnedders

Simple example:

<!doctype html>
<script src="dojo.js"></script>
<p id=test></p>
<script>
alert(dojo.query("p#test").length);
</script>

That will fail as "p" (from the query) != "P" (from tagName). If you make the p in the query uppercase it will succeed.

comment:2 Changed 10 years ago by James Burke

Milestone: tbd1.5

comment:3 Changed 9 years ago by Adam Peller

Milestone: 1.51.6

comment:4 Changed 9 years ago by alex

Status: newassigned

comment:5 Changed 9 years ago by bill

Milestone: 1.6future

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

comment:6 Changed 8 years ago by Chris Mitchell

Owner: changed from alex to dylan

please review/triage

comment:7 Changed 8 years ago by dylan

Milestone: future1.7
Owner: changed from dylan to kriszyp
Status: assignednew

Is this relevant after the recently query refactor?

comment:8 Changed 8 years ago by bill

Owner: changed from kriszyp to Kris Zyp

comment:9 Changed 8 years ago by ben hockey

Keywords: needsreview added
Priority: highlow

comment:10 Changed 8 years ago by bill

Owner: changed from Kris Zyp to bill
Status: newassigned

Yes.

Version 0, edited 8 years ago by bill (next)

comment:11 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

In [27976]:

Redo logic for detecting if document is case sensitive a.k.a. an XML document, according to advice from gsnedders, thanks! Fixes #10320 !strict.

comment:12 Changed 7 years ago by Colin Snover

In [30035]:

Redo logic for detecting if document is case sensitive a.k.a. an XML document, according to advice from gsnedders, thanks! Fixes #10320 !strict. Backport to 1.7

comment:13 Changed 7 years ago by Colin Snover

Milestone: 1.81.7.5
Note: See TracTickets for help on using tickets.