1 Posted by Peter Siewert on 18 October 2005 | Permalink

Congratulations John.

On a side note, would it be worth adding 2 additional lines of code to make it semi-work on older browsers?

In addEvent after the
if{}
else if{}
put a final
else{ obj["on"+type] = obj["e"+type+fn]; }

In removeEvent after the
if{}
else if{}
put a final
else{ obj["on"+type] = null; }
This will make the event registration work as long as only one function is registered per event type on any given object.
I'll leave it to smarter people than I to discuss whether partial functionality is better or worse than no functionality at all.

2 Posted by Tino Zijdel on 18 October 2005 | Permalink

I'm a bit disappointed; first of all I copy/paste these new functions into the original contest page, and it doesn't work in IE6 (see http://therealcrisp.xs4all.nl/upload/addEvent_winner.html )
Secondly, before the rewrite, John's entry leaked memory.
Thirdly, since this is for IE a wrapper around attachEvent, the execution order of the events is different ('random' according to Microsofts documentation, but LIFO in practice against FIFO using the W3C event model) so in my opinion this is still not a truly crossbrowser solution.

I do however applaud John's entry for it's simplicity and I think it is a big step forward compared to the old addEvent script.

3 Posted by Milo van der Leij on 18 October 2005 | Permalink

For browsers supporting *both* addEventHandler() and attachEvent() (such as maybe IE7 or IE8), the fact that detachEvent() is tried first in removeEvent() will be a problem.

4 Posted by Analgesia on 18 October 2005 | Permalink

first of all. why does addEvent check for obj.addEventListener and then obj.addEventListener, but removeEvent the other way around. Is there a special reason for this? As far as I see this will break if IE starts implementing W3C's model.

Secondly. Am I right in assuming that one should still put the following at the beginnen of an eventhandling-function?:
if(!e)e=window.event;

5 Posted by Tino Zijdel on 18 October 2005 | Permalink

Here's a working version for IE: http://therealcrisp.xs4all.nl/upload/addEvent_winner_fixed.html

drawbacks: leaks memory like a sieve in IE

another difference I found: when you attach the same function to the same event for the same element it gets executed twice in IE and only once in other browsers.

6 Posted by Brian LePore on 18 October 2005 | Permalink

Tino,

The code as posted by PPK as my new add event function, and it attaches my form validation script fine for IE6 for my contact page ( http://www.powrsurg.com/contact/ ), so it does work in some cases....

Of course, now for some reason Gecko runs my script on submit and then submits the form anyways... but that is probably something wrong on my end.

7 Posted by Tino Zijdel on 18 October 2005 | Permalink

Brian: the events do get attached, but you are not using the 'this' keyword in your functions (scope) - that's where PPK's function goes wrong...
Furthermore: return false won't cut it for you in your function; you will need to use preventDefault() in Gecko to prevent the submit.

8 Posted by Jonas on 19 October 2005 | Permalink

Well, I didnt liked the result... I was expecting a more creative entry to win :)

Was it really necessary to make a contest to arrive in such simple code?

9 Posted by Jeffery To on 19 October 2005 | Permalink

Yes this is quite brief. Congratulations John. addevent() lives on.

John's version preserved 'this' by wrapping obj["e"+type+fn] inside obj[type+fn]. Without this intermediate step, 'this' reverts back to the global object in Explorer.

A different issue: Say we have two separate anonymous function objects with identical bodies, ie:

var foo = function () { alert('foo'); };
var bar = function () { alert('foo'); };

When both are added to the same object and event type, in Explorer we would be able to remove only one of them.

Would this happen in real-world applications? Not likely. Still it's something I'd keep in mind.

10 Posted by David Flanagan on 19 October 2005 | Permalink

The winning entry is certainly brief.

But it relies on Function.prototype.toString() to work on IE. Aesthetically very difficult to swallow. And I assume quite inefficient (unless the IE JS implemenation keeps the original source text of a function around).

I don't know if IE has any documented or undocumented limits on the length of a property name, but it there are limits, use of this function will make people exceed them.

If you're going to promote this solution, at least avoid invoking Function.prototype.toString() *twice* in each function. And, I think you should use the delete operator instead of simply setting the property to null on removal. Otherwise the property (with its very long name) still exists.

My sense is that this solution (which involves decompiling compiled JavaScript code) is worse than the problem (broken this keyword) it addresses.

11 Posted by Lucky on 19 October 2005 | Permalink

I would suggest this:
/*
Original idea by John Resig
Tweaked by Scott Andrew LePera, Dean Edwards, Peter-Paul Koch, and Jon Casey
*/

function addEvent(obj, type, fn) {
if (obj.addEventListener)
obj.addEventListener(type, fn, false);
else if (obj.attachEvent) {
obj.detachEvent('on'+ type, obj['e'+ type + fn]);
obj.attachEvent('on'+ type, obj['e'+ type + fn] = fn);
}
}

function removeEvent(obj, type, fn) {
if (obj.removeEventListener)
obj.removeEventListener(type, fn, false);
else if (obj.detachEvent)
obj.detachEvent('on'+ type, obj['e'+ type + fn]);
obj['e'+ type + fn] = null;
delete obj['e'+ type + fn];
}
}

and had one final question, would this work in opera as well? esp, the window events needing to apply to document.addEventListener instead?

12 Posted by Lucky on 19 October 2005 | Permalink

sorry posted too soon, had a typo:

/*
Original idea by John Resig
Tweaked by Scott Andrew LePera, Dean Edwards, Peter-Paul Koch, and Jon Casey
*/

function addEvent(obj, type, fn) {
if (obj.addEventListener)
obj.addEventListener(type, fn, false);
else if (obj.attachEvent) {
obj.detachEvent('on'+ type, obj['e'+ type + fn] || new Function);
obj.attachEvent('on'+ type, obj['e'+ type + fn] = fn);
}
}

function removeEvent(obj, type, fn) {
if (obj.removeEventListener)
obj.removeEventListener(type, fn, false);
else if (obj.detachEvent) {
obj.detachEvent('on'+ type, obj['e'+ type + fn]);
obj['e'+ type + fn] = null;
delete obj['e'+ type + fn];
}
}

13 Posted by Angus Turnbull on 19 October 2005 | Permalink

PPK: Your changes have done more harm than good, unfortunately!

The key point in John's initial entry is storing the closure as a property of the object (so it can be successfully detached later, something Tino's tweak doesn't cover) and calling the attached function as a method of the object from within the closure. Your changes have negated these benefits; calling attachEvent with obj["e"+type+fn] will call that as a direct function reference rather than a method (breaking the "this" keyword in MSIE/Win). Also, calling with "window.event" within the closure works fine and I'd keep it in place, as "window" is global and works regardless of the scope, and its "event" property is obtained at runtime.

Your other changes are good, though, namely:
* A conditional test on addEventListener for older browsers.
* Cleaning up both obj[type+fn] and obj["e"+type+fn] to prevent memory leaks.
* Changing the order so addEventListener is preferred (although ensure that removeEventListener is also preferred!).

Also, Peter Siewert's idea of a DOM0 fallback is good, like the original addEvent -- might as well keep an additional "else if" in there. And storing the sourcecode of the function as a variable will speed it up. So, I'll post my mods:

