incubator-callback-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Patrick Mueller (Resolved) (JIRA)" <j...@apache.org>
Subject [jira] [Resolved] (CB-82) removeEventListener broken
Date Thu, 01 Dec 2011 20:50:39 GMT

     [ https://issues.apache.org/jira/browse/CB-82?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Patrick Mueller resolved CB-82.
-------------------------------

    Resolution: Fixed

fixed in version 1.6.1
                
> removeEventListener broken
> --------------------------
>
>                 Key: CB-82
>                 URL: https://issues.apache.org/jira/browse/CB-82
>             Project: Apache Callback
>          Issue Type: Bug
>          Components: weinre
>            Reporter: Patrick Mueller
>            Assignee: Patrick Mueller
>            Priority: Critical
>
> from: https://github.com/callback/callback-weinre/issues/2
> *pgreyson opened this issue November 07, 2011*
> commit: [041490b|https://github.com/callback/callback-weinre/commit/041490bdae1121de0328f2933836ab27a8e0beba]
> With the code here:
> http://pastebin.com/qHRkh1FF
> The click handler still executes after removeEventListener is called.
> Modifying ExceptionalCallbacks.coffee as follows fixes the problem (but disables debugging
of event callbacks)
> addHookEventListener HookSites.Node_addEventListener, callSite_nodeAEL
> *pmuellr commented November 08, 2011*
> Thanks for the bug report and the sim
> ple test case. But I don't think the referenced commit has anything to do with the bug,
does it?
> *pgreyson commented November 08, 2011*
> The referenced commit is just to identify the snapshot I'm using.
> *pmuellr commented November 09, 2011*
> Took a quick look this afternoon. It appears that I'll need to override removeEventListener
- seems like a stupid oversight on my part, sorry. Won't really be able to do anything about
it till I get back from vacation on 11/14.
> In terms of the fix, first thing to try is to attach a new object to the function the
user passes in, which can be used to obtain the actual function to pass into REL. Not happy
about having to do this (extend user functions), but don't see another obvious solution.
> *pgreyson commented November 09, 2011*
> I think that's right. I wonder though whether this is weinre trying to do too much. I
love having the remote javascript console and the DOM inspector. But there's a limit to what
you can do without having real debugger hooks and seeing weinre hooking addEventListener gave
me pause because it's changing program behavior. I'd rather do that kind of thing via my own
wrapper functions which will be on whether weinre is there or not.
> *pmuellr commented November 09, 2011*
> absolutely - it's a bit of a dilemma - folks actually expect that weinre can do this
sort of thing, and so it's of course nice to be able to provide the functionality. Unfortunately,
it's mucking with your runtime more than you would want it to, at least in this case. I have
less hesitation adding properties to DOM object, for instance.
> And in most cases, the things weinre does probably doesn't cause any harm.
> Still, kinda tending to agree with you that perhaps this should be moved to a separate
diagnostic library - actually, I prototyped it a bit here - https://github.com/pmuellr/log-callback-error
- and then folks can just add that if they need it.
> Another option would to make it - somehow - optional.
> Got some time to noodle about it before next week ... :-)
> *pmuellr commented November 14, 2011*
> So, my current thinking is to:
> * make the event handling callback checking code optional
> * turn it on by default
> One of the requirements will then be a way of toggling it off. Smells like a requirement
for a "settings" page. Don't have one right now, but this raises a bunch of other issues -
are "settings" saved on the server, in the debug client, the debug target, or (oh noes) some
combination of the three.
> Pretend like we have a sane story for options :-) Do you think enabling this by default
is ok?
> This is veering off-topic at this point, prolly better to create a new issue to track
it, and use this one to get the actual problem fixed ASAP.
> *pgreyson commented November 14, 2011*
> My preference would be to have it off by default. But that's because I would never turn
it on because I think that exception handling should be part of the application code.
> For storing settings, localStorage on the debug target could be a good option.
> *pmuellr commented November 14, 2011*
> OTOH, I've had feedback from people who assume that weinre provides this type of support;
and are surprised that it didn't (in earlier versions).
> Kinda depends on the experience level of the developer I guess. And I happen to think
there are far fewer experts, such as yourself, than there are non-experts, using weinre.
> *pgreyson commented November 14, 2011*
> Well you did ask :)
> I can understand people wanting to have Weinre act like a full debugging console and
I guess catching exceptions in the event dispatcher is a reasonable second best.
> On the other hand, I think it could be very confusing. e.g., if you keep the Weinre server
process running but don't have the weinre console up then Weinre could be catching and recovering
from exceptions that will kill your app when you turn the server off.
> *pmuellr commented November 14, 2011*
> weinre should not be "recovering" from exceptions at all. At the end, I [rethrow the
exception|https://github.com/callback/callback-weinre/blob/master/weinre.web/modules/weinre/target/ExceptionalCallbacks.coffee#L47].
Which itself is not great - this re-roots the origin of the exception to my exception handling
code, instead of the real code. But since the runtime doesn't really do anything interesting,
right now, with the origin of the exception, it shouldn't be hurting anything.
> All the more reason to make this optional, BTW. Never know, some platform may actually
add decent callback exception handling to the runtime at any point in time, and folks would
want to disable this without waiting for a new weinre release.
> *pgreyson commented November 14, 2011*
> Oh sorry I didn't notice that you re-throw. That all seems fine then.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message