Created attachment 185853 [details] [review]
accounts_dialog_cms_prepare_cb: don't update the settings if we are already preparing one

This may lead to ignoring the existing setting if it's a slow one to prepare
(as Haze for example). (#647641)

I wonder if we shouldn't move this block in accounts_dialog_update_settings()

  if (priv->settings_ready != NULL)
    {
      g_signal_handler_disconnect (priv->settings_ready,
          priv->settings_ready_id);
      priv->settings_ready = NULL;
      priv->settings_ready_id = 0;
    }

after the

  if (!settings)
    {
    }

block. I guess the "priv->settings_ready = NULL" is what is causing your issue, but if you don't call accounts_dialog_update_settings() at all, are we doing the select_first() or the add_button_clicked_cb() somewhere else?

if we do that won't fix the block as we'll still disconnect the signal.

All this code was already executed one (which is why we had priv->settings_ready != NULL), but calling it a second time disconnect the signal and so the widget is never populated.

We wouldn't disconnect it if we reorder the two blocks as I suggested, since the "if (!settings)" block returns from the function.

I.e. reordering the if blocks would have the same effect as:

@@ -917,7 +917,7 @@ accounts_dialog_update_settings (EmpathyAccountsDialog *dialog,
 {
   EmpathyAccountsDialogPriv *priv = GET_PRIV (dialog);
 
-  if (priv->settings_ready != NULL)
+  if (settings != NULL && priv->settings_ready != NULL)
     {
       g_signal_handler_disconnect (priv->settings_ready,
           priv->settings_ready_id);

I'm not sure if this would have other side-effects, just wondering if your fix would mark the first account as selected or not, and if not whether this would be more correct.

(In reply to comment #4)
> I'm not sure if this would have other side-effects, just wondering if your fix
> would mark the first account as selected or not, and if not whether this would
> be more correct.

With my fix the first account is still selected because
accounts_dialog_model_select_first() is called in
accounts_dialog_accounts_setup().

Actually, that thanks to this call that we select the first item and that
accounts_dialog_update_settings() is called in
accounts_dialog_model_selection_changed() with a proper settings.

All of this code is pretty hairy tbh, so I think it would be best to avoid
changing it too much to avoid unexpected side effects.

(In reply to comment #5)
> All of this code is pretty hairy tbh

Agreed!

> so I think it would be best to avoid
> changing it too much to avoid unexpected side effects.

Yeah, I'm not too sure of my change anyway and I haven't even tested it, so let's go for your fix. r+

thanks, merged to 3.0 and master.

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.