Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TIMOB-7434 In rhino, be wary of trying to call undefined/null event call... #1286

Merged
merged 2 commits into from Jan 31, 2012
Merged

Conversation

billdawson
Copy link
Contributor

...backs.

http://jira.appcelerator.org/browse/TIMOB-7434 <--- see my RHINO-related comments therein.

DON'T close the ticket based on this PR, because it's a double ticket and something may need to be done for V8 as well.

As I say in the ticket, I wasn't able to reproduce in Rhino exactly what Eric saw. Maybe we can get Eric to hammer away again at it?

@rusticphilosopher
Copy link
Contributor

Don't merge this change until further discussion / clarification

@rusticphilosopher
Copy link
Contributor

CR Accepted, FR in progress

@rusticphilosopher
Copy link
Contributor

Code reviewed and functional test passed on V8 and Rhino. Should be noted that when exiting KS under V8, a NDK crash occurs on both master and with this PR. The crash is non visible and does not seem to impact the relaunch of the app. As the crash occurs on the master version, I feel that this is related to the upcoming memory fix and should not stand in the way of this fix.

In regards to this fix, in theory this change should address both V8 and Rhino if both runtimes suffer from the same problem. As the issue on V8 has never been able to be reproduced, this cannot be confirmed. It should also be noted that this change addresses the calling of a null handler in JS but not the root problem of events continuing to fire (which should be address with a larger change to event registration and removal).

This change may also be made slightly obsolete with the addition of proper cleanup associated with the memory fix but in general, checking for the null handler is not a bad thing in either case so I am accepting this.

Accepted

rusticphilosopher pushed a commit that referenced this pull request Jan 31, 2012
TIMOB-7434 In rhino, be wary of trying to call undefined/null event call...
@rusticphilosopher rusticphilosopher merged commit d7417b8 into tidev:master Jan 31, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants