After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 642358 - Empathy should check Reconnect_Required before calling Reconnect()
Empathy should check Reconnect_Required before calling Reconnect()
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Accounts
2.33.x
Other Linux
: Normal normal
: ---
Assigned To: Danielle Madeley
Depends on:
Blocks:
 
 
Reported: 2011-02-15 09:24 UTC by Danielle Madeley
Modified: 2011-02-21 13:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/account-reconnect-642358 (5.28 KB, patch)
2011-02-15 10:22 UTC, Guillaume Desmottes
reviewed Details | Review
Trivia for reconnect_required (1.32 KB, patch)
2011-02-21 01:32 UTC, Danielle Madeley
accepted-commit_now Details | Review

Description Danielle Madeley 2011-02-15 09:24:08 UTC
In empathy-account-settings tp_account_update_parameters_finish() does not check the value of @reconnect_required.

http://telepathy.freedesktop.org/spec/Account.html#Method:UpdateParameters

Currently Empathy calls tp_account_reconnect_async() indiscriminately.
Comment 1 Guillaume Desmottes 2011-02-15 10:22:01 UTC
Created attachment 180886 [details] [review]
http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/account-reconnect-642358

 libempathy-gtk/empathy-account-widget.c |   17 ++++++++++++++---
 libempathy/empathy-account-settings.c   |   17 ++++++++++++++++-
 libempathy/empathy-account-settings.h   |    1 +
 src/empathy-account-assistant.c         |    2 +-
 4 files changed, 32 insertions(+), 5 deletions(-)
Comment 2 Guillaume Desmottes 2011-02-15 10:22:42 UTC
I didn't have the chance to test with DBus_Property parameters, so please give it a try if you can.
Comment 3 Danielle Madeley 2011-02-16 01:53:16 UTC
This looks like it's working correctly now within Empathy. However I think possibly Mission Control is doing the wrong thing.

If you update a parameter:

method call sender=:1.187 -> dest=org.freedesktop.Telepathy.AccountManager serial=97 path=/org/freedesktop/Telepathy/Account/badger/badger/badger_2dtest_2d20; interface=org.freedesktop.Telepathy.Account; member=UpdateParameters
   array [
      dict entry(
         string "ofdT.Connection.Interface.Badger.ShowBadger"
         variant             boolean true
      )
   ]
   array [
   ]

method return sender=:1.183 -> dest=:1.187 reply_serial=97
   array [
   ]

If you unset a parameter:

method call sender=:1.176 -> dest=org.freedesktop.Telepathy.AccountManager serial=110 path=/org/freedesktop/Telepathy/Account/badger/badger/badger_2dtest_2d20; interface=org.freedesktop.Telepathy.Account; member=UpdateParameters
   array [
   ]
   array [
      string "ofdT.Connection.Interface.Badger.ShowBadger"
   ]

method return sender=:1.183 -> dest=:1.176 reply_serial=110
   array [
      string "ofdT.Connection.Interface.Badger.ShowBadger" 
   ]

It's worth noting that in this case, MC doesn't call Set() on the CM either.

We could work around this in Empathy by never "unsetting" parameters, but I'm not sure if that's the correct behaviour (I assume it unsets parameters when they're now the default?).

Filed https://bugs.freedesktop.org//show_bug.cgi?id=34316 against Mission Control.
Comment 4 Danielle Madeley 2011-02-16 01:59:21 UTC
Review of attachment 180886 [details] [review]:

Minor fixes, but happy to commit once fixed.

::: libempathy-gtk/empathy-account-widget.c
@@ +924,3 @@
   EmpathyAccountWidget *widget = EMPATHY_ACCOUNT_WIDGET (user_data);
   EmpathyAccountWidgetPriv *priv = GET_PRIV (widget);
+  gboolean reconnect_requiered;

"required"

::: libempathy/empathy-account-settings.c
@@ +1453,3 @@
+  if (g_strv_length (reconnect_required) > 0)
+    /* We have to reconnect the account */
+    g_simple_async_result_set_op_res_gboolean (priv->apply_result, TRUE);

Rather than branch,

g_simple_async_result_set_op_res_gboolean (priv->apply_result, g_strv_length (reconnect_required) > 0);
Comment 5 Guillaume Desmottes 2011-02-16 10:28:55 UTC
Thansk, I amend the commit and merged to 2.34 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.
Comment 6 Danielle Madeley 2011-02-21 01:32:40 UTC
Created attachment 181431 [details] [review]
Trivia for reconnect_required

Prevent segv attempting to free uninitialised value. Fix leak.
Comment 7 Guillaume Desmottes 2011-02-21 09:43:31 UTC
Review of attachment 181431 [details] [review]:

++; 2.34 and master please.
Comment 8 Guillaume Desmottes 2011-02-21 13:02:33 UTC
Merged to master and 2.34

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.