14 Posted by Angus Turnbull on 19 October 2005 | Permalink

/* Here' my modifications discussed in the comment above; "View Source" for nicely-formatted version */

function addEvent( obj, type, fn ) {
if ( obj.addEventListener ) {
obj.addEventListener( type, fn, false );
}
else if ( obj.attachEvent ) {
var eProp = type + fn;
obj["e"+eProp] = fn;
obj[eProp] = function() { obj["e"+eProp]( window.event ); };
obj.attachEvent( "on"+type, obj[eProp] );
}
else {
obj['on'+type] = fn;
}
};
function removeEvent( obj, type, fn ) {
if ( obj.removeEventListener ) {
obj.removeEventListener( type, fn, false );
}
else if ( obj.detachEvent ) {
var eProp = type + fn;
obj.detachEvent( "on"+type, obj[eProp] );
obj['e'+eProp] = null;
obj[eProp] = null;
}
else {
obj['on'+type] = null;
}
};

15 Posted by Erik Arvidsson on 19 October 2005 | Permalink

Doing a detachEvent before adding solves one of the issues.

I agree that relying on toString is stretching it a bit.

The delete operator does not work on DOM nodes and removeAttribute does not work on window :'(

The FIFO/LIFO problem is actually a real issue.

Storing expandos will leak memory so don't forget to clean up afterwards.

16 Posted by Robert Nyman on 19 October 2005 | Permalink

I'd also make sure to add the capture as a parameter. Now it's hardcoded like this:

obj.addEventListener( type, fn, false );

but I guess it should be:

obj.addEventListener( type, fn, bCapture );

17 Posted by Erik Arvidsson on 19 October 2005 | Permalink

As Tino pointed out. This solution does not call the funtion with the right object and therefore 'this' still points at the global object

18 Posted by Angus Turnbull on 19 October 2005 | Permalink

There's still a few problems I haven't fixed with that modification, as other commentators have pointed out:

1) It can't attach the same method of multiple objects (due to function source code hashing), as Jeffery To noted. Here's another example of something that will fail:

var a = new Widget('foo'), b = new Widget('bar');
addEvent(window, 'load', a.setup);
addEvent(window, 'load', b.setup);

However, this is easy to workaround with another intermediate function:

addEvent(window, 'load', function() { a.setup() });
addEvent(window, 'load', function() { b.setup() });

1) It'll leak memory like a sieve in MSIE. Not just the "storing long property names" issue that David Flanagan raised (a good point -- a quick test reveals that IE may be able handle several-hundred-KB-sized object properties though), but the whole "DOM-to-JS hanging references not removed ONUNLOAD" issue. I solved this in my own competition entry, but John's has the substantial advantage of brevity on its side ;). I guess this'll motivate IE users to switch/upgrade, especially if addEventListener() is preferred here, and supported in IE7.

3) Random order of event execution in MSIE, as Tino observed. If you code with this in mind it shouldn't be a problem.

19 Posted by Lon Boonen on 19 October 2005 | Permalink

For some reason my entry was deleted 2 minutes after I submitted it. Maybe because I suggested that this contest was forgetting some stuff, like:
1. code-license
2. event-object compatibility

Anyway. Here is my entry once more, it fixes all problems mentioned in this discussion:
1. no memory leaks by design
2. this context available
3. event available (of course)
4. multiple at/detaches possible

The code has 32 lines and will appear in the next comment as this one is full.

groetjes Lon

20 Posted by Lon on 19 October 2005 | Permalink

// copyright Q42 (www.q42.nl)
function addEvent(obj, type, fn) {
if (obj.attachEvent) {
var idx = __attachedFns.length;
__attachedFns[idx] = { obj:obj, type:type, fn:fn };
var dfn = __attachedFns[idx].dfn = __createDecoupledFn(idx);
obj.attachEvent("on" + type, dfn);
} else
obj.addEventListener(type, fn, false);
}
function removeEvent(obj, type, fn) {
if (obj.attachEvent) {
for (var i=0; i<__attachedFns.length; i++) {
var data = __attachedFns[i];
if (data.obj == obj && data.type == type && data.fn == fn) {
obj.detachEvent("on" + data.type, data.dfn);
data.obj = null;
}
}
} else
obj.removeEventListener(type, fn, false);
}
var __attachedFns = [];
function __createDecoupledFn(idx) {
return function(evt) {
var obj = __attachedFns[idx].obj;
obj.___fn = __attachedFns[idx].fn;
var result = obj.___fn(evt);
obj.___fn = null;
return result;
}
}

21 Posted by Lon on 19 October 2005 | Permalink

Oops, forgot one small thing: please insert "break;" after "data.obj = null;"

Excusez moi.

22 Posted by ppk on 19 October 2005 | Permalink

Well, obviously I made a few mistakes in tweaking the functions. I changed this entry a bit, so that it now offers a working function.

23 Posted by ppk on 19 October 2005 | Permalink

A few more answers to some comments:

- I obviously have a lot of studying to do when it comes to these tricky bits. I rarely need them in my day-to-day work, so I keep forgetting the salient points.

- This function is not meant to combat memory leaks. I agree that it's a serious problem, but now that we've got a centralized function to set and remove event handlers, it's not very hard to add a bit that removes all event handlers onunload. But let's get the basics right, first.

- David, you're right about the functions: they can become a problem. Maybe we could use a counter instead of the function? As to delete, I see your point here, too.

- Analgesia: you are completely right; the two models should be called in the same order in addEvent() and removeEvent(). The new version does so, and gives the standard model pride of place.

- Robert: I don't want to add the capturing flag since the attachEvent() model doesn't support capturing anyway. If you try using event capturing the results in the browsers will differ. Better not to pretend that capturing is supported. Besides, I've never needed it in a practical project.

All in all I've got plenty of material to take a stab at writing a version 1.1

24 Posted by Lon on 19 October 2005 | Permalink

Removing event handlers onunload is not a solution if you have DHTML. The element that needs removal of attached events might already have been removed, thus causing memory to leak.

Save yourself the hassle of keeping an administration of all attached events by attaching them the right way in the first place. Id est in a clean closure.

25 Posted by ppk on 19 October 2005 | Permalink

Lon,

Do you have a page where you explain how your functions work? I vaguely see the point, but I'd prefer having things spelled out for me.

26 Posted by Cal Smith on 19 October 2005 | Permalink

I wasn't a contestant, but an observer and find the final comments of the contest organizers a bit surprising. Not sure what they expected. It seems to me they can save themselves the trouble and 'disappointment' of wading through unnecessary, non-sensical lines of low-quality, uneconomical code submissions by simply not asking for them in the future.

Quirksmode is a great resource to have, and I appreciate the time and effort it must take to keep it current and living. I can't imagine, though, that such comments will encourage entries for future contests. Maybe a sign should be hung on the homepage: Gurus only.

27 Posted by Angus Turnbull on 19 October 2005 | Permalink

Thanks for fixing that up, ppk!

As Lon mentioned, this still isn't a good starting point to combat memory leaks. You'd really need a means of tracking objects to which events have been attached in the addEvent() function (say, a global array of all objects the script has processed) at least.

