3

I need a cross-browser function for registering event handlers and a (mostly) consistent handler experience. I don't need the full weight or functionality of a library such as jQuery, so I've written my own. I believe I've accomplished my goals with the code below, and so far my testing has been successful, but I've been staring at it for too long. Are there any flaws in my logic or gotchas that I'm missing?

EDIT 1: Commented each block with browser intent for clarity. Updated IE block to not call func right away (thanks to Andy E's keen eyes).

EDIT 2: Updated IE block to call func.call() with this instead of elem.

EDIT 3: Updated to pass JSLint with "the Good Parts."

function hookEvent(elem, evt, func)
{
    if (typeof elem === "string")
    {
        elem = document.getElementById(elem);
    }
    if (!elem)
    {
        return null;
    }
    var old, r;
    if (elem.addEventListener)  //w3c
    {
        elem.addEventListener(evt, func, false);
        r = true;
    }
    else if (elem.attachEvent)  //ie
    {
        elem[evt + func] = function ()
        {
            func.call(this, window.event);
        };
        r = elem.attachEvent("on" + evt, elem[evt + func]);
    }
    else                        //old
    {
        old = elem["on" + evt] ? elem["on" + evt] : function (e) { };
        elem["on" + evt] = function (e)
        {
            if (!e)
            {
                e = window.event;
            }
            old.call(this, e);
            func.call(this, e);
        };
        r = true;
    }
    return r;
}
Community
  • 1
  • 1
Billy Jo
  • 1,326
  • 19
  • 32
  • 1
    jQuery is a very lightweight library. What's wrong with using it? Someone's already done all the work for you. Or are you just not interested in learning it? – Matt Ball Aug 25 '10 at 17:49
  • 2
    @Bears will eat you - 66K plus, compared to this, is far from lightweight. Especially if that's all he needs. @Bill Ayakatubby - By "cross browser", you really mean, "standard stuff that IE doesn't handle properly". – Rob Aug 25 '10 at 17:52
  • 2
    @Bears: At this point in time, the only cross-browser concern I have is event registration, so I have no need of jQuery or another library. If my situation changes, I'll reevaluate the necessity of a library. – Billy Jo Aug 25 '10 at 17:54
  • There's really little point in the last branch now. There are no major browsers left that don't support either `addEventListener` or `attachEvent`. – Tim Down Aug 25 '10 at 23:01
  • migrate to http://codereview.stackexchange.com/ – Knu May 30 '11 at 00:50

1 Answers1

3

There's a problem on this line:

r = elem.attachEvent("on" + evt, func.call(elem, window.event));

This will execute func() immediately, instead of attaching it as a handler for the event. Instead, the return value of func() will be assigned to the event, which will throw an error if it's type isn't "function".

I can understand that you don't want to use a framework, but many (many) others have written cross-browser event handling snippets. John Resig has one version, Google for "javascript addEvent" for many more.

http://www.google.com/search?q=javascript+addevent

Andy E
  • 338,112
  • 86
  • 474
  • 445