I have found it very handy while building widgets to be able to
connect custom signals on DOM elements to arbitrary handlers.
Unfortunately "signal( elem, signalName, param1, param2, ...)" didn't
work as expected.
I traced this down to the isDOM check in the connect function.
Signal.connect is assuming that if the src object has an
addEventListener or an attachEvent attribute then the handler wants to
be passed the MochiKit.Event object. For my handlers bound to custom
signals this is not the case, it wouldn't really matter except that I
also like to pass arguments via MochiKit.signal to these handlers.
My fix is based on the documented assumption that MochiKit assumes an
`on' prefix for all DOM event signals. I assume that MochiKit's goal
here is to homogonise the different events between browsers. For
example, MochiKit assumes `on...' and then removes it for browsers
that want to have `click' connected instead of 'onclick', etc.
Thus if in Signal.connect the isDOM calculation is strengthened to
make this assumption explicit then we get the ability to connect
custom signals to DOM elements and have Signal.signal pass any extra
parameters correctly. Eg;
var isDOM = (src.addEventListener || src.attachEvent) &&
(sig.substr(0,2) === 'on');
The only caveat is that custom signals cannot start with 'on' which I
think is acceptable as that is already a documented assumption by
MochiKit.
This allows for the following code to work;
var myHandler = function( one, two, three ) { log( one, two,
three ); };
connect( 'my-div', 'my-custom-event', myHandler );
signal ( 'my-div', 'my-custom-event', 1, 2, 3 );
this will log "1 2 3".
Below is a diff against SVN Revision 1368 including changes to the
test suite.
I have tested against FF2 and IE7 which is all I have access to, I
would appreciate it if others with access to other browsers could run
the test too.
- var isDOM = !!(src.addEventListener || src.attachEvent);
+ // The documentation states that DOM signals all start with
+ // 'on', so make this an explicit check, which has the bonus
+ // of allowing us to connect custom signals to elements and
+ // have our handler get called with the params passed to
+ // signal instead of the wrapped native Event object.
+ var isDOM = (src.addEventListener || src.attachEvent) &&
(sig.substr(0,2) === 'on');
+
if (isDOM && (sig === "onmouseenter" || sig ===
"onmouseleave")
&& !self._browserAlreadyHasMouseEnterAndLeave()) {
var listener = self._mouseEnterListener(src,
sig.substr(2), func, obj);
@@ -626,19 +632,20 @@
var listener = self._listener(src, sig, func, obj,
isDOM);
}
-
+
ident = connect('submit', 'onmousedown', eventTest);
triggerMouseEvent('submit', 'mousedown', false);
t.is(i, 3, 'Connecting an event to an HTML object and firing
a synthetic event');
@@ -107,39 +107,61 @@
triggerMouseEvent('submit', 'mousedown', false);
t.is(i, 3, 'Disconnecting an event to an HTML object and
firing a synthetic event');
+ // Custom Signals bound to thunks on DOM Elements do not get
+ // wrapped in an event object, when they shouldn't.
+ var customSubmitMethodParam1 = null;
+ var customSubmitMethodParam2 = null;
+ var customSubmitMethod = function( param1, param2 ){
+ // console.log( 'customSubmitMethod called with this bound
to:', this, 'params are', param1, param2 );
+ customSubmitMethodParam1 = param1;
+ customSubmitMethodParam2 = param2;
+ };
+ var customDiv = getElement('custom-signal-test');
+ update( customDiv, { dynamicMethod : customSubmitMethod } );
+ bindMethods( customDiv );
+ ident = connect( 'custom-signal-test', 'custom-event',
customDiv.dynamicMethod);
+ signal( 'custom-signal-test', 'custom-event', 'PARAM1',
'PARAM2' );
-
- }
+ t.ok( (customSubmitMethodParam1 === 'PARAM1' &&
+ customSubmitMethodParam2 === 'PARAM2'), 'checking that
methods bound to elements are correctly signaled' );
+
+ disconnect(ident);
+ ident = connect('custom-signal-test', 'custom-event',
customSubmitMethod);
+
...
I've also been bitten by this same issue, but didn't dig deeper into it before. Great analysis and nice with a patched solution!
Looking at the code in MochiKit.Signal, I have two comments:
1. connect(src, signal, dst, func) uses late binding to dst[func] for DOM signals, but uses early binding in other cases (with MochiKit.Base.bind). I think we should use late binding at all times instead.
2. I don't agree with the solution to the issue that signal() doesn't fulfill its contract (as per the documentation). Rather, I think a better solution would be to modify signal() to call ident.objOrFunc and ident.objOrFunc directly instead of using ident.listener. Then it would be fully up to the caller of signal() which arguments are to be specified and in which order. Using the "on" prefix seems a bit like an ad-hoc work-around to me.
Cheers,
/Per
On Mon, May 5, 2008 at 7:44 AM, simon.cus...@gmail.com
> I have found it very handy while building widgets to be able to > connect custom signals on DOM elements to arbitrary handlers. > Unfortunately "signal( elem, signalName, param1, param2, ...)" didn't > work as expected.
> I traced this down to the isDOM check in the connect function.
> Signal.connect is assuming that if the src object has an > addEventListener or an attachEvent attribute then the handler wants to > be passed the MochiKit.Event object. For my handlers bound to custom > signals this is not the case, it wouldn't really matter except that I > also like to pass arguments via MochiKit.signal to these handlers.
> My fix is based on the documented assumption that MochiKit assumes an > `on' prefix for all DOM event signals. I assume that MochiKit's goal > here is to homogonise the different events between browsers. For > example, MochiKit assumes `on...' and then removes it for browsers > that want to have `click' connected instead of 'onclick', etc.
> Thus if in Signal.connect the isDOM calculation is strengthened to > make this assumption explicit then we get the ability to connect > custom signals to DOM elements and have Signal.signal pass any extra > parameters correctly. Eg;
> The only caveat is that custom signals cannot start with 'on' which I > think is acceptable as that is already a documented assumption by > MochiKit.
> This allows for the following code to work;
> var myHandler = function( one, two, three ) { log( one, two, > three ); }; > connect( 'my-div', 'my-custom-event', myHandler ); > signal ( 'my-div', 'my-custom-event', 1, 2, 3 );
> this will log "1 2 3".
> Below is a diff against SVN Revision 1368 including changes to the > test suite.
> I have tested against FF2 and IE7 which is all I have access to, I > would appreciate it if others with access to other browsers could run > the test too.
> - var isDOM = !!(src.addEventListener || src.attachEvent); > + // The documentation states that DOM signals all start with > + // 'on', so make this an explicit check, which has the bonus > + // of allowing us to connect custom signals to elements and > + // have our handler get called with the params passed to > + // signal instead of the wrapped native Event object. > + var isDOM = (src.addEventListener || src.attachEvent) && > (sig.substr(0,2) === 'on'); > + > if (isDOM && (sig === "onmouseenter" || sig === > "onmouseleave") > && !self._browserAlreadyHasMouseEnterAndLeave()) { > var listener = self._mouseEnterListener(src, > sig.substr(2), func, obj); > @@ -626,19 +632,20 @@ > var listener = self._listener(src, sig, func, obj, > isDOM); > }