Lon's entry, simply put, calls attachEvent() with a function that adds the real function-to-be-run as an expando property onto the DOM, runs it, then removes it. This avoids memory leaks as (as far as I know) attachEvent() itself doesn't cause memory leaks, only appending DOM properties, so adding/removing on the fly won't leave them in place when the page unloads. It's a good function -- IMHO, superior to the chosen one -- with its only major drawbacks being:

1) The random event order issue common to all attachEvent-using scripts.
2) It doesn't check for already-attached functions; attach the same function twice and it'll work in IE and fail in others. It's an easy fix as he doesn't hash on function sourcecode; add a for() loop in addEvent() to fix.
3) Its global array isn't spliced in removeEvent() so adding/removing the same function multiple times will cause it to grow infinitely long.
4) It doesn't work in IE/Mac (not really an issue).

28 Posted by Nikola Klaric on 19 October 2005 | Permalink

Uhm, I guess the whole point of the exercise was to find a replacement with _less_ ramificications, not just _different_ ones.

The "final" code in it's present form still has two major problems:

(1) The line

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

serializes the function body into part of the name of a property. I dare to say that future versions of both Mozilla * and Internet/Windows Explorer will most certainly throw an exception due to prohibited characters that are perfectly fine in a function body but not as a property name.

(2) The lines

obj[type+fn] = null;
obj["e"+type+fn] = null;

do _not_ delete the respective properties, but rather set them to the null value. Therefore the description, "Secondly, John omitted to remove obj["e"+type+fn] in the removeEvent() function," is wrong, the present code will not do what it claims.

Please use

delete obj[type+fn];

instead.

Overall the "solution" is highly implicit as opposed to explicit. The code itself lacks comments, and many statements are needlessly general.

For instance, I'd rather omit

if (obj.removeEventListener)

and write instead

if (typeof(obj.removeEventListener) == "function")

or, even more explicitly,

if (typeof(obj.removeEventListener) === typeof(new Function()))

29 Posted by Angus Turnbull on 19 October 2005 | Permalink

Also, ppk, you note in the article that "We had to wade through dozens of lines of script that didn't really make much sense to us" and say that "I vaguely see the point, but I'd prefer having things spelled out for me" in the comments.

Given that cross-browser JS hacking is a pretty dark art at the best of times with all sorts of random issues (note: I'm not faulting you or anyone for not picking through all the entries with a fine tooth comb!), this whole competition could probably benefit from broader input as we have in the comments here.

So, why not post your shortlist of the functions that satisfy the competition criteria on a wiki? Let everyone append pros/cons to each entry. After a day or two, narrow it down, and allow editing/patching the function source for elegance/compatibility/issues/etc. We'd get a kick-ass addEvent() function benefiting from a lot of expertise.

30 Posted by Lon on 19 October 2005 | Permalink

Hi Agnus,

>1) The random event order issue common to all attachEvent-using scripts.

I wasn't even aware of this problem. In all my years of coding I have never encountered this. In my opinion your code shouldn't rely on it. Who attaches multiple event-handlers and depends on the order in which they are executed?

>2) It doesn't check for already-attached functions; attach the same function twice and it'll work in IE and fail in others. It's an easy fix as he doesn't hash on function sourcecode; add a for() loop in addEvent() to fix.

Right. Another solution would be not to attach multiple identical event handlers.

>3) Its global array isn't spliced in removeEvent() so adding/removing the same function multiple times will cause it to grow infinitely long.

Correct. Although entries are disabled after removeEvent. I didn't want to use splice to keep IE50. And I didn't want to add more lines to simulate splice as brevity seems to be a major selling point.

>4) It doesn't work in IE/Mac (not really an issue).

I wasn't aware there was such a browser (insert smiley here).

Lon

31 Posted by Marcus Tucker on 19 October 2005 | Permalink

Lon's code definitely seems to be the best of the lot, but with the 3 reservations that Angus has posted (4 is a moot point).

Angus, please elaborate on point 2 (exactly what change should be made?) and presumably point 3 is fixable?

As for 1, code which *must* be executed in a specific order can be handled simply by wrapping the code in an intermediate function, so this is just a caveat which needs to be kept in mind when using these event scripts, rather than a showstopper.

32 Posted by ppk on 19 October 2005 | Permalink

As far as I can see the most important remaining issue is how to look up events that have already been attached, either to remove them or to prevent duplicate attachments.

Meanwhile I've become convinced that the obj[type+fn] solution can lead to problems. On the other hand, looping through an array of all attached functions can become unwieldy, too.

The site I worked on when I discovered the addEvent() problems had about 250 event handlers, and I'm not sure that looping through all of them is a good idea, especially not when you want to check for duplicates while attaching these 250 handlers.

What we'd really need is some sort of hashtable to look up attached events, with some sort of unique key that is not the function body.

Maybe we should give each element a separate hashtable instead of using one global one?

The event type + function *name* would be a good choice for a key, but I'm not sure how to read out the function name. Besides, there's always the problem of anonymous functions.

I envision something like:
"has function doSomething already been attached to obj as a mouseover event handler? See if obj['mouseover_doSomething'] exists"

Any bright ideas?

33 Posted by Lucky on 19 October 2005 | Permalink

My post #12 shows the proper way to wire up IE, so it will not cause duplicate events.

You simply detachEvent before adding, to assure it does not duplicate your execution.

IMHO, this is a flaw in IE's handling, for duplicate execution of the EXACT same function is ridiculous + redundant :) to say the least.

AS FOR memory?
Simple use of attachEvent + detachEvent will not cause a Memory leak in IE.

it is the assigning of:
element[onclick] = function() {}
and not nulling it beforeunload.
and if you
declare a variable to element reference within a scope, and not nullify it.

g'luck!

34 Posted by James Mc Parlane on 19 October 2005 | Permalink

Robert - if you want a something that handles capture and bubbling consistently across all the major browsers, check out my entry in the original competition page.

35 Posted by Chris Wesseling on 19 October 2005 | Permalink

One thing remaining is. Cancelling default behaviour and preventing propagation.

Elsewhere on this site I found that event.preventDefault() can be done in IE by setting event.cancelBubble to true.. While in fact you should set event.returnValue to false.

I tried

window.event.preventDefault = function()
{
this.returnValue = false;
}
window.event.stopPropagation = function()
{
this.cancelBubble = true;
}

before passing it to the eventlistener, but it ends up "undefined"...

I guess for older browsers you will still have to return false, to cancel default behaviour.

36 Posted by James Mc Parlane on 19 October 2005 | Permalink

Re: ppk - the hashtable

Thats what I did in my entry. There is a global hashtable of elements that have listeners. Each element entry in the hashtable has its own array of handler types.

Each handler type has an array of listener functions and the event propagation type.

Keeps it very neat. On listener removal the hash entries are kept consistent such that when the last listener is removed for an element, that elements hash table entry is removed as well. I always know if an element has listeners and I always know what type they have assigned through a simple array look-up.
Aids in performance as well. I doubt I could emulate the full w3 event behaviors without this structure to filter potential target elements in the bubble and capture phases.

37 Posted by Ismael Jurado on 19 October 2005 | Permalink

Con gratulations to the winner!

