BTW. A patch for mouse scroll events in MochiKit.Signal would be a
great contribution! :-)
Cheers,
/Per
On Wed, May 14, 2008 at 9:03 AM, [EMAIL PROTECTED]
<[EMAIL PROTECTED]> wrote:
>
> Hi Per,
>
> Thanks for the feedback.
>
> I am unsure how making it late bound helps, doesn't that just shift
> the problem? How would that behave in the following two cases;
>
> 1. A request to connect "onclick" occurs.
>
> Does the listener early bind a function to the native event onclick
> which then calls the clients function late bound? If so what
> advantage does this have?
>
> 2. A request to connect(someDomElement, "my-custom-signal",
> myHandler); occurs.
>
> When signal(someDomElement, "my-custom-signal", "param1", "param2")
> occurs how does the late binding know whether to try and wrap a
> native event to pass to myHandler or whether to pass the arguments
> passed to signal? Wont signal() still need to look at the signal
> name to determine if it is a DOM event handler or not?
>
> My original point about the documentation was not so much that I
> thought the contract wasn't being fullfilled but rather that in order
> to implement cross browser DOM event handling MochiKit had already
> decided to always expect the "on" prefix and to remove it for those
> browsers that don't want it. (MochiKit could have gone the other way
> and never accepted the "on" when registering events but then it would
> have to add it for those browsers that do want it).
>
> So seeing as MochiKit has already made the decision that all DOM Event
> signals must start with "on" then we could take advantage of this
> documented expectation to allow handling for custom signals on DOM
> Elements without breaking any backwards compatibility.
>
> On a side note, I am currently working on a patch for handling mouse
> scroll events in a uniform way. Different browsers use completely
> different names for these events I was planning on early binding
> browser specific handlers that normalise the values, etc so I am keen
> to solve this problem so that my next patch wont conflict.
>
> Regards, Simon.
>
> On May 6, 6:00 pm, "Per Cederberg" <[EMAIL PROTECTED]> wrote:
>> 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, [EMAIL PROTECTED]
>>
>> <[EMAIL PROTECTED]> wrote:
>>
>> > 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
>>
>> ...
>>
>> read more ยป
>
> >
>
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"MochiKit" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/mochikit?hl=en
-~----------~----~----~----~------~----~------~--~---
