Refactor login() method to invoke FB.login() synchronously. Fixes issue ...

[obibring]

Fixes #24 by placing calls to FB.login outside of asynchronous $timeout calls.

[obibring]

@Ciul, issue #24 is caused by the asynchronous $timeout call to FB.login(). While different browsers will block window.open() based on different conditions, a universal solution is to make the call synchronous.

I've removed the call to $timeout, and in its place, have added synchronous call to FB.login(). If the FB SDK is not loaded at the time ngFacebook.login() is invoked, the promise is rejected.

Hope this makes it in. I'd like to use this library in production but can't do so until this issue is resolved. :).

[obibring]

See: http://stackoverflow.com/questions/2587677/avoid-browser-pop-up-blockers

[samstr]

It's a shame this repo seems quite inactive recently and no improvements have made it into master for about 7 months.

[luiscarlosjayk]

Hi Sam,
I've added you as collaborator.

This repo is open source, which means that people who use it should be the same that maintains it. I haven't worked in any more projects that need facebook login, that's why I haven't done any change in it anymore. But, anyone who wants to collaborate can write to me and I will add him/her right away.

Regards,
Luis

[obibring]

@Ciul, can you add me as a collaborator? I'd be happy to help maintain the project.

[luiscarlosjayk]

Of course @obibring :)
Thanks for helping

[mrzmyr]

http://plnkr.co/edit/XS49bbJyYlDIKubXxhtA?p=preview

here's is an example which works nicely with this refactoring. Looking forward to merge it soon.

Can anybody proof that this example is working, too ?

[obibring]

@mrzmyr, the plunker you provided calls $scope.login()(line 60) inside the callback to Facebook.getLoginStatus(). Because the callback is invoked asynchronously (See lines 294-297), browsers may block the popup, and this is what I see on Chrome V36.0.1985.125. To test this pull request, the call to $scope.login() must be invoked directly in response to user click, a la:

var userIsConnected = false;
Facebook.getLoginStatus(function(response) {
   if (response.status === 'connected')
       userIsConnected = true;
});
$scope.IntentLogin = function() {
  if (!userIsConnected)
      $scope.login();
};

[mrzmyr]

@obibring thanks, i updated the plnkr.

[obibring]

@mrzmyr Works for me now 👍

[obibring]

@mrzmyr, I tested the both the previous and current versions of your plunker against ie10 and ie11. Both browsers blocked the popup in the previous version of the plunker, and both browsers showed the popup in the newest version of the plunker.

[mrzmyr]

@obibring thanks for testing 👯

[obibring]

@mrzmyr, any concerns? I think we should be ready to go with this one.

[mrzmyr]

no concerns, but it would great to have a test for it. I already set up a protractor test for it yesterday, but get stuck when spawning up the app instance. The app (got it from the current plnkr without the new changes) always allowed the popup. This is another topic, so i'm looking forward to open up a issue for that.

[mrzmyr]

So i added a test in #66 for it.