QueryBufferItem disconnect IrcUser when removing

Add call to disconnect() when removing IrcUser from QueryBufferItem,
preventing _ircUser from triggering removeIrcUser() again when it's
destroyed by going out of scope.  Additionally, limit dataChanged()
to when IrcUser is removed.  May offer slight speed benefits.

Fixes at least one case of the client showing some nicknames as
offline when they're actually available (e.g. restarting the client
without restarting the core reveals them as online).

Test case from Bitlbee - receive following (no delay between AWAYs):
:nick!name@gmail.com JOIN :&bitlbee
:nick!name@gmail.com AWAY :Away (Testing)
:nick!name@gmail.com QUIT :Leaving...
:nick!name@gmail.com AWAY :User is offline

Before, 'nick' was wrongly shown as not available.  After, 'nick' is
shown as away with the message "User is offline".

(Bitlbee does this as GTalk/XMPP allow for messaging offline users)

Resolves GH-239.

diff --git a/src/client/networkmodel.cpp b/src/client/networkmodel.cpp
index dd5b848..974f31a 100644 (file)
--- a/src/client/networkmodel.cpp
+++ b/src/client/networkmodel.cpp
@@ -660,8 +660,22 @@ void QueryBufferItem::setIrcUser(IrcUser *ircUser)
 
 void QueryBufferItem::removeIrcUser()
 {
-    _ircUser = 0;
-    emit dataChanged();
+    if (_ircUser) {
+        // Disconnect the active IrcUser before removing it, otherwise it will fire removeIrcUser()
+        // a second time when the object's destroyed due to QueryBufferItem::setIrcUser() connecting
+        // SIGNAL destroyed(QObject*) to SLOT removeIrcUser().
+        // This fixes removing an active IrcUser if the user had quit then rejoined in a nonstandard
+        // manner (e.g. updateNickFromHost calling newIrcUser, triggered by an away-notify message).
+        disconnect(_ircUser, 0, this, 0);
+
+        // Clear IrcUser (only set to 0 if not already 0)
+        _ircUser = 0;
+
+        // Only emit dataChanged() if data actually changed.  This might serve as a small
+        // optimization, but it can be moved outside the if statement if other behavior depends on
+        // it always being called.
+        emit dataChanged();
+    }
 }