This project is archived and is in readonly mode.

#683 ✓resolved
Jesse

Element.hasChild Implementation

Reported by Jesse | June 8th, 2009 @ 12:11 AM | in 2.0 (closed)

The Element.hasChild() in MooTools 1.2.2 implementation iterates over all child elements of the parent to see if the parameter matches any of them:

hasChild: function(el){
   el = $(el, true);
   if (!el) return false;
   if (Browser.Engine.webkit && Browser.Engine.version < 420)
      return $A(this.getElementsByTagName(el.tagName)).contains(el);
   return (this.contains) ? (this != el && this.contains(el)) : !!(this.compareDocumentPosition(el) & 16);
},

Wouldn't it be faster and achieve the same purpose to instead iterate up from the (potential) child element through it's parentNode's until one of them matches the parent? Certainly the number of parentNodes of the child is less than the number of child nodes of the parent?

hasChild: function(el){
   el = $(el, true);
   if (!el) return false;
   while( el = el.parentNode ) if( elem == this ) return true;
   return false;
},

Perhaps even drop the $(el, true) for better performance in IE.

  • Jesse

Comments and changes to this ticket

  • Chris the Developer

    Chris the Developer June 8th, 2009 @ 07:52 AM

    Good idea, but there's just a small typo:

    hasChild: function(el){
       el = $(el, true);
       if (!el) return false;
       while( el = el.parentNode ) if( el == this ) return true;
       return false;
    },
    
  • Jesse

    Jesse June 8th, 2009 @ 08:12 AM

    Testing in Safari 3.2.3 (on Mac) my method is slightly faster - but not by much. However, testing in Firefox (on Mac) the MooTools method is much faster.

    I should have looked at the MooTools code more closely - I was focused on the "webkit < 420" implementation and didn't notice the native methods "contains" and "compareDocumentPosition" are going to beat out my while loop!

    I compared the performance of both methods in Safari 2.0.4 to try the "webkit < 420" case.

    Turns out my implementation is 1,300 times faster in Safari 2.0.4!

    Here is my test code:

    var main = $('main'); var elem = $$('span')[0];
    var has1 = false; var has2 = false;
        
    var start1 = (new Date()).getTime();
    for( var i=0; i<1000; i++ ) has1 = main.hasChild(elem);
    var end1 = (new Date()).getTime();
    
    var start2 = (new Date()).getTime();
    for( var i=0; i<1000; i++ ) has2 = main._hasChild(elem);
    var end2 = (new Date()).getTime();
    
    alert( has1+':'+has2+', time: '+((end1-start1)/1000.0)+':'+((end2-start2)/1000.0) );
    

    Here are representative performance numbers:

                 MooTools  Mine
    

    Safari 3.2.3: 0.0003 0.0002
    Firefox 3.0.10: 0.0007 0.0011
    Safari 2.0.4: 500.375 0.38

    Seems that the best method would be to retain the MooTools method for modern browsers and use my code for "webkit < 420":

    hasChild: function(el){
       el = $(el, true);
       if (!el) return false;
       if (Browser.Engine.webkit && Browser.Engine.version < 420){
          while (el = el.parentNode) if (el == this) return true;
          return false;
       }
       return (this.contains) ? (this != el && this.contains(el)) : !!(this.compareDocumentPosition(el) & 16);
    },
    

    A little more testing by someone with many browsers to play with would confirm the best mix of methods. For sure though the "$A(...)" in the MooTools version has got to go :)

    Jesse

  • Jesse

    Jesse June 8th, 2009 @ 08:30 AM

    I forgot to remove the el = $(el, true) from the final code above. This method is now faster in IE and Safari 2.x.

    hasChild: function(el) {
       if (!el) return false;
       if (Browser.Engine.webkit && Browser.Engine.version < 420){
          while (el = el.parentNode) if (el == this) return true;
          return false;
       }
       return (this.contains) ? (this != el && this.contains(el)) : !!(this.compareDocumentPosition(el) & 16);
    },
    

    Jesse

  • Valerio

    Valerio June 9th, 2009 @ 10:41 PM

    • Assigned user set to “Valerio”
    • State changed from “new” to “resolved”

    I implemented this for slick (the selector engine for mootools 2.0).

    http://github.com/subtleGradient/slick/commit/49634a58b0b506fb69c0f...

    I dont think we'll need this for 1.2.x.

  • Jesse

    Jesse June 9th, 2009 @ 10:50 PM

    Thanks. Looking forward to mootools 2.0.

    Jesse

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Shared Ticket Bins

People watching this ticket

Pages