[Fixed] Erratic behavior on first page load

garbear · garbear · commit bc0ee4b4d34d · 2012-02-14T19:31:49.000-08:00
This behavior would redirect the user to Special:Connect, even though they were already connected. On Special:Connect, because the user is already logged in, they wouldn't see any useful information and there would be no "return to previous page" button. The only way to exit Special:Connect would be to explicitly click the back button in their browser.

Steps to reproduce the bug can be found in the comment in FacebookHooks.php.
diff --git a/Facebook/FacebookHooks.php b/Facebook/FacebookHooks.php
@@ -316,10 +316,37 @@ public static function ResourceLoaderGetConfigVars( &$vars ) {
 		if ( $wgUser->isLoggedIn() ) {
 			global $facebook;
 			$ids = FacebookDB::getFacebookIDs($wgUser);
-			// If the Facebook session is invalid, let the client know who we expect
-			// to click "Log in with Facebook". Otherwise, if we have have a valid
-			// session but for *someone else*, let the client know this as well.
-			if (count($ids) && (!$facebook->getUser() || $facebook->getUser() != $ids[0])) {
+			// If the user is logged in, let the JavaScript code know who the
+			// account belongs to. The primary reason for this is so that if a
+			// user logs in to Facebook with a different account, we can show
+			// the "facebooklogoutandcontinue" form.
+			// 
+			// Previously, if the user was logged in and had a valid Facebook
+			// session, we would skip this step with the mentality that it was
+			// unnecessary as the JavaScript code would obviously already know
+			// the Facebook ID. However, this created a problem that can be
+			// reproduced as follows:
+			//    1. Log in to MediaWiki with a valid Facebook session
+			//    2. In another tab, log out of Facebook and then log in again
+			//       as the same user, creating a different session
+			//    3. Now reload the MediaWiki page. The server will see a valid
+			//       session and skip the fbId variable. However, the client
+			//       will fire the Facebook Login event because a new session
+			//       was picked up, even though the user was also logged in the
+			//       last time the page loaded.
+			// Now, because the JavaScript can't find the fbId variable, it
+			// assumes that the MediaWiki account isn't connected to a Facebook
+			// account and shows the "facebookmergeaccount" form. However, when
+			// this form is retrieved, the new session is synchronized to the
+			// server and the AJAX request is invalid because you can't merge
+			// an account that is already connected to a Facebook user.
+			// 
+			// In and of itself, we skip this extra check and always include
+			// the fbId variable.
+			// 
+			// There must be a prize for finding bugs like this one. Because
+			// seriously, I deserve it.
+			if ( count( $ids ) ) {
 				$vars['fbId'] = strval( $ids[0] );
 			}
 		}
diff --git a/Facebook/modules/ext.facebook.js b/Facebook/modules/ext.facebook.js
@@ -102,6 +102,9 @@
 							// if the server needs a valid access_token immediately, it can be
 							// accomplished by posting the access_token to an AJAX method that
 							// calls $facebook->setPersistentData('access_token', $access_token);
+							// It also might be the case where an AJAX call, even without
+							// explicitly setting the access token, causes this to happen by
+							// automatically synchronizing the new cookie state with the server.
 							//window.location.href = window.location.href;
 							
 							// Because we don't reload, acknowledge the login by hiding the
@@ -114,17 +117,7 @@
 						}
 					} else {
 						// New connection, show a MergeAccount form
-						// 
-						// 2/5/12 - This is showing erratic behavior when the page first
-						// loads. For now, send all traffic directly to Special:Connect.
-						// It seems the root cause is that the AJAX request actually
-						// syncs the Facebook session to the server when it requests the
-						// form. If this is the cause, we can use it to our advantage.
-						// TODO: In FacebookInit::init(), only instantiate the Facebook
-						// API if it isn't an AJAX request. Then, in the AJAX form classes,
-						// instantiate the API *after* we compute the state of authentication.
-						window.getAndShowGetForm();
-						//window.getAndShowGetForm('facebookmergeaccount');
+						window.getAndShowGetForm('facebookmergeaccount');
 					}
 				} else {
 					// User is trying to log in with Facebook. If the user exists, they will
