Hi all,
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.
Regards, Simon Cusack.
diff -r ae903235ba94 MochiKit/Signal.js
--- a/MochiKit/Signal.js Mon May 05 13:37:27 2008 +1000
+++ b/MochiKit/Signal.js Mon May 05 15:39:44 2008 +1000
@@ -63,7 +63,7 @@
str += '}';
}
}
- if (this.type() == 'mouseover' || this.type() == 'mouseout'
||
+ if (this.type() == 'mouseover' || this.type() == 'mouseout'
||
this.type() == 'mouseenter' || this.type() ==
'mouseleave') {
str += ', relatedTarget(): ' +
repr(this.relatedTarget());
}
@@ -516,10 +516,10 @@
if (sig === 'onload' || sig === 'onunload') {
return function (nativeEvent) {
obj[func].apply(obj, [new E(src, nativeEvent)]);
-
+
var ident = new MochiKit.Signal.Ident({
source: src, signal: sig, objOrFunc: obj,
funcOrStr: func});
-
+
MochiKit.Signal._disconnect(ident);
};
} else {
@@ -531,10 +531,10 @@
if (sig === 'onload' || sig === 'onunload') {
return function (nativeEvent) {
func.apply(obj, [new E(src, nativeEvent)]);
-
+
var ident = new MochiKit.Signal.Ident({
source: src, signal: sig, objOrFunc: func});
-
+
MochiKit.Signal._disconnect(ident);
};
} else {
@@ -613,7 +613,13 @@
obj = src;
}
- 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);
}
- if (src.addEventListener) {
- src.addEventListener(sig.substr(2), listener, false);
- } else if (src.attachEvent) {
- src.attachEvent(sig, listener); // useCapture unsupported
+ if (isDOM) {
+ if (src.addEventListener) {
+ src.addEventListener(sig.substr(2), listener, false);
+ } else if (src.attachEvent) {
+ src.attachEvent(sig, listener); // useCapture
unsupported
+ }
}
-
var ident = new MochiKit.Signal.Ident({
- source: src,
- signal: sig,
- listener: listener,
- isDOM: isDOM,
- objOrFunc: objOrFunc,
- funcOrStr: funcOrStr,
+ source: src,
+ signal: sig,
+ listener: listener,
+ isDOM: isDOM,
+ objOrFunc: objOrFunc,
+ funcOrStr: funcOrStr,
connected: true
});
self._observers.push(ident);
diff -r ae903235ba94 tests/test_MochiKit-Signal.html
--- a/tests/test_MochiKit-Signal.html Mon May 05 13:37:27 2008 +1000
+++ b/tests/test_MochiKit-Signal.html Mon May 05 15:39:44 2008 +1000
@@ -4,26 +4,29 @@
<script type="text/javascript" src="../MochiKit/Iter.js"></
script>
<script type="text/javascript" src="../MochiKit/DOM.js"></script>
<script type="text/javascript" src="../MochiKit/Style.js"></
script>
- <script type="text/javascript" src="../MochiKit/Signal.js"></
script>
- <script type="text/javascript" src="../MochiKit/Logging.js"></
script>
- <script type="text/javascript" src="SimpleTest/SimpleTest.js"></
script>
+ <script type="text/javascript" src="../MochiKit/Signal.js"></
script>
+ <script type="text/javascript" src="../MochiKit/Logging.js"></
script>
+ <script type="text/javascript" src="SimpleTest/SimpleTest.js"></
script>
<link rel="stylesheet" type="text/css" href="SimpleTest/
test.css">
</head>
<body>
Please ignore this button: <input type="submit" id="submit" /><br />
+Please ignore this div: <br />
+<div id="custom-signal-test" />
+<br />
<pre id="test">
<script type="text/javascript" src="test_Signal.js"></script>
<script type="text/javascript">
try {
-
+
tests.test_Signal({ok:ok, is:is});
ok(true, "test suite finished!");
-
+
} catch (err) {
-
+
var s = "test suite failure!\n";
var o = {};
var k = null;
diff -r ae903235ba94 tests/test_Signal.js
--- a/tests/test_Signal.js Mon May 05 13:37:27 2008 +1000
+++ b/tests/test_Signal.js Mon May 05 15:39:44 2008 +1000
@@ -3,7 +3,7 @@
if (typeof(tests) == 'undefined') { tests = {}; }
tests.test_Signal = function (t) {
-
+
var submit = MochiKit.DOM.getElement('submit');
var ident = null;
var i = 0;
@@ -14,7 +14,7 @@
i += this.someVar;
}
};
-
+
var aObject = {};
aObject.aMethod = function() {
t.ok(this === aObject, "aMethod should have 'this' as
aObject");
@@ -37,19 +37,19 @@
disconnect(ident);
submit.click();
- t.is(i, 2, '...and then disconnected');
-
- if (MochiKit.DOM.getElement('submit').fireEvent ||
- (document.createEvent &&
+ t.is(i, 2, '...and then disconnected');
+
+ if (MochiKit.DOM.getElement('submit').fireEvent ||
+ (document.createEvent &&
typeof(document.createEvent('MouseEvents').initMouseEvent) ==
'function')) {
-
- /*
-
- Adapted from:
+
+ /*
+
+ Adapted from:
http://www.devdaily.com/java/jwarehouse/jforum/tests/selenium/javascr...
License: Apache
Copyright: Copyright 2004 ThoughtWorks, Inc
-
+
*/
var triggerMouseEvent = function(element, eventType,
canBubble) {
element = MochiKit.DOM.getElement(element);
@@ -98,7 +98,7 @@
t.ok((typeof(e.key()) === 'undefined'), 'checking that
key() is undefined');
};
-
+
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);
+ signal( 'custom-signal-test', 'custom-event', 'PARAM3',
'PARAM4' );
+ t.ok( (customSubmitMethodParam1 === 'PARAM3' &&
+ customSubmitMethodParam2 === 'PARAM4'), 'checking that
functions are correctly signaled' );
+
+ }
// non-DOM tests
var hasNoSignals = {};
-
+
var hasSignals = {someVar: 1};
var i = 0;
-
+
var aFunction = function() {
i++;
if (typeof(this.someVar) != 'undefined') {
i += this.someVar;
}
};
-
+
var bFunction = function(someArg, someOtherArg) {
i += someArg + someOtherArg;
};
-
+
var aObject = {};
aObject.aMethod = function() {
i++;
};
-
+
aObject.bMethod = function() {
i++;
};
-
+
var bObject = {};
bObject.bMethod = function() {
i++;
@@ -235,13 +257,13 @@
t.is(i, 0, 'Exception raised, signal should not have fired');
i = 0;
-
+
connect(hasSignals, 'signalOne', aObject, 'aMethod');
connect(hasSignals, 'signalOne', aObject, 'bMethod');
signal(hasSignals, 'signalOne');
t.is(i, 2, 'Connecting one signal to two slots in one object');
i = 0;
-
+
disconnect(hasSignals, 'signalOne', aObject, 'aMethod');
disconnect(hasSignals, 'signalOne', aObject, 'bMethod');
signal(hasSignals, 'signalOne');
@@ -260,7 +282,7 @@
signal(hasSignals, 'signalOne');
t.is(i, 0, 'Disconnecting one signal from two slots in two
objects');
i = 0;
-
+
try {
connect(nothing, 'signalOne', aObject, 'aMethod');
@@ -276,8 +298,8 @@
} catch (e) {
t.ok(true, 'An exception was raised when disconnecting
undefined');
}
-
-
+
+
try {
connect(hasSignals, 'signalOne', nothing);
signal(hasSignals, 'signalOne');
@@ -292,8 +314,8 @@
} catch (e) {
t.ok(true, 'An exception was raised when disconnecting an
undefined function');
}
-
-
+
+
try {
connect(hasSignals, 'signalOne', aObject, aObject.nothing);
signal(hasSignals, 'signalOne');
@@ -301,7 +323,7 @@
} catch (e) {
t.ok(true, 'An exception was raised when connecting an
undefined method');
}
-
+
try {
connect(hasSignals, 'signalOne', aObject, 'nothing');
signal(hasSignals, 'signalOne');
@@ -359,13 +381,13 @@
signal(hasSignals, 'signalTwo');
t.is(i, 0, 'disconnectAll works with implicit signals');
i = 0;
-
+
var toggle = function() {
disconnectAll(hasSignals, 'signalOne');
connect(hasSignals, 'signalOne', aFunction);
i++;
};
-
+
connect(hasSignals, 'signalOne', aFunction);
connect(hasSignals, 'signalTwo', function() { i++; });
connect(hasSignals, 'signalTwo', toggle);
@@ -413,7 +435,7 @@
signal(hasSignals, 'signalTwo');
t.is(testObj.countOne, 0, 'disconnectAllTo obj+str');
t.is(testObj.countTwo, 1, 'disconnectAllTo obj+str');
-
+
has__Connect = {
count: 0,
__connect__: function (ident) {
@@ -444,5 +466,5 @@
connect(src, 'signal', sink.f);
signal(src, 'signal', 'worked');
t.is(sink.ev, 'worked', 'custom signal does not re-bind
methods');
-
+
};
