Implementing a new event in an old mootools version I'm emulating a little bit the .on() from jQuery in an old Mootools version. One of the project requirements is that Mootools can't (shouldn't) be upgraded (using version 1.2.x).

Basically I'm checking the element itself and its two closest ancestors (like .parents() in jQuery).

My working code is:

Element.implement({
            addLiveEvent: function(event, selector, fn){
                this.addEvent(event, function(e){
                    var t = $(e.target);
                    if (!t.match(selector)) {
                        t = $(e.target.parentNode);
                        if(!t.match(selector)) {
                            t = $(e.target.parentNode.parentNode);
                            if(!t.match(selector)) {return false;} 
                        }
                    }
                        fn.apply(t, [e]);
                }.bindWithEvent(this, selector, fn));
            }
        });


I know those if() and the way I'm setting the variable t are not a good practice, however I'm not sure how to improve it.
return false
I'd consider this a bug. Returning false "kills" the event, meaning nothing else will happen. Just because your event handler shouldn't be called doesn't mean that's the end of the story.

For instance, if you have a regular, normal link in the element you're attaching the event listener to, then that link won't behave normally anymore. Clicking it will cause an event to bubble up and get handled by your addLiveEvent code, but if the event target (the link) doesn't match the selector, your code just defaults to returning false, effectively cancelling the click. So now your links don't work anymore. Oops.

Related to this: You should return whatever fn return, if fn gets called at all. Otherwise you have the opposite problem: You want to "cancel" a click, but the return value from the fn handler you define just gets ignored.

Here's an example (note that the 2nd link isn't cancelled by the code, as it should be, but may be cancelled by the browser, since snippets run inside an iframe, and are thus subject to security restrictions).




// OP's code
Element.implement({
  addLiveEvent: function(event, selector, fn) {
    this.addEvent(event, function(e) {
      var t = $(e.target);
      if (!t.match(selector)) {
        t = $(e.target.parentNode);
        if (!t.match(selector)) {
          t = $(e.target.parentNode.parentNode);
          if (!t.match(selector)) {
            return false;
          }
        }
      }
      fn.apply(t, [e]);
    }.bindWithEvent(this, selector, fn));
  }
});


// listen for clicks on a selector that won't match anything
$("case1").addLiveEvent("click", ".no-such-class", function() {});

// listen for link-clicks and try to cancel them
$("case2").addLiveEvent("click", ".cancel-click", function() {
  return false; // doesn't work
});
&lt;script src="//ajax.googleapis.com/ajax/libs/mootools/1.2.5/mootools-yui-compressed.js"&gt;&lt;/script&gt;

&lt;p id="case1"&gt;
  &lt;a href="http://codereview.stackexchange.com"&gt;A link that should take you to CodeReview&lt;/a&gt; (but doesn't)
&lt;/p&gt;

&lt;p id="case2"&gt;
  &lt;a class="cancel-click" href="http://codereview.stackexchange.com"&gt;A link that &lt;em&gt;should not&lt;/em&gt; navigate anywhere&lt;/a&gt; (but does)
  &lt;br&gt;
  &lt;small&gt;Or, well, it &lt;em&gt;tries&lt;/em&gt; to navigate there but may be blocked because it's running inside an &lt;code&gt;iframe&lt;/code&gt;. Check the console&lt;/small&gt;
&lt;/p&gt;





fn.apply(t, [e])
There's no need for apply when it's just one argument; just use call
bindWithEvent
As far as I can tell, this isn't necessary. You're attaching an even listener to this, and then you're binding it to this. Makes no difference, as far as I can tell.


And yeah, those nested ifs aren't very nice. You're also limiting it to 2 ancestors, which seems like something that could easy prove too limited.

You could do a recursive solution, but it's much simpler to loop through an array instead.

I looked up the docs, and it seems there's a Element.getParents method you can use. So if you know the event.target, you can simply get an array of all its ancestors, and go through them. Of course, you'll want to stop if you hit the element that you originally added the live event listener to (otherwise you'll continue to check ancestors all the way to document), but that's a simple equality comparison.

Please note that it's been almost a decade since I last used Mootools, but this should work:



Element.implement({
  addLiveEvent: function(event, selector, fn) {
    this.addEvent(event, function(event) {
      var target = $(event.target), // get event target
          path = target.getParents(), // get ancestors
          i, l;

      // add the target to the list, so it gets checked first
      path.unshift(target);

      // loop through the list
      for (i = 0, l = path.length; i &lt; l; i++) {
        if ($(path[i]).match(selector)) {
          return fn.call(path[i], event); // got a match
        }

        if (path[i] === this) {
          // we've reached the element we attached to, so stop
          break;
        }
      }
    });
  }
});

// Usage
$("container").addLiveEvent("click", ".clickable", function (event) {
  // here, `this` will refer to the matched element
  alert(this.title);
});
div {
  padding: 0.5em 1em 1em 1em;
  margin-top: 1em;
  border: 1px solid #000;
}

.clickable {
  background: green;
}
&lt;script src="//ajax.googleapis.com/ajax/libs/mootools/1.2.5/mootools-yui-compressed.js"&gt;&lt;/script&gt;

&lt;div id="container"&gt;
  Container (listener attached here)
  &lt;div&gt;
    Random DIV
    &lt;div&gt;
      Another DIV
      &lt;div class="clickable" title="Clicked the target"&gt;
        Click target
        &lt;div&gt;
          Nested target
        &lt;/div&gt;
      &lt;/div&gt;
    &lt;/div&gt;
  &lt;/div&gt;
&lt;/div&gt;