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.

Sign up for GitHub

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

Jump to bottom

TIMOB-5994 - Facebook #722

Merged
merged 30 commits into from
Nov 21, 2011
Merged

TIMOB-5994 - Facebook #722

Changes from 6 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
7a9748c
[TIMOB-5994] Integrated Facebook and hooked up authentication.
nebrius Nov 14, 2011
1170111
[TIMOB-5994] Cleaned up authorization and implemented requests.
nebrius Nov 15, 2011
b81ab59
[TIMOB-5994] Implemented Facebook dialogs.
nebrius Nov 15, 2011
fbdd6de
Merge branch 'timob-5994' of github.com:bryan-m-hughes/titanium_mobil…
nebrius Nov 16, 2011
c1cdd80
[TIMOB-5994] Tweaked the output of the Facebook request methods to ma…
nebrius Nov 17, 2011
59e8079
Merge branch 'master' of git://github.com/appcelerator/titanium_mobil…
nebrius Nov 17, 2011
3b64152
[TIMOB-5994] Implemented some code cleanup suggestions.
nebrius Nov 17, 2011
ec249d7
[TIMOB-5994] Forgot to add some stuff on the last commit.
nebrius Nov 17, 2011
f91735c
[TIMOB-5859] Fixed bug with evaluating string-based modules.
cb1kenobi Nov 17, 2011
443c61e
Merge branch 'timob-5994' of github.com:bryan-m-hughes/titanium_mobil…
cb1kenobi Nov 17, 2011
c4bc042
Merge branch 'master' of github.com:cb1kenobi/titanium_mobile into ti…
cb1kenobi Nov 18, 2011
516b395
[TIMOB-5859] Fixed issue where exports variable can get clobbered in …
cb1kenobi Nov 18, 2011
f72b309
Merge branch 'timob-5859' into timob-5994
cb1kenobi Nov 18, 2011
b0b7236
[TIMOB-5994] Added support for resuming sessions and shortened some c…
nebrius Nov 18, 2011
1d7348c
Merge branch 'timob-5994' of github.com:bryan-m-hughes/titanium_mobil…
cb1kenobi Nov 18, 2011
2a510f8
[TIMOB-5994] Bug fixes in auth, code otpimizations, and sanity checking.
nebrius Nov 18, 2011
c6374ed
Merge branch 'timob-5994' of github.com:bryan-m-hughes/titanium_mobil…
cb1kenobi Nov 18, 2011
bdd9c1c
[TIMOB-5994] Few small bug fixes and some code cleanup.
nebrius Nov 18, 2011
b34efd2
Merge branch 'timob-5994' of github.com:bryan-m-hughes/titanium_mobil…
cb1kenobi Nov 18, 2011
f587c7d
[TIMOB-5994] Code size optimizations.
nebrius Nov 18, 2011
57356e3
Merge branch 'timob-5994' of github.com:bryan-m-hughes/titanium_mobil…
cb1kenobi Nov 18, 2011
0f715e6
[TIMOB-5994] More code optimizations.
nebrius Nov 18, 2011
fbe3379
Merge branch 'timob-5994' of github.com:bryan-m-hughes/titanium_mobil…
cb1kenobi Nov 18, 2011
2d3cce4
[TIMOB-5994] Another bug fix and more code size reduction.
nebrius Nov 18, 2011
a28bc55
Merge branch 'timob-5994' of github.com:bryan-m-hughes/titanium_mobil…
cb1kenobi Nov 18, 2011
4fcd118
[TIMOB-5994] Cleaned up some of the Ti.Facebook code.
cb1kenobi Nov 18, 2011
c5ca33e
[TIMOB-5994] Rearchitected facebook so that it isn't initialized unti…
nebrius Nov 18, 2011
6c194a0
[TIMOB-5994] Reworked Facebook so that it will auto-login after the a…
nebrius Nov 20, 2011
41ec287
[TIMOB-5994] Renamed a variable to make it less confusing.
nebrius Nov 20, 2011
ecd76a9
[TIMOB-5994] Moved _loginInternal to the proper outer scope.
nebrius Nov 20, 2011
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
148 changes: 127 additions & 21 deletions mobileweb/src/Ti.Facebook/facebook.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@
set: function(val){return _expirationDate = val;}
});

