A problem with John Resig’s addEvent

A while ago, Peter-Paul Koch and some other JavaScript-interested bloggers held a competition for improving the "addEvent" function. The winner was John Resig's flexible JavaScript events function.

It creates a new property of the object you add an event listener to as follows:

obj['e'+type+fn] = fn;

The main purpose of that is to make the "this" keyword work inside event listeners. It is a clever hack, though looks a bit ugly – to ensure that the object property has a unique name, it concatenates "type" and "fn", meaning the browser has to decompile the function and use its entire source as part of the property name. Decompiling a function object is slow, the property name becomes nothing short of ugly. Perhaps a few lines of code to make a prettier unique property name would be worth it?

Besides the ugliness, here comes the real problem: the code assumes that all implementations can decompile functions. Opera can on desktop, but to save footprint this feature is disabled on mobile phones. Function.prototype.toString simply returns "[ECMAScript code]". Hence, each call to Resig's addEvent will overwrite the previous function object for that type, and the last function added will be called as many times as addEvent itself has been called. I'm afraid the clever hack has turned rather destructive – bad enough to break cross-platform compatibility for any app.

As written, the function makes the assumption that all browsers which have attachEvent support has IE's bugs. The simple fix would be to follow this rule: use object detection and try the standards-compliant path first. First looking for the standards-compliant (thus probably the best defined) function might be a good general rule of thumb for compatibility. Cross-browser support for a method that is implemented by trying to figure out what another browser is doing is likely worse than support for a method specified in detail by the W3C, so by choosing the W3C stuff if it exists should get you better compatibility.

Advertisements

9 thoughts on “A problem with John Resig’s addEvent

  1. Yeahhhh…. I don't really recommend that anyone actually try and use that function. You have to remember that in this particular contest they rewarded brevity of code over accuracy (which was a mistake, in my opinion), and only required that the code work in the most recent browsers. I simply followed the letter of the contest and came up with the shortest solution that met the rules.Since then, I've been using a derivative of Dean Edwards' addEvent solution. It's suited me well, but even that has a number of extenuating circumstances (like the fact that you need to use attachEvent to get an IFrame load event in IE, amongst other weird bugs). For this reason, in jQuery we're going to be moving to a system that's based on Dean's system, but uses only W3C Events, or IE's attachEvent (rather than the DOM 0 onevent properties).We're hoping to have this in place for the upcoming 1.1.3 release, but we'll have to see, as there may be a number of unforeseen circumstances.

  2. @FataL: Nope, we forked Dean's addEvent code a while back (Jan 2006). More information about his solution can be found here:http://dean.edwards.name/weblog/2005/10/add-event/It looks like he has an alternate solution contained within his new base2 library, but we're probably not going to use it (for the main reason that he implements methods that don't exist in a browser, and jQuery takes a firm policy of not implementing missing functions).Although, in the end, while the two solutions (jQuery and base2) will be pretty similar in functionality; the actual implementation is where they will differ the most (especially considering that he doesn't go out of his way to fix any extra event properties).

  3. ah, that contest ;)I still silently blame PPK for being short-sighted there; at least Dean gave me credits for my entry because it was quite simular to his' and we managed to create quite a useful solution in a joint effort – which in fact I'm still using, although slightly modified, today 🙂

  4. I thought concatenating function source code was crazy and thus the solution appeared flawed to me from the start. I'd never use it. It wastes resources and is not accurate (imagine two different instances of a function). Sorry, but the judges must have been drinking ;)@crisp: If you are referring to this version, I think you're right, it's a beautiful solution.Dean's/your concept is tempting… for it changes the order in that events fire AND it would be possible to implement a whole new event dispatch system (i.e. you could implement capturing event listeners cross browser – at a heavy loss of performance, I suppose).I personally don't think it's prudent to use inline event handlers (on+type) in a solution that has to work with other code. It's nice that you preserve other's functions and execute them, but they won't be as courteous. Your move to put W3C's method first is the only right one. I don't know if it's worth using attachEvent, but generally I can't stand the thought that some other code prevents events from firing because it overrides an on+type property…

  5. _Grey_: I agree that it is not the most perfect solution but I can't think of any other solution that is more perfect. The fact is that IE's event-model is just too broken to be anywhere useful and that some older browsers don't support either the W3C event-model or IE's event-model (eg older versions of Safari and IE for the Mac).

  6. I don't really recommend that anyone actually try and use that function

    Would you mind telling the web developers for Scandinavian Airlines off for doing so? ;-)Seriously, given the problems we're beginning to see for Opera Mobile I'd appreciate if you would update the contest blog post with a few comments about the drawbacks of your solution..

  7. @John – base2 doesn't add any missing functions either. It does not touch any built-in objects. Although I have made it easy to enhance Array if you want to.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s