Review of fix to eventFirer.removeListener()
colinbdclark at gmail.com
Wed Jun 16 21:12:04 UTC 2010
I noticed this issue when I looked at the patch as well. The extra type check is just going to slow us down.
My first instinct is to remove the type checks in the else case altogether and go for a duckier solution. The key thing we're interested in here is the presence of the special $$guid property which identifies the listener to the event system. If it's there, remove the listener. If it's not, don't.
On 2010-06-16, at 4:38 PM, Anastasia Cheetham wrote:
> Laurel, I've just had a look at the patch you attached to
> Your test is nice and simple, and clearly illustrates that removeListener() does not actually remove listeners.
> I'm wondering about the fix: The problem is that removeListener() is only trying to remove listeners of type "object" instead of type "function." Your patch changes the condition to look for *both* "object" and "function." I'm not sure whether or not a listener of type "object" would even be reasonable, but my understanding of the event system is not quite as thorough as it could be.
> I'm wondering if anyone else has a moment to look at the patch, and comment on whether my suspicion (that removeListener() need only look for "function", and not "object) is correct or not?
Technical Lead, Fluid Project
More information about the fluid-work