[PATCH] Event enhancements - IE scope correction and event object normalization

Funny this being closed - Event module doesn't have tests in the first place.

Andrew, this is a sweet idea - but the patch is kind of clumsy. It really doesn't need element.handlers, Event._IEFixes, Function.prototype._fixScope. This is bloat to the framework.

more elegant

OK I've added my patch based on Andrew's great work.

• automatic scope correction! try the test for it in IE
• normalizes the event object in IE, bringing it closer to W3C DOM
• Event.observe can now take an array as a first parameter

Try it, play with it. Ideas how to make unit tests or improve functional ones welcome :)

Replying to mislav: I like it. Sorry about that first patch -- I was, umm, drunk. Yeah, that's it.

Replying to savetheclocktower:

I like it.

Thanks. It can even be further improved, but I'm worrying about reducing performance if I did some of the stuff I thought about.

And, BTW, read "but it in [dir]..." as "put it in [dir]..." :) I don't even know if I should call them functional tests, but they are certainly useful. I'll investigate if events can be simulated (programatically triggered) in a cross-browser manner.

Replying to mislav:

And, BTW, read "but it in [dir]..." as "put it in [dir]..." :) I don't even know if I should call them functional tests, but they are certainly useful. I'll investigate if events can be simulated (programatically triggered) in a cross-browser manner.

Afaik, no. But we can do mock object for this, simulating what the "ideal" browser would call, and even simulating if we're on IE, Safari, Firefox or whatever. That's something we could need for other cross-browser stuff, too.

I've updated the patch and tests. Take a look at it. It also fixes #2706 (inspect) and #6497 (bindAsEventListener).

It now uses the sandboxing technique to make private methods (_observeAndCache, _extend) and properties (observers array) really private.

If someone tested in Safari that'd be neat!

updated & polished with more love

I have updated the patch again, adding a killer bonus feature. The module has been given some sweet closure and OOP love. Sneak preview:

observe: function(element, type, observer, useCapture) {
  if (applyToList(arguments)) return;
  
  new Observer(element, type, observer, useCapture).add();
},
stopObserving: function(element, type, observer, useCapture) {
  if (applyToList(arguments)) return;
  
  new Observer(element, type, observer, useCapture).remove();
}

Features so far

• IE scope correction
• IE event object normalization
• IE doesn't error out on Object.inspect(evt) anymore (#2706)
• bindAsEventListener fix from #6497
• you can call observe and stopObserving with a collection (array) of elements! For instance you can pass this as an element: $A($('list').childNodes)
• killer feature: if you omit the handler function from stopObserving(), it removes all the observers of given type from the element, or all the observers of any type if type isn't specified! Wow!! For instance, do this to clean out everything from an element: $('foo').stopObserving()

Tests

Every feature has tests. Download the updated event.html. I ran them on FF2, Opera 8.5 and IE6. I tested for memleaks with IE Sieve. More tests to come.

This is proceeding pretty nicely. Some thoughts:

* Move UA detection to a higher-up level, so it's useful elsewhere (there are a few UA tests here and there). On some sites, when I need it, I use an Engine object that can be queried by doing if(Engine.isMSIE) ... and so on and so forth. * Maybe we can find a way to solve #4957 * Add events unit testing (possibly with mock objects, see my comment above)

Anyway, great stuff coming here. :)

Replying to madrobby:

Move UA detection to a higher-up level, so it's useful elsewhere

I was thinking about that... but where? And wouldn't people start abusing that detection if it would be a part of the framework?

Maybe we can find a way to solve #4957

I don't have OS X to test, but I can try and others (you?) can test for me

Anyway, great stuff coming here. :)

Thanks!

Replying to madrobby:

Move UA detection to a higher-up level, so it's useful elsewhere

I just submitted #6800 to address this. Let me know what you think.