I think I resolved the problem for the hashtable and the name of the key on my contest submission (http://www.ismaelj.com/articulos/addevent-recoding-contest/):

a) In IE every HTML element has sourceIndex property that maps as an index to document.all. I use this property instead of element.uniqueId so I can later find the function on removeEvent
b) a new property is added to the function being attached to the event. The name is formed by the event type (mouseover, onload,..) + element.sourceIndex

The neat effect is that we are storing the event/element pair being assigned as a property IN the function, and the benefits are:

a) No duplicated functions for a given event on the same object can be assigned

b) We don't need an array to store the event/elements relationship

38 Posted by Lon on 19 October 2005 | Permalink

To Ismael, number 37.

Your code looks nice, but is faulty on two respects:
1. you cannot attach events to the window object. This can be remedied rather easily

but worse:

2. your code potentially leaks memory
If fn is a closure that contains a (indirect or direct) reference to the HTML element it is attached to.

To be really sure your memory is not leaking, which is the goal of this whole exercise, the addEvent code has to make absolutely sure there is no (circular) reference.

To solve this you will have to resort to the array/hash model again, I'm, afraid.

39 Posted by Tino Zijdel on 19 October 2005 | Permalink

I agree with the remark of Angus; obviously this isn't getting us anywhere.
I have seen some pretty nice scripts (even some good ideas in the scripts that you bluntly discarded in the first round); I'd say combining those ideas instead of the whole contest concept will be far more efficient and will yield a better result.

Aiming for the briefest script doesn't quite appeal to me - what's an extra K or so in a script that most likely will only be downloaded by a client once and read from cache from thereon. I'd rather have a script that's straight-forward, understandable and can easily be enhanced or improved. Brief also doesn't always imply that it's fast.

by the way; there is a logical place to store eventhandlers for an object, namely within the DOM space of the object itself - something I utilized in my entry (which I updated today, new version can be found here: http://therealcrisp.xs4all.nl/upload/addEvent2.html ).

40 Posted by David Flanagan on 19 October 2005 | Permalink

ppk: in response to your comment #32. I don't think you can use anything other than toString() as a hash key. JavaScript just doesn't have a hashCode() method.

If you don't like looping through an array of all handlers ever registered, the solution is to have one array for each element that has handlers. My contest entry (as well as others) takes this approach. (Although I also keep a global array of *all* handlers so I can remove them on unload.)

41 Posted by Nikola Klaric on 19 October 2005 | Permalink

Why not make a hash function part of the solution? For instance, CRC32 fits in less than 10 lines of JavaScript code.

42 Posted by Jim Ley on 19 October 2005 | Permalink

Nikola Klaric, typeof (obj.addEventListener) doesn't have to return "function" or the same as typeof (new Function) it's a host object not a javascript one, so implementations are free to do what they want. UA's certainly take advantage of this e.g. typeof(window.external.ShowBrowserUI) in IE.

43 Posted by Nikola Klaric on 19 October 2005 | Permalink

Jim,

I don't see how and why any javascript host would ever want to implement a callable method such as addEventListener by any other means than making it an instance of Function. The example you cited dates back to MSIE 4.0, implements a highly exotic feature, and was never meant as good practice in that and later browsers.

Also, there are plenty of reference examples from Microsoft themselves that use typeof(method) == "function" tests ...

44 Posted by Nikola Klaric on 19 October 2005 | Permalink

(FWIW, window.external.ShowBrowserUI is "undefined" in MSIE 6.x anyway.)

45 Posted by Angus Turnbull on 20 October 2005 | Permalink

Lon and Marcus: Thanks for your responses! I'd just like to stress that I think Lon's function is great, and my quibbles were minor; I especially like its approach to stopping IE leaks (which is better than ONUNLOAD handling). I've actually updated my own entry:

Competition: http://www.twinhelix.com/test/ppk_addevent.html
Download: http://www.twinhelix.com/javascript/addevent/

to store its event arrays as a global variable rather than an object property, which enhances its leak-busting abilities.

In more detail the two fixes I was considering were:

1) In addEvent(), have a "for" loop through the global list of event objects, and if the object and type are equal, and (stored function == new function) return.
2) Since IE5.0 doesn't support splice, use a "function removed" variable in the "for" loop in removeEvent(), and set array[i] = array[i + 1] for each entry once you've found your function, before calling array.length-- at the end. Voila, homemade splice :).

Yeah, they raise bytesize, but I think they're worthwhile (see my entry for example implementations). My own entry already addressed all of the points raised in this discussion (like Lon's it doesn't hash on source, but unlike his, it doesn't use attachEvent at all).

46 Posted by John Serris on 20 October 2005 | Permalink

Seriously guys, the wiki idea is very good. Put everyone's function on a wiki and let people make comments on them. There are so many little quirks that it's impossible for the 3 judges to know them all.

The great thing is that everyone seems to have some experience with a particular quirk. Why not combine everyone's knowledge and let them ALL be judges?

A lot of these ideas could be combined into one super script that deals with almost every issue mentioned. :)

47 Posted by Laurens van den Oever on 20 October 2005 | Permalink

As usual at Q42, Lon's code went around and was improved on. Since I just wrote a closure function which wraps the original function in such a way that 'this' is set to a given object [1]. Using the closure function the addEvent and removeEvent implementations are trivial, see next post.

1. http://laurens.vd.oever.nl/weblog/items2005/closures/

48 Posted by Laurens van den Oever on 20 October 2005 | Permalink

// copyright Q42 (www.q42.nl)
function addEvent(obj, type, fn)
{
if (obj.addEventListener)
obj.addEventListener(type, f, false);
else if (obj.attachEvent)
{
removeEvent(obj, type, fn);
obj.attachEvent("on" + type, fn.closure(obj));
}
}

function removeEvent(obj, type, fn)
{
if (obj.removeEventListener)
obj.removeEventListener(type, f, false);
else if (obj.detachEvent)
obj.detachEvent("on" + type, fn.closure(obj));
}

49 Posted by Laurens van den Oever on 20 October 2005 | Permalink

Ehm.. f must be fn ofcourse.

50 Posted by Dean Edwards on 20 October 2005 | Permalink

Here's my solution:

http://dean.edwards.name/weblog/2005/10/add-event/

51 Posted by Tino Zijdel on 21 October 2005 | Permalink

I like the idea from Dean to strip all object detections and go with something that should work in almost every browser that has a basic JS implementation. My latest version can be found here: http://therealcrisp.xs4all.nl/upload/addEvent3.html

It also allows for removal of attached anonymous functions through addEvent.

52 Posted by Angus Turnbull on 21 October 2005 | Permalink

Laurens: Your code relies on Function.apply() so won't work in IE5.0/Win, as per the competition rules.

Dean: Your entry is similar to mine in that it implements a full event handler using the DOM0 model to avoid attachEvent woes (and so works all the way back to v4 browsers). However, it doesn't work with:

// Your code:
addEvent(document, 'click', yourFunction);

// Someone else's script programmed in a similar fashion:
document._oldOC = document.onclick;
document.onclick = function(e) {
if (this._oldOC) this._oldOC(e);
otherFunction(e);
};

// Another one of your scripts:
addEvent(document, 'click', yourOtherFunction);

Even if you adjust your pre-existing function check, since you use one array, it'll invoke handleEvent() twice. My entry uses nested arrays to fix this.

