This project is archived and is in readonly mode.

#571 ✓wontfix
Michal

Patch for bindWithEvent: now can 'make space' for multiple events

Reported by Michal | January 25th, 2009 @ 01:33 AM | in 2.0 (closed)

I have occasionally come across the issue where multiple parameters are passed with an event (say onSuccess from Request.HTML), and I would like to use bindWithEvent to create a binding, where all of the passed parameters from the event are passed to the bound function.

If I have no optional 'args' parameter passed to bindWithEvent, then the current code seems to work fine (even though I don't think this case is documented). However, if there are optional 'args', then only the first parameter passed with the event is passed to the bound function.

The attached proposed patch to Function.js rectifies this issue. It gives 'bindWithEvent' a third optional parameter 'numEvents': this specifies how many parameters to 'make space' for in the bound function. The default is 1 if this is omitted.

I have also included extra specs that test the code.

I have also written it so that if 'numEvents' is less than the actual numbers of parameters passes, only the first 'numEvents' of them are passed to the bound function. If 'numEvents' is greater than the number of passed parameters, then the empty variables are left undefined.

The patch modifies Function.create. The original did mention the term 'window.event' . I can't for the life of me figure out what this is for, so I did not include an equivalent in the patch. Therefore there could be a case/browser that the patch does not work for.

Comments and changes to this ticket

  • Michal

    Michal January 25th, 2009 @ 09:52 AM

    About the 'window.event' issue: I'm suspecting right now it's a throwback to some day when MooTools handled events differently.

    From reading around, I think for some events IE does not pass an event object directly to its listener, but uses the global window.event object instead. However, looking at the source of Event.js and Element.Event.js, I think that MooTools already accounts for this by 'wrapping' the actual listener and (essentially) passing window.event to to the listener given to 'addEvent'. Therefore by the time the function defined by 'bindWithEvent' is actually called, there is already an event parameter passed (and it might be window.event).

    However I'm not sure: this is all at the boundary of my Javascript knowledge!

  • Michal

    Michal January 25th, 2009 @ 03:35 PM

    I've now made another patch that cleans things up a bit (the patch must be applied after the first patch above)

    • bindWithEvent does not take an extra parameter now, it acts exactly as before
    • bindWithEvents is added that does take an extra parameter, the number of events to make room for.

    Also changed is the boolean 'event' value used in the options passed to create. This is now an integer 'events' value that specified the number of events to make room for. (In my first patch above there was a sort of overlap of use between the boolean 'event' and integer 'numEvents').

    One thing I'm not sure about though: technically the bindWithEvent(s) functions aren't necessary. The extra numerical 'events' parameter could be added to the usual 'bind' as an optional parameter that defaults to 0. This means instead of:

    
    myFunc.bind(this, someArgs) 
    myFunc.bindWithEvent(this, someArgs)
    myFunc.bindWithEvents(this, someArgs, 2)
    

    we could have:

    
    myFunc.bind(this, someArgs) 
    myFunc.bind(this, someArgs, 1)
    myFunc.bind(this, someArgs, 2)
    

    I'm not sure which form is better.

  • fakedarren

    fakedarren February 8th, 2010 @ 05:03 PM

    • State changed from “new” to “open”

    Val, sorry, assigning to you as 'open' for your comments.

  • Valerio

    Valerio February 16th, 2010 @ 02:22 PM

    • State changed from “open” to “wontfix”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Shared Ticket Bins

People watching this ticket

Pages