var _forceDialogAuth = null;
Object.defineProperty(api, 'forceDialogAuth', {
get: function(){return _forceDialogAuth;},
set: function(val){return _forceDialogAuth = val;}
get: function(){return true;},
set: function(val){return true;}
});

var _loggedIn = null;
var _loggedIn = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to initialize this variable correctly. That's fine to default to false, but then later after Facebook's API is loaded, query it to see if you're logged in. If I log in, then reload, I can't query because it thinks I'm not logged in. However, I still have a valid session and I'm able to post status updates.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read my new comment for line 76 below...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it doesn't work that way. "Logged in" is something of a misnomer, and a bad choice of words IMO on the part of Appcelerator. It's not that the user is logged in, but rather that the application is authorized. This must be done every time because the application must have an access token to make queries.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but updating a status message does not require a token? We need things to work across reloads.

Object.defineProperty(api, 'loggedIn', {
get: function(){return _loggedIn;},
set: function(val){return _loggedIn = val;}
Expand All @@ -45,31 +44,138 @@
set: function(val){return _uid = val;}
});

// Setup the Facebook initialization callback
var _facebookInitialized = false;
var _authAfterInitialized = false;
window.fbAsyncInit = function() {
FB.init({
appId : _appid, // App ID
status : true, // check login status
cookie : true, // enable cookies to allow the server to access the session
oauth : true, // enable OAuth 2.0
xfbml : true // parse XFBML
});
_facebookInitialized = true;
if (_authAfterInitialized) {
api.authorize();
}
};

// Create the div required by Facebook
fbDiv = document.createElement('div');
fbDiv.id = 'fb-root';
document.getElementsByTagName('body')[0].appendChild(fbDiv);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to reliably do document.body.appendChild(fbDiv);


// Load the Facebook SDK Asynchronously.
var fbScriptTag, id = 'facebook-jssdk';
if (!document.getElementById(id)) {
fbScriptTag = document.createElement('script');
fbScriptTag.id = id;
fbScriptTag.async = true;
fbScriptTag.src = "//connect.facebook.net/en_US/all.js";
document.getElementsByTagName('head')[0].appendChild(fbScriptTag);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this Facebooks recommended way? The best way to ensure async script loading in all browsers is to do something like:

var s = document.getElementsByTagName("script")[0];
s.parentNode.insertBefore(fbScriptTag, s);

This is the recommend way that Google Analytics does it. It's the way Steve Souders recommends. It's the way Dojo does it. It's the way my require() loader does it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so my loader is super rad. I almost forgot that it could do this. You can use require to asynchronously load scripts regardless of domain.

require(["//connect.facebook.net/en_US/all.js"], function() {
    console.debug("Facebook is downloaded and evaluated, now check if logged in and set _loggedIn appropriately!");
})

Note, I'm using the async version by wrapping the URL in an array.

Now you can rip out that huge chunk of code. If you require() the script twice, it does nothing. Pretty cool huh?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'll get it implemented. The current code was copied from the Facebook JS SDK example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably don't need to pass in a callback to require() since FB uses window.fbAsyncInit to notify when things are loaded.

}