Also, you should remove the $$handleEvent method once run (I made your function leak in MSIE with Tino's leak test), and I'm not sure if storing functions in element.events is helping there either. Using a for(;;;) loop rather than a for(x in y) loop will avoid Object.prototype extension woes (yeah, that's bad practice, but people do it). I'm also not sure how your function deals with adding one function to multiple events (a global GUID will break that).

53 Posted by Lucky on 21 October 2005 | Permalink

dean, you miss one very crucial part?
event.stopPropagation()??
i don't see how your model would cause the other handlers to not fire, if one needs to cancelBubble ??

54 Posted by James Mc Parlane on 21 October 2005 | Permalink

I like your code a lot Dean, I've taken a similar approach and emulated event propagation under IE, but you seem to be emulating for all browsers.

The code is beautiful, but its just not as fast as it could be if you did an object detect and used addEventListener where available.

55 Posted by Angus Turnbull on 21 October 2005 | Permalink

James: Yes, that's what my entry does - I feel that we should default to the DOM standard model as that's the way we're moving as an industry, but fallback to a fully compliant emulated version where DOM Events are unsupported.

Seriously, I'd be interested if anyone can break my addEvent script. Most of the objections raised here (execution order, identical functions, memory leaking, etc) I've (hopefully) addressed already. Link:

http://www.twinhelix.com/javascript/addevent/

View the demo and read the explanation in the HTML document as the source itself is pretty compact. Feedback is welcome!

56 Posted by Angus Turnbull on 21 October 2005 | Permalink

D'oh, just found a small bug in my addEvent(). Fixed version uploaded at the same URL as above. Any feedback yet?

Also, Tino: please don't extend Array.prototype in your script! For instance, try running this with your script in the page:

var foo = [1, 2, 3];
for (var i in foo) alert(i);

You'll see it includes 'indexOfFunction' in the for loop.

Secondly, you're using "delete" on an array -- it doesn't actually change the array length. Your handleEvent() function copes with this though, but it'll cause your array to grow quite large with repeated add/removeEvent calls.

Finally, many of the points I raised above in relation to Dean's script apply to yours too. Try the code I posted above!

57 Posted by stylo~ on 21 October 2005 | Permalink

I didn't write this (was Tim Morgan), but this works even in ie5 mac and so I've used it for years. Any problems with it that people can see?

// adds an event listener to an element
function aE(el,ev,f){
ev=ev.replace(/^on/i,'') ;
if(typeof f=='string')f=new Function('e',f.replace(/this/,'e.currentTarget')) ;
if(!el['_on'+ev+'callees']){ // first event of this type
el['_on'+ev+'callees']=[] ;
// create a wrapper function that intercepts the event...
var wF=el['_on'+ev+'w']=function(e){
e=e||event ;
if(!e.target)e.target=e.srcElement ;
if(!e.currentTarget)e.currentTarget=el ; // create this for IE
for(var i=0 ; i<el['_on'+ev+'callees'].length ; i++) el['_on'+ev+'callees'][i](e) ;
} ;
if (el.addEventListener) el.addEventListener(ev,wF,false) ;
else if (el.attachEvent) el.attachEvent('on'+ev,wF) ;
else el['on'+ev]=wF ;
}
el['_on'+ev+'callees'][el['_on'+ev+'callees'].length]=f ;
} ;

// remove an event listener
function rE(el,ev,f){
ev=ev.replace(/^on/i,'') ;
var fd=0 ;
var c=el['_on'+ev+'callees'] ;
for(var i=0 ; i<c.length ; i++){
if (c[i]==f) fd=1 ;
if(fd&&i<c.length-1) c[i]=c[i+1] ;
}
if(fd) c.length-- ;
if(c.length==0){
var wF=el['_on'+ev+'w'] ;
if(el.removeEventListener)el.removeEventListener(ev,wF,false) ;
else if(el.detachEvent)el.detachEvent('on'+ev,wF) ;
else el['on'+ev]=null ;
}
} ;

58 Posted by stylo~ on 21 October 2005 | Permalink

The last bit:

else if(el.detachEvent)el.detachEvent('on'+ev,wF) ;
else el['on'+ev]=null ;
}
} ;

59 Posted by Tino Zijdel on 21 October 2005 | Permalink

Angus: I would love to give you feedback, but then I would like to see a non-compacted version ;)
Also I see no harm in using Array.prototype - lots of websites use prototyping on the Array object to simulate missing methods in for instance IE5. I also use it to simulate some of the methods introduced in Gecko 1.8 for IE (like indexOf which is quite handy). If you use for-in on an array you should cope with that (I made the same mistake before), else use an Object since prototyping on Object is less general and is considered bad practice.

Furthermore I don't think the expansion of the array by using delete is that costly; I'll test it sometime though.

60 Posted by Angus Turnbull on 21 October 2005 | Permalink

stylo~: That's an interesting script (like Dean's and mine it works in very old browsers), but it doesn't deal with the "this" keyword, which was the main point of this recoding contest. You might be able to address that, but it might still have memory leak problems too.

Tino: That is the main version -- what, are you scared by single-letter variable names? :P. Maybe I should put up a "friendly" translation. Still, it's easily possible to follow through.

Regarding arrays, I don't see why you can't replace:

Array.prototype.foo = function(bar) { this.doStuff(bar); }

with:

function processFoo(arr, bar) { arr.doStuff(bar); }

There's no real size or functionality difference...? I just feel that for a script that's destined to be used everywhere, we shouldn't force changes to global objects.

61 Posted by Laurens van den Oever on 21 October 2005 | Permalink

Agnus: From the closure.js script:

// Ancient browser compatibility
if (!Function.prototype.apply)
{
Function.prototype.apply = function (obj, args)
{
obj.___fn = this;
var str = "";
for (var i = 0; i < args.length; i++)
str += (i != 0 ? "," : "") + "args[" + i + "]";
eval("var result = obj.___fn(" + str + ");");
obj.___fn = null;
return result;
};
}

62 Posted by Angus Turnbull on 21 October 2005 | Permalink

Laurens: Whoops -- I missed that. Sorry!

However (in the spirit of playing devil's advocate here), it's a fairly large amount of script devoted to a fix. And you need to deal with arguments that are strings containing double-quotes or ending in a backslash, without corrupting your evaluated string :).

PPK: Just wondering, where are we headed from here?

63 Posted by Niko Klaric on 21 October 2005 | Permalink

Angus,

uhm, if you're asking people for feedback it might be helpful to provide a non-obfuscated version of the code. Dissecting a line like

'o = x.o, a = x["' + n + '"][' + (t.length - 1) + '];'

is not really worthwile, as I'm pretty sure you have a human-readable version of the code somewhere.

64 Posted by Sjoerd Visscher on 21 October 2005 | Permalink

Angus: "And you need to deal with arguments that are strings containing double-quotes or ending in a backslash, without corrupting your evaluated string :)."

No, you are confused with other implementations of apply. Laurens' version evals a string like:

"var result = obj.___fn(args[0],args[1],args[2]);"

65 Posted by Tino Zijdel on 21 October 2005 | Permalink

Angus: I tried to understand your functions a little bit. It looks to me that using your function the same handler for the same event on the same object can be attached multiple times in IE which is not the behavior of attachEvent or addEventListener.
Unfortunately IE raises an javascript error on attaching the first event (length is null or not an object).

Normally I have no problems dissecting some code, but you should really provide a non-obfuscated/compacted version.
In general I think that a script that is destined to be used everywhere should be easy to follow ;)

As to your remark about me using a prototyped method instead of a function: I think you are right on this one, but only because this is not a general method but one with a specific use. Before I used a more general indexOf method of which I feel that prototyping is preferable since it is a handy method that can also be used in other scripts.

66 Posted by Angus Turnbull on 21 October 2005 | Permalink

By popular demand I've put together a human-readable version (underneath the bytesize-optimised version in the main project document). Comments and long variable names a-plenty!

(There's no changes/bugfixes in this update, by the way).

67 Posted by Angus Turnbull on 21 October 2005 | Permalink

Hmm, simultaneous posting! In reply to the posts I missed above:

Sjoerd: You're quite right; my apologies to yourself and Laurens. That apply() method is much more robust than my mistaken interpretation! I'll quit whining about that script now ;).

Tino: A readable version is uploaded. Also, make sure you uncomment the three commented lines in addEvent() if you want duplicate checking in IE (it's commented at the moment as IMHO that's a non-essential feature).

68 Posted by Tino Zijdel on 21 October 2005 | Permalink

Angus: I don't think duplicate checking is non-essential; your scripts acts differently now in different browsers ;)

It would also help if you told us where to find the readable version ;)

69 Posted by Niko Klaric on 21 October 2005 | Permalink

Angus,

thanks for providing a readable version.

A few suggestions/questions:

- Why do you make the methods explicit properties of window instead of declaring functions in the current context?
- Use while- instead of for-loops.
- Your methods don't have an explicit return-value, so there's no point in returning the addEventListener/removeEventListener results.

70 Posted by Angus Turnbull on 21 October 2005 | Permalink

Tino: Same file, scroll down a bit. Also, you're right regarding duplicate checking, so since my last post, I've uncommented it by default.

Niko: Good points! In order:

1) I'm assigning addEvent dynamically as the script is wrapped in a block that tests for "aeOL" to allow multiple inclusions per page to interoperate correctly. Using "function addEvent" syntax means that IE will instantiate that function at "compile" time. Here, the function is created only once at runtime, and the first addEvent in the page is the only copy that runs. This will allow different versions to interoperate without crashing, too.

2) "while" loops? I guess I could, but when incrementing a counter variable I use "for" loops out of habit. There's not much difference IMHO.

71 Posted by Angus Turnbull on 21 October 2005 | Permalink

... 3) That's a holdover from the bytesize-optimised version where I can call add/removeEventListener and quit all in one line :). There's no significance to the return value, as the script always tries to work regardless of the browser event model support (i.e. it never identifies a "failed" event addition).

Ack, I'm tired and going to bed (New Zealand time). Happy hacking, all!

72 Posted by Niko Klaric on 21 October 2005 | Permalink

I'm not sure I understand what you're trying to say.

if (!window.aeOL) {
function addEvent(obj, name, fn, legacy) {}
}

will also declare the function "dynamically", i.e. only when the condition is true.

Also, I don't understand why you'd want several revisions of addEvent running. The point is that the caller will not differentiate between them anyway, so why not just declare the function and have the container sort it out?

As for while-loops, you could rewrite e.g.

for (var i = 0; i < typeArr.length; i++) {
...
if (splice) { arrayFns.length--; return }
}

as

var i = 0;
while (i++ < typeArr.length) {
...
if (splice) {
arrayFns.length--;
break;
}
}

which is a lot cleaner (note the break statement).

73 Posted by Lucky on 21 October 2005 | Permalink

Can I break up the flow a bit?
This may be a practical solution that works for many?
It grows upon Laurens closure() approach, but enhanced to consistently .translate the given event object, so you always get a somewhat w3c Event Object model as your argument.

All done w/ no memory leaks in IE, and using the addEventListener || attachEvent methods, so event bubbling preventDefault work wonderfully.

LINKS:
http://www.teamcon.com/pub/concepts/events/events.js
http://www.teamcon.com/pub/concepts/events/events.htm
http://www.teamcon.com/pub/concepts/events/events-stress.htm

74 Posted by RIcklas on 21 October 2005 | Permalink

What is wrong with just using the old addEvent function and finding the Target element rather than going through all this trouble just so you can use the 'This' Keyword?

75 Posted by Lucky on 21 October 2005 | Permalink

It is all in preference.
NO one is saying you must use the methods provided here.

A) use browser supplied addEventListeners, and then translate the event object accordingly every time in the handler:
if (!e) e = window.event;
var target = e.srcElement || e.target;
placed in every handler you create!!

B) Use a system of consistently translating the events model to an Event model you would want to use now and for upcoming browsers. (that finally come around and update their models to the appropriate one) hint, hint! :)

76 Posted by Tino Zijdel on 21 October 2005 | Permalink

RIcklas, Lucky: the biggest problem with the IE event model lies in the fact that the handler runs in the scope of the window object instead of the scope of the object that fired the event. Furthermore by lack of a currentTarget property there is no way of knowing which object is actually handling the event in bubbling phase.

77 Posted by Lucky on 21 October 2005 | Permalink

yes, and my solution is fixing that.

78 Posted by Tino Zijdel on 21 October 2005 | Permalink

Lucky: no it doesn't. target/srcElement contains a reference to the element that first received the event, and that is not neccesarily the element that actually has the handler defined on (that could be an underlaying element).
For instance: an anchor wrapped in a list-item, with a mouseover handler attached to the list-item to change the background-color. Now when you mouseover the anchor the mouseover of the list-item fires (because events bubble up), but event.srcElement will always point to the anchor.

79 Posted by Lucky on 21 October 2005 | Permalink

you are talking to the wrong person, i'm well aware of this situation, and the code i show, does call the method within the scope of the element you want the listener attached to, and yes, srcElement will be a dif reference in cases.

perhaps you meant to address the earlier post, of WHY DO THIS?

for you preaching to choir on the NEED for this approach.

80 Posted by Tino Zijdel on 21 October 2005 | Permalink

Lucky: I'm merely explaining why point A) in your remark #75 is not a suitable solution (you made it look like that); at least not for IE, making that point invalid so only option B) remains. The 'why do this' is definitely born out of a need, and PPK already explained that earlier: http://www.quirksmode.org/blog/archives/2005/08/addevent_consid.html

I'm not sure what you'd like to discuss further. Your code perhaps? I'll look into it when I have the time; at this point I think Dean's solution is probably one of the best after some tweaks.

81 Posted by Lucky on 21 October 2005 | Permalink

This just seemed like a bad place to continue soo many discussions on this topic, it's not a real forum for posting soo much information, and we could possibly get more solutions flushed out within another environment.

Yes, please look upon the code in links post #73; thanks, love to have some extra GURU eyes upon the code.

My only issue on Dean's at present time was the lack for event.stopPropagation + event.preventDefault and IE counterparts of this event.cancelBubble + event.returnValue.. blah..

and to incorporate that functionality will bloat his code quite a bit.

perhaps we should stop looking for "pretty code" and really look into "fully-functioning" code, that you don't need to worry about!
create a bullet-proof solution and never need to look at it again, until some other browser comes and improves upon, or breaks :)

82 Posted by Angus Turnbull on 21 October 2005 | Permalink

Lucky: Your code needs Function.call. Perhaps borrow and modify Laurens' script above for IE5.0 compatibility, as per the rules?

Niko: Thanks for the extra feedback! To explain myself, please try this code:

if (!window.foo) { function foo() { alert(1) }; }
if (!window.foo) { function foo() { alert(2) }; }
foo();

In Firefox it alerts "1" whereas in MSIE6/WinXP it alerts "2". Counter-intuitive, huh?

So, I'm considering the situation where two developers (say, of a AJAX script and a popup tooltip script) both incorporate addEvent into their code, and a web page designer includes both via . I want the first inclusion of addEvent() to handle all events on that page, and the second to be completely ignored, which means I have to work around that IE bug. I admit it's funny coding but I hope you can see my reasons!

Also, with while() loops "i" is incremented at the start, which means that you'd have to refer to "array[i-1]" or adjust your conditional test. A "do while" loop could be a better fit, but a "for" loop works perfectly well.

Finally -- yeah, "break" and "return" are equivalent there. I'll substitute "break" in the next revision. Good spotting :).

83 Posted by Angus Turnbull on 21 October 2005 | Permalink

Tino: Yours, Dean's and my scripts are all conceptually similar. However, please run this:

addEvent(document, 'click', new Function('alert(1)'));
document._oldOC = document.onclick;
document.onclick = function(e) {
if (this._oldOC) this._oldOC(e);
alert(2);
};
addEvent(document, 'click', new Function('alert(3)'));

This is why I use nested arrays and "instances" of an event handler function -- both your scripts run that in an incorrect order, and/or multiple times. This situation will definitely be an issue with several scripts by different authors running on a page.

Also, why not default to addEventListener/removeEventListener and return after calling them?

Lucky: Cancelling events is easily handled by a separate function. I bundled a cancelEvent() routine with my entry, and PPK had a bubble-canceller in his original contest template. I can't see why we need to worry about putting that in addEvent() itself.

84 Posted by Tino Zijdel on 21 October 2005 | Permalink

Angus: I tested that with both mine and Dean's script. I did not notice any mistaken duplicate calls, but you are right about the order.
However... I (finally) have come to the conclusion that order is irrelevant. Both the W3C specifications on the DOM 2 event model as Microsofts documentation on their event model specifically state that handling order is undefined. So per spec you cannot rely on the exection order, which makes it per definition bad practice to rely on the characteristics of implementations of these event models.

85 Posted by Tino Zijdel on 21 October 2005 | Permalink

btw, I did some reworking on Dean's script which result you can find here: http://therealcrisp.xs4all.nl/upload/addEvent_dean.html

86 Posted by Niko Klaric on 22 October 2005 | Permalink

Angus,

I was not aware of those ramifications. Intuitively I'd thought that any function declaration by a given symbol overrides the former one, i.e. that Firefox' behavior is wrong.

But since you can't rely on the script's host actual behavior I wouldn't waste any time and space correcting for that behavior, and just define the functions as such, i.e. not in the explicit scope of window.

87 Posted by Angus Turnbull on 22 October 2005 | Permalink

Tino: Good point, I guess. IMHO, if we can match implementations we might as well do so, as a lot of web tech is based on a consensus notion of how the standards should be implemented. But then again, maybe it's worthwhile just saying "warning, execution order is not defined" and letting people keep that in mind as they code.

Then again, if we're not worried about order (and are happy to ignore IE/Mac), we might as well just use attachEvent() like Lon's script...?

I still think you should default to using the DOM methods though :). If we're having a "new standard", it should use the real standards where supported, as they'll be faster in practice and you'll never have forwards-compatibility problems.

88 Posted by Niko Klaric on 22 October 2005 | Permalink

Tino,

interesting. But you should really leave out those cosmetical changes, they make it a lot harder to see where you improved on Dean's code. (And I don't consider renaming "event" to "e" an improvement.)

What's the purpose of the line

if (!Object.prototype[i])

You don't mean hasOwnProperty(), do you?

And you should rethink initializing retValue with true (and subsequently change the boolean expression below). If a single event is applied and returns null then retValue should be null as well.

And this.__fn should be deleted with the delete operator.

89 Posted by Angus Turnbull on 22 October 2005 | Permalink

Tino: Your changes are good (especially incorporating add/removeEventListener) but Dean's script has one really fatal flaw: attaching a GUID to functions doesn't allow you to attach the same function to several events. Try:

function foo() { alert(1) };
addEvent(document, 'mouseup', foo);
addEvent(document, 'click', foo);

Additionally, you need to wrap the whole thing in an "if (!window.addEvent && !addEvent.guid)" block, as otherwise including it multiple times in a page will keep resetting the GUID and horribly breaking things.

Niko: Assigning window.addEvent means I can rely on the host's behaviour in that regard. Relax, it doesn't detract from the functionality of the script in any way :).

90 Posted by Tino Zijdel on 22 October 2005 | Permalink

Lucky: looked at your code (link to the forum is gone, but I don't feel like registering for just this anyway)

1) You really seem to go out of your way just to prevent memory-leaks which makes it overly complex and adds to overhead when registering events
2) Use of call() makes it unusable for IE5.0 and possibly other browsers
3) Not usable for browsers that do not have either addEventListener or attachEvent implemented
4) From the look of it this will also add some overhead for each eventcall; to what extend I did not test
5) currentTarget: currentTarget || e.srcElement - ouch, currentTarget != srcElement (!)
6) (just a tip) /^[13]$/.test(o.nodeType) - ouch, compiling a regexp in each iteration; I personally think this is quite a nice alternative:

var nodecheck = {1:1,3:1};
if (o.nodeType in nodecheck) { }

7) (just a tip) for (var i = 0; i < this.cache.length; i++) {} can be rewritten as var i = this.cache.length; while (i--) {} which is faster and order doesn't matter here.

91 Posted by Niko Klaric on 22 October 2005 | Permalink

Angus,

yes, it does. For example, incorporating the event handling script inside an HTC will let the handlers be declared in the scope of window, while in actual fact symbols (i.e. variables and functions) declared in the HTC are not visible in window (i.e. the HTC container) but in the document object of the HTC. These are two separate planes.

So still no benefit in using the window scope, but at least one example where the script misbehaves.

92 Posted by Tino Zijdel on 22 October 2005 | Permalink

Angus: try this in IE and be surprised:

function foo() { alert(1); }
document.attachEvent('onmouseup', foo);
document.attachEvent('click', foo);

and then try this and see that both mine and Dean's functions are working fine:

function foo() { document.body.appendChild(document.createTextNode('it works! ')); }

As for the wrapping; I personally have not ever seen anybody do that. People copy/pasting scripts from various sources are bound to walk into problems anyway and I don't feel the need to accomodate their own lack of interest in at least getting a clue about the scripts they are using.

Niko: I haven't read up on hasOwnProperty() yet, but this construction seems to work for me when I add a Object.prototype.foo = function() { alert('oops'); }
I'm not so happy about the retValue thing either, but at least this is better than nothing. I guess it could be improved indeed.

About using delete for this.__fn; I agree, but IE5.0 doesn't ;)

93 Posted by ppk on 22 October 2005 | Permalink

I think this discussion is ready for a slight pause. Comments temporarily disabled.

In the mean time, I'd LOVE to see a short and clear summary of the discussion, so if someone could prepare it and post it when I re-enable the comments, I'd be very happy.

94 Posted by Angus Turnbull on 25 October 2005 | Permalink

Woo, comments reopened :).

Tino, Niko and I have been emailing around modifications over the last couple of days, and I'm now beginning to think that execution order is less important. Also, that "alert" issue I posted above is definitely a red herring -- I should've looked deeper into MSIE's alert handling, as in real life it has no effect on event handlers.

So, Dean's script with some of Tino's memory-leak-avoiding tweaks, and a defaulting-to-DOM-Events modification could well fit the requirements for the new AddEvent!

95 Posted by Marcus Tucker on 25 October 2005 | Permalink

Sounds promising... is a Dean+Tino hybrid script available somewhere?

96 Posted by Tino Zijdel on 25 October 2005 | Permalink

http://therealcrisp.xs4all.nl/upload/addEvent_dean.html

Dean is currently working on a new version

97 Posted by Tino Zijdel on 25 October 2005 | Permalink

Summary of the discussion sofar: http://therealcrisp.xs4all.nl/upload/addEvent_discussion_summary.html

98 Posted by James Mc Parlane on 26 October 2005 | Permalink

I think there are two schools of thinking on what an addEvent replacement should be.

One believes in creating a solution with the absolute minimum of code that does the absolute minimum required.

The other believes that any solution should involve making IE conform to the W3C standard for addEventListener.

99 Posted by Lucky on 26 October 2005 | Permalink

for the dean + tino code:

addEvent(a, "click", handler1Func);
addEvent(a, "click", handler2Func);

a.onclick = function(e) {blah blah}
now accidentally wiped out all stored events? :O

silly = yes
happens = yes

100 Posted by James Mc Parlane on 26 October 2005 | Permalink

Lucky - yes it does happen, but I would not hold it against the dean and tino code.

addEventListener browsers would treat

a.onclick = function(e) {blah blah}

as

a.removeEventListener("click",a.onclick,false);
a.addEventListener("click",function(e) {blah blah},false);

I have made an effort to emulate this behavior as much as possible but without a proper document 'mutation' listener, its not really practical. The alternative is expensive reconciliation of the listeners vs hash-tables, its bordering on the not really worthwhile.

I only go to the extent that I do so that my behavior library can work on top of existing HTML with inline events with no side effects, but at a point even I have to draw the line at using a replacement for addEventListener and then wilfully mixing event handler styles that bypass the replacement.

101 Posted by martin on 6 November 2005 | Permalink

I'm afraid modified winner's code could still cause memory leaks in MSIE. Here is simple example.
http://p2b.jp/demos/IE-memory-leak1-en.html

102 Posted by Tino Zijdel on 6 November 2005 | Permalink

Martin: if you took the time to read the comments and/or my summary ( http://therealcrisp.xs4all.nl/upload/addEvent_discussion_summary.html ) you would have known that that is an acknowledged issue and that we've been busy writing an alternative addEvent that doesn't have the disadvantages of the "winner's" code.

103 Posted by Tino Zijdel on 6 November 2005 | Permalink

Mmmz, seems like I spoke to soon since your alternative indeed uses my and Dean's approach.
I do like Dean's idea to generate a unique ID for function references better since it provides quicker lookups to see if a function is already attached as a handler for some element.
Be aware that the for-in constructs could create havock when you prototype the array object.
Also IE5.0, besides the lack of call/apply, doesn't support Array.push() and deleting properties from a DOM object.

104 Posted by martin on 7 November 2005 | Permalink

Tino: thank for your reply and discussion summary. I think it is IE's memory leakage problem that should be solved first in this context. Using of attachEvent is not essential and there are surely alternatives, so we'd better use them until IE solves his own problem. I'm looking forward to more sophisticated approaches from pros like you and hope IE7 will free from it.

105 Posted by Tino Zijdel on 7 November 2005 | Permalink

Martin: indeed attachEvent is not essential. Circumventing use of IE's propriety methods has advantages or at least saves us from the disadvantages. That's why I (and Dean as well) choose this approach.

I'm actually waiting for PPK for a more formal conclusion so far. Right now the discussion here seems dried up and gone to other places without an actual concensus from the ones who started al this, which is a shame...

106 Posted by Lon on 8 November 2005 | Permalink

Usage of element["on" + evtType] is not guaranteed to be compatible with other code that uses this method of attaching events. So don't do it. Oh, and it's so 20th century...

A perfect multi-usable solution has been suggested by Laurens. It will not only help you in this case, but in all other possible cases where you want to prevent memory leakage.

I do like the fixEvent solution though, let's keep that one. And let's add some cross-browser button-detection to the event object as well.

107 Posted by Tino Zijdel on 8 November 2005 | Permalink

Lon: people that just copy/paste code from the web, combining it without knowing what they're doing are likely to get burned anyway.
If you want to use this addEvent solution then you just shouldn't mix it with other methods of attaching events.

108 Posted by Lon on 8 November 2005 | Permalink

Tino: I disagree. A piece of code so generic as attachEvent should have no dependancies.

Suppose I want to build a nice little tool using this code that people then can integrate into their website. For instance a bookmarklet, or a statistics-counter, or a cross-site comments-app, or whatever.

Or what if people using attachEvent also use some statistics-counter that is doing document.onclick = whatever...

We are trying to build a new version of attachEvent that can be used in all kinds of projects/sites/applications. This is the time to make sure there are no known problems if you decide to use this code.

109 Posted by Tino Zijdel on 8 November 2005 | Permalink

Using attachEvent for IE to me is no option, it is just too broken.
And what about browsers that support neither addEventListener or attachEvent (IE/Mac for instance)?
And what are the odds that some other code in a website will overwrite an eventhandler attached to an element of *your* tool (just steer away from attaching events to document or window)?

People mixing codesnippets from various sources are still bound to get into trouble, even if your tool does everything to prevent them.

addEvent as it is now can be used fine in all kinds of projects/sites/applications, you just need to be aware when you mix it with other scripts just like you always need to be aware when you use 3rd party scripts.

Just document possible pitfalls and ways to resolve them or work around them. I don't think there is a perfect solution to this particular problem.

110 Posted by Bruce Hodo on 21 November 2005 | Permalink

For most of this year, I've been using a javascript library written by Caio Chassot that I found in A List Apart. The library not only allows adding and deleting events, it also allows easy (and standards- compliant) implementation of pop-ups, class switching, and more. And it works on element id's, element classes, and arrays of objects, automatically! It has saved me DAYS in js coding and debugging and works in IE and Mozilla.

You can find more at http://www.v2studio.com/k/code/. The article in A List Apart is at http://www.alistapart.com/articles/popuplinks/ but be aware that the code from Caio's own page is a more efficient update of the code in the ALA article.

It may not be a contest winner, but it works for me!

111 Posted by Hallvord R. M. Steen on 28 March 2007 | Permalink

I've found an issue with John Resig's function: it relies on decompiling function objects. This can be a problem with some implementations. See this blog post for suggestions on fixing it:

http://my.opera.com/hallvors/blog/2007/03/28/a-problem-with-john-resigs-addevent