TIMOB-6087 Android Facebook: Make sure dialogs use current activity #782
Conversation
… from FacebookModule (rather than a view proxy). Make sure dialog() runs on UI thread.
|
|
||
| final Activity activity = TiApplication.getInstance().getCurrentActivity(); | ||
| if (activity == null) { | ||
| // There should always be a current activity. |
There was a problem hiding this comment.
This actually may not be the case -- it's possible when transitioning between activities for the current activity to be null (we've seen this in other code as well). We may not be running into this currently because the KS test opens the dialog in response to a button click event, but if the dialog() method was called after calling another heavy-weight window.open(), you might run into this race condition.
For the purposes of this code, it might be better to use TiUIHelper.waitForCurrentActivity since these methods are async.. thoughts?
…FB dialog. Use our waitForCurrentActivity utility method to be safe
|
This is better but it looks like we're using getCurrentActivity() all over the facebook module, and it could have unintended consequences... Logout apparently needs to be called on the same "Context" object that "login" is called on, according the facebook javadoc here: Our current LoginButton seems to take care of this by using the Button's creation Context, but we should probably be caching the Context we login with somehow to make sure this is consistent (theoretically this won't actually do anything if the user opens a new Activity before logging out) |
…entActivity. Try hard to have logout() use same context as authorize().
|
Comments appreciated :). Functional and code review passed, request accepted |
|
Pull Accepted: Tests included login and full run through Facebook tests. Used both force and do not force dialog as well as quitting and relaunching tests 3 times each device. |
|
Code reviewed. Request Accepted |
TIMOB-6087 Android Facebook: Make sure dialogs use current activity
http://jira.appcelerator.org/browse/TIMOB-6087
Test case is the failcase described in the ticket, plus keep these points in mind: Make sure you can do test mentioned in the ticket multiple times for each runtime (v8/rhino). Also try backing out of KitchenSink and going back into it, and doing that test again.