// Methods
api.authorize = function(){
console.debug('Method "Titanium.Facebook.authorize" is not implemented yet.');

// Sanity check
if (_appid == null) {
console.debug('App ID not set. Facebook authorization cancelled.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this should be logged as an error and probably an exception thrown since it's a programmer error, not a runtime error.

return;
}

// Check if facebook is still initializing, and if so queue the auth request
if (!_facebookInitialized) {
_authAfterInitialized = true;
return;
}

// Authorize
FB.login(function(response) {
var undef;
var oEvent = {
cancelled : false,
data : response,
error : undef,
source : undef,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't source have a value? What is the source on Android and iOS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, source should not have a value. I based this code on the Android code in fact because I wasn't sure at first either. Facebook is a non-instantiatable class (think static class in Java), so there is no source to set.

success : false,
type : undef,
uid : response.id
};
if (response.authResponse) {
_expirationDate = new Date((new Date()).getTime() + response.authResponse.expiresIn * 1000);
FB.api('/me', function(response) {
if (!response) {
oEvent.error = "An unknown error occured.";
} else if (response.error) {
oEvent.error = response.error;
} else {
_loggedIn = true;
_uid = response.id;
oEvent.success = true;
oEvent.uid = _uid;
}
api.fireEvent('login', oEvent);
});
} else {
oEvent.cancelled = true
oEvent.error = "The user cancelled or an internal error occured."
api.fireEvent('login', oEvent);
}
}, {'scope':_permissions.join()});
};
api.createLoginButton = function(){
api.createLoginButton = function(parameters){
console.debug('Method "Titanium.Facebook.createLoginButton" is not implemented yet.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warn level and/or exception (fail fast)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do that, but it's important to note that this is the current convention in mobile web for unimplemented methods. There are hundreds of these things scattered throughout the codebase.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kk. we should fix them as we find them. Doesn't have to be a mass effort.

};
api.dialog = function(){
console.debug('Method "Titanium.Facebook.dialog" is not implemented yet.');
api.dialog = function(action,params,callback){
params.method = action;
FB.ui(params,function(response){
if (!response) {
var undef;
callback({'success':false,'error':undef,'path':path});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC we set source and type on all event objects sent back as a titanium standard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to the above mention about source, there is no source to set. Also, the API docs don't mention a source object anywhere (is this a problem with the docs?)

} else if (response.error) {
callback({'success':false,'error':response.error,'path':path});
} else {
callback({'success':true,'result':response,'path':path});
}
});
};
api.logout = function(){
console.debug('Method "Titanium.Facebook.logout" is not implemented yet.');
FB.logout(function(response) {
_loggedIn = false;
var undef;
var oEvent = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check parity w/ other platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you give me some more feedback? Parity of what?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On events, which we talked about. So nothing to do now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No sense creating the oEvent variable, I would prefer passing the object directly into fireEvent(). Saves 17 bytes and 1 less variable in the scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Easy enough, although it's worth noting that this code was copied from another class. These types of declarations are all over the codebase.

source : undef,
type : undef
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You do not need to pass in a type here and you can get rid of undef. fireEvent() will set the type for you. In this case it will automatically set type to "logout". Even if you set type to "hello", it gets clobbered.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is true of all events being passed through fireEvent then, correct? I think there are a few other areas where we can get rid of this.

};
api.fireEvent('logout', oEvent);
});
};
api.request = function(){
console.debug('Method "Titanium.Facebook.request" is not implemented yet.');
api.request = function(method,params,callback){
params.method = method;
params.urls = 'facebook.com,developers.facebook.com';
FB.api(params,function(response){
if (!response) {
var undef;
callback({'success':false,'error':undef,'method':method});
} else if (response.error) {
callback({'success':false,'error':response.error,'method':method});
} else {
callback({'success':true,'result':JSON.stringify(response),'method':method});
}
});
};
api.requestWithGraphPath = function(){
console.debug('Method "Titanium.Facebook.requestWithGraphPath" is not implemented yet.');
api.requestWithGraphPath = function(path,params,httpMethod,callback){
FB.api(path,httpMethod,params,function(response){
if (!response) {
var undef;
callback({'success':false,'error':undef,'path':path});
} else if (response.error) {
callback({'success':false,'error':response.error,'path':path});
} else {
callback({'success':true,'result':JSON.stringify(response),'path':path});
}
});
};

// Events
api.addEventListener('login', function(){
console.debug('Event "login" is not implemented yet.');
});
api.addEventListener('logout', function(){
console.debug('Event "logout" is not implemented yet.');
});
})(Ti._5.createClass('Titanium.Facebook'));