views:

176

answers:

3

I want all buttons to perform an action before and after their normal onclick event. So I came up with the "brilliant" idea of looping through all those elements and creating a wrapper function.

This appeared to work pretty well when I tested it, but when I integrated it into our app, it fell apart. I traced it down to the 'this' value was changed by my wrapper. The sample code illustrates this; before you wrap the event handlers, each button displays the button id when you click, but after wrapping it the displayed name is 'undefined' in this example, or 'Form1' if you run it from within a form.

Does anybody know either a better way to do the same thing? Or a good way to maintain the originally intended 'this' values?

As you can imagine, I don't want to modify any of the existing event handler code in the target buttons.

Thanks in advance.

PS-The target browser is IE6 & up, crossbrowser functionality not required

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" 
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"&gt;

<html xmlns="http://www.w3.org/1999/xhtml"&gt;
<script language="javascript" type="text/javascript">
    function btnWrap_onClick()
    {
     var btns = document.getElementsByTagName("button");
     for( var i = 0; i < btns.length; i++)
     {
      var btn = btns[i];

      // handle wrap button differerntly
      if( "btnWrap" == btn.id)
      {
       btn.disabled = true;
       continue; // skip this button
      }

      // wrap it
      var originalEventHandler = btn.onclick;
      btn.onclick = function()
      {
          alert("Starting event handler");
          originalEventHandler();
          alert("Finished event handler");
      }
     }

     alert("Buttons wrapped successfully");
    }
</script>
<body>
    <p>
    <button id="TestButton1" onclick="alert(this.id);">TestButton1</button>
    <button id="TestButton2" onclick="alert(this.id);">TestButton2</button>
    </p>
    <button id="btnWrap" onclick="btnWrap_onClick();">Wrap Event Handlers</button>
</body>
</html>
+1  A: 

Your problem is the way closures work in JavaScript. Honestly, I'd recommend using a framework. Any of them should make event-handling far nicer than doing it by hand.

Hank Gay
+4  A: 

You can use the call method to resolve the binding, e.g. originalEventHandler.call(btn);

Alternatively, a library like prototype can help - its bind method lets you build a new function bound to a specified object, so you'd have declared originalEventHandler as var originalEventHandler = btn.onclick.bind(btn);

Finally, for a good backgrounder on binding issues, see also Getting Out of Binding Situations in JavaScript

Paul Dixon
+1 Thanks alot for the hints Paul. Just to let you know, I gave the answer to 'some' because he anticipated and answered some barriers I ran into. Thanks again.
John MacIntyre
+2  A: 

Like Paul Dixon said, you could use call but I suggest you use apply instead.

However, the reason I am answering is that I found a disturbing bug: You are actually replacing all your event handlers with the event handler of the last button. I don't think that was what you intended, was it? (Hint: You are replacing the value for originalEventHandler in each iteration)

In the code below you find a working cross-browser solution:

function btnWrap_onClick()
{
    var btns = document.getElementsByTagName("button");
    for( var i = 0; i < btns.length; i++)
    {
        var btn = btns[i];

        // handle wrap button differerntly
        if( "btnWrap" == btn.id)
        {
            btn.disabled = true;
            continue; // skip this button
        }

        // wrap it

        var newOnClick = function()
        {
            alert("Starting event handler");
            var src=arguments.callee;
            src.original.apply(src.source,arguments);
            alert("Finished event handler");
        }
        newOnClick.original = btn.onclick; // Save original onClick
        newOnClick.source = btn; // Save source for "this"
        btn.onclick = newOnClick; //Assign new handler
    }
alert("Buttons wrapped successfully");
}

First I create a new anonymous function and store that in the variable newOnClick. Since a function is an object I can create properties on the function object like any other object. I use this to create the property original that is the original onclick-handler, and source that is the source element that will be the this when the original handler is called.

Inside the anonymous function I need to get a reference to the function to be able to get the value of the properties original and source. Since the anonymous function don't have a name I use use arguments.callee (that has been supported since MSIE5.5) to get that reference and store it in variable src.

Then I use the method apply to execute the original onclick handler. apply takes two parameters: the first is going to be the value of this, and the second is an array of arguments. this has to be the element where the original onclick handler was attached to, and that value was saved in source. arguments is an internal property of all functions and hold all the arguments the function was called with (notice that the anonymous function don't have any parameters specified, but if it is called with some parameters anyway, they will be found in the arguments property).

The reason I use apply is that I can forward all the arguments that the anonymous function was called with, and this makes this function transparent and cross-browser. (Microsoft put the event in window.event but the other browsers supplies it in the first parameter of the handler call)

some
Now when I look at this again I realize that it should be refactored to use the same anonymous function, and store the value of the old caller on the original object. That would use less memory.
some
Thanks some, I did notice that bug, and was totally perplexed as to how I was going to reference the real original event hanlder, and calling control. You Rock!
John MacIntyre
@John MacIntyre: Thank you John! (I'm just curious if thats your real name or if is a reference to Trapper? Maybe both?) Btw, if you are going to use this in production you should consider to refactor it like I said in my comment above.
some
Real name. I did use it, and it will be rolled into a < 100 user production environment late next week. But I didn't follow what you were talking about with the refactoring that you mentioned. Could you append it to your answer? thx.
John MacIntyre
@John: Happy New Year! I'm on vacation for a week and don´t have my tools with me. The solution above works, it is clean, crossbrowser and don't interfere with anything else (it even works if you accidentally call it twice). However, it isn't the most memory efficient since it creates one (cont..)
some
anonymous function for every event handler. I would consider to refactor it to use only one function, and that the necessary data was stored at the element object instead.
some
Actually, the more I think about it, the solution I have in my answer is a generic one that works for every type of event. As long as you keep the code in the function short it is not a big waste of memory.
some
Thanks, Some. Happy New Year to you as well. I understand what you are saying. My function is exactly the same as what you have above, with the 2 'alert()'s replaced with my own function calls. Putting it in the button does make sense, but I'm pretty pleased with what we've got now.
John MacIntyre
May I ask you why don't you use your real name? You're obviously a pretty bright guy, googling you and finding an answer like this would be a pretty good thing I would think. Or are you some famous programmer who doesn't want to be hassled. ;-)
John MacIntyre
@John: Thank you! Sorry to disappoint you: I'm not a famous programmer. I usually don't use my real name on the net unless it is for official work. Sometimes it is good to be searchable and sometimes it's not and you usually don't know what will bite you until it is too late. I try to prevent (cont)
some
bad things from happening by using different names on different sites. It's an old habit and has worked so far. :)
some