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
});


A link that should take you to CodeReview (but doesn't)

A link that should not navigate anywhere (but does)


Or, well, it tries to navigate there but may be blocked because it's running inside an iframe. Check the console




-
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
div {
padding: 0.5em 1em 1em 1em;
margin-top: 1em;
border: 1px solid #000;
}

.clickable {
background: green;
}


Container (listener attached here)

Random DIV

Another DIV

Click target

Nested target




`

StackExchange Code Review Q#93861, answer score: 3