use Node.contains() to implement dojo.isDescendant()

[bill]

Use Node.contains() to implement dojo.isDescendant(). It's a little faster (although on my iPad it's 3x faster), and a little less code than the current implementation.

Unfortunately we need a separate code path for FF<9. (All other browsers support Node.contains().) So the full code becomes something like:

newIsDescendant = document.createElement("div").contains ?
function(node1, node2){
	// summary:
	//		Replacement function for dojo.isDescendant() (FF9+, IE, webkit, opera, iOS)
	node2 = dojo.byId(node2);
	return !!node2 && node2.contains(dojo.byId(node1));
} :
function(node1, node2){
	// Fallback function needed for FF8 and below
	node1 = dojo.byId(node1);
	node2 = dojo.byId(node2);
	return !!(node1 && node2 && (node2.compareDocumentPosition(node1) & 16));
};

See attachment for test case and benchmark.

See also #14570 about dropping FF3.6 support, but it says 1.9.

[bill]

test case and benchmark

[bill]

PS: could save a few more bytes by inlining the assignments:

 newIsDescendant = document.createElement("div").contains ?
 function(node1, node2){
        // Replacement for dojo.isDescendant() (FF9+, IE, webkit, opera, iOS)     
        return !!( (node2 = dojo.byId(node2)) && node2.contains(dojo.byId(node1)) );
 } :
 function(node1, node2){
        // Fallback function needed for FF8 and below
        return !!( (node1 = dojo.byId(node1)) && (node2 = dojo.byId(node2)) &&
               (node2.compareDocumentPosition(node1) & 16) );
 };

That feature test should also be in a has.add() so that it can be filtered out in a build.

[Eugene Lazutkin]

Replying to bill:

That feature test should also be in a has.add() so that it can be filtered out in a build.

Bill, could you provide a (trivial) patch for that? I am unsure where to put it.

[bill]

Turns out that unfortunately IE has issues with the new code in the edge cases:

• document.contains() is undefined. document.documentElement.contains() works though.
• DispHTMLDomTextNode.contains() is undefined. Clearly a text node cannot contain another node, so we should return false in this case, but it's another condition to check.
• normalDomNode.contains(DispHTMLDomTextNode) also doesn't work. Not sure how to work around that.

So, I worked up a patch using has() that defaults to the old code for FF <= 8 and IE <= 8, and uses the new code otherwise, but not sure if it's worth it. It's up to you. Pros: runs faster for modern browsers, and reduces code size for mobile builds. Cons: increases code size for non-browser-specific build case.

[bill]

patch using has()

[bill]

PS: We should definitely leave this ticket open though, because once we drop support for IE8 (since FF8 is already desupported), the patch becomes both smaller and faster than the current code.

[dylan]

We should probably land this anyway, as it's a good improvement and reduces build sizes with the static feature.

[Dylan Schiemann <[email protected]…>]

In 59b32a5eab1c4f48da9b3d2660900f0564a2bcac/util:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[dylan]

Closed via ​https://github.com/dojo/dojo/commit/13aa0fcc89d4433f376f6b329408b4efdb55ff7b

Not backporting as it's not a huge benefit, and could potentially break someone that's already implemented a similar patch in their own code. Please re-open if you feel that we should backport this.

[dylan]

Required an additional fix in master, ​https://github.com/dojo/dojo/commit/d28eb010cce1b4f977520357a4cbc5191da886a6 , as document isn't defined in the default global in all cases obviously.