webworks.event.isOn should take the callback as a param

[rwmtse]

@nukulb @jeffheifetz

I notice a problem with webworks.event.isOn when working with contact picker cards. I see that if I call the invokeContactPicker function multiple times (in the test app), then the callbacks don't get fired.

The reason is because of how we use this code when we register the callback:

if (!window.webworks.event.isOn(_contactDoneEventId, onDone)) {
    window.webworks.event.once(_ID, _contactDoneEventId, onDone);
}

When the invoke picker function is called multiple times, the callback in the later calls simply are not registered in the handlers map.

I propose adding the callback to the isOn function, so that it works like this:

isOn: function (name, callback) {
    if (callback) {
        return _handlers[name] && _handlers[name].indexOf(callback) !== -1;
    } else {
        return !!_handlers[name]; // this statement is the current implementation
    }
}

Also, I think whenever we call webworks.event.once(), our code has to handle removing the callback from the handler map. I don't think we ever do that in all of our APIs. --> Sorry I take this back, I see that it is actually done in webworks.event.trigger(), for triggered once callbacks, we are ok, it's just that for un-triggered once callbacks, we need a way to clean up.

[rwmtse]

@nukulb Once I changed isOn() to the implementation above, the weird problems in my contact picker functional test is gone.

[nukulb]

Where all are we using isOn?

From: Rosa Tse To: blackberry/BB10-WebWorks-Framework To: Nukul Bhasin Reply To: blackberry/BB10-WebWorks-Framework Re: [BB10-WebWorks-Framework] webworks.event.isOn should take the callback as a param (#407) 2012-11-30 6:18:28 PM

@nukulbhttps://github.com/nukulb Once I changed isOn() to the implementation above, the weird problems in my contact picker functional test is gone.

\u2014 Reply to this email directly or view it on GitHubhttps://github.com/blackberry/BB10-WebWorks-Framework/issues/407#issuecomment-10908076.

---

This transmission (including any attachments) may contain confidential information, privileged material (including material protected by the solicitor-client or other applicable privileges), or constitute non-public information. Any use of this information by anyone other than the intended recipient is prohibited. If you have received this transmission in error, please immediately reply to the sender and delete this information from your system. Use, dissemination, distribution, or reproduction of this transmission by unintended recipients is not authorized and may be unlawful.

[jeffheifetz]

I think the method could be useful, but likely the correct solution if you want multiple rs active is to use unique event names

[rwmtse]

@jeffheifetz you are correct, I talked to @pagey and found the correct way to implement the callbacks.

There are 3 callbacks in my api: onDone, onCancel, onInvoke.

onInvoke will always fire, it should use its own event id Only one of onDone and onCancel will get fired, in order for the callback to clean up correctly, done & cancel should be registered under one common event id.

This fixes the problems I saw in my functional tests.

All cards implementation currently use separate event ids for each callback, those code needs to be revisited.

[nukulb]

this will create many bugs, any chance any of you guys can fix this soon so that we can include the bug fix in the gold release

[bryanhiggins]

@nukulb I will fix this in the other cards

[nukulb]

Bryan, can you come on IRC, I will explain what needs to be done so that we can get a better solution.

From: bryanhiggins notifications@github.com<mailto:notifications@github.com> Reply-To: blackberry/BB10-WebWorks-Framework reply@reply.github.com<mailto:reply@reply.github.com> Date: Sat, 1 Dec 2012 08:31:34 -0800 To: blackberry/BB10-WebWorks-Framework BB10-WebWorks-Framework@noreply.github.com<mailto:BB10-WebWorks-Framework@noreply.github.com> Cc: Nukul Bhasin nbhasin@rim.com<mailto:nbhasin@rim.com> Subject: Re: [BB10-WebWorks-Framework] webworks.event.isOn should take the callback as a param (#407)

@nukulbhttps://github.com/nukulb I will fix this in the other cards

— Reply to this email directly or view it on GitHubhttps://github.com/blackberry/BB10-WebWorks-Framework/issues/407#issuecomment-10918895.

---

This transmission (including any attachments) may contain confidential information, privileged material (including material protected by the solicitor-client or other applicable privileges), or constitute non-public information. Any use of this information by anyone other than the intended recipient is prohibited. If you have received this transmission in error, please immediately reply to the sender and delete this information from your system. Use, dissemination, distribution, or reproduction of this transmission by unintended recipients is not authorized and may be unlawful.

[bryanhiggins]

Updated the existing cards to use the single event approach: https://github.com/blackberry-webworks/BB10-WebWorks-Framework/pull/308

However, I can think of one corner case in which the events still won't be removed:

If the webplatform doesn't call done or cancel even though the card has closed. In most places we're ok, but I noticed with camera it could be possible if the reason doesn't match anything we're checking.

While keeping the handler around is undesirable, the big problem is that the isOn returns true the next time we try to open that same card and the new event handler doesn't get added. To be honest, I don't really understand why we do this at all. We should probably just add the event and not worry if it has already been added.