From 5fa6933f9cc3e6d0df8c6896871cbe6cc09a9492 Mon Sep 17 00:00:00 2001 From: Tobias Markmann <tm@ayena.de> Date: Sun, 19 Feb 2017 20:55:46 +0100 Subject: Do not clear join/leave queue for MUC rooms on CSN messages Previously chat-state notification messages would cause the join/leave queue of a MUC room to be cleared, resulting in taking up more vertical space than it had to. Test-Information: Compared two Swift builds (one with this patch and one without) in a room where some occupants would send CSN messages from time to time. With this patch, a CSN message clearly does not cause the join/leave queue to be cleared. Added unit test to verify new behaviour. Tested on macOS 10.12.3 with Qt 5.7.1. Change-Id: I0aee733fa5d16bbfb497a17b3d7a3ffe3fea8f26 diff --git a/Swift/Controllers/Chat/MUCController.cpp b/Swift/Controllers/Chat/MUCController.cpp index 8fa98f6..545eab5 100644 --- a/Swift/Controllers/Chat/MUCController.cpp +++ b/Swift/Controllers/Chat/MUCController.cpp @@ -539,8 +539,14 @@ void MUCController::preHandleIncomingMessage(std::shared_ptr<MessageEvent> messa if (messageEvent->getStanza()->getType() == Message::Groupchat) { lastActivity_ = boost::posix_time::microsec_clock::universal_time(); } - clearPresenceQueue(); std::shared_ptr<Message> message = messageEvent->getStanza(); + + // This avoids clearing join/leave queue for chat state notification messages + // which are not readable (e.g. have no body content). + if (!(!messageEvent->isReadable() && message->getPayload<ChatState>())) { + clearPresenceQueue(); + } + if (joined_ && messageEvent->getStanza()->getFrom().getResource() != nick_ && messageTargetsMe(message) && !message->getPayload<Delay>() && messageEvent->isReadable()) { chatWindow_->flash(); } diff --git a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp index 00df3da..1958408 100644 --- a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp +++ b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp @@ -116,6 +116,7 @@ class ChatsManagerTest : public CppUnit::TestFixture { CPPUNIT_TEST(testChatControllerFullJIDBindingOnTypingAndNotActive); CPPUNIT_TEST(testLocalMUCServiceDiscoveryResetOnDisconnect); CPPUNIT_TEST(testPresenceChangeDoesNotReplaceMUCInvite); + CPPUNIT_TEST(testNotSplittingMUCPresenceJoinLeaveLinesOnChatStateNotifications); // MUC PM Tests CPPUNIT_TEST(testChatControllerPMPresenceHandling); @@ -1095,6 +1096,73 @@ public: CPPUNIT_ASSERT_EQUAL(std::string("testling@test.com has gone offline."), MockChatWindow::bodyFromMessage(window->lastAddedPresence_)); } + void testNotSplittingMUCPresenceJoinLeaveLinesOnChatStateNotifications() { + JID mucJID("mucroom@rooms.test.com"); + std::string nickname = "toodles"; + + MockChatWindow* window = new MockChatWindow(); + mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(mucJID, uiEventStream_).Return(window); + + uiEventStream_->send(std::make_shared<JoinMUCUIEvent>(mucJID, boost::optional<std::string>(), nickname)); + + auto genRemoteMUCPresence = [=]() { + auto presence = Presence::create(); + presence->setFrom(mucJID.withResource(nickname)); + presence->setTo(jid_); + return presence; + }; + + { + auto presence = genRemoteMUCPresence(); + auto userPayload = std::make_shared<MUCUserPayload>(); + userPayload->addStatusCode(110); + userPayload->addItem(MUCItem(MUCOccupant::Owner, jid_, MUCOccupant::Moderator)); + presence->addPayload(userPayload); + stanzaChannel_->onPresenceReceived(presence); + } + + { + auto presence = genRemoteMUCPresence(); + presence->setFrom(mucJID.withResource("someDifferentNickname")); + auto userPayload = std::make_shared<MUCUserPayload>(); + presence->addPayload(userPayload); + stanzaChannel_->onPresenceReceived(presence); + } + CPPUNIT_ASSERT_EQUAL(std::string("someDifferentNickname has entered the room."), window->bodyFromMessage(window->lastAddedPresence_)); + CPPUNIT_ASSERT_EQUAL(std::string(), window->bodyFromMessage(window->lastReplacedMessage_)); + window->resetLastMessages(); + + { + auto presence = genRemoteMUCPresence(); + presence->setFrom(mucJID.withResource("Romeo")); + auto userPayload = std::make_shared<MUCUserPayload>(); + presence->addPayload(userPayload); + stanzaChannel_->onPresenceReceived(presence); + } + CPPUNIT_ASSERT_EQUAL(std::string(), window->bodyFromMessage(window->lastAddedPresence_)); + CPPUNIT_ASSERT_EQUAL(std::string("someDifferentNickname and Romeo have entered the room"), window->bodyFromMessage(window->lastReplacedMessage_)); + window->resetLastMessages(); + + { + auto message = std::make_shared<Message>(); + message->setFrom(mucJID.withResource("Romeo")); + message->setTo(mucJID); + message->setType(Message::Groupchat); + message->addPayload(std::make_shared<ChatState>(ChatState::Composing)); + manager_->handleIncomingMessage(message); + } + + { + auto presence = genRemoteMUCPresence(); + presence->setFrom(mucJID.withResource("Juliet")); + auto userPayload = std::make_shared<MUCUserPayload>(); + presence->addPayload(userPayload); + stanzaChannel_->onPresenceReceived(presence); + } + CPPUNIT_ASSERT_EQUAL(std::string(), window->bodyFromMessage(window->lastAddedPresence_)); + CPPUNIT_ASSERT_EQUAL(std::string("someDifferentNickname, Romeo and Juliet have entered the room"), window->bodyFromMessage(window->lastReplacedMessage_)); + } + template <typename CarbonsType> Message::ref createCarbonsMessage(std::shared_ptr<CarbonsType> carbons, std::shared_ptr<Message> forwardedMessage) { auto messageWrapper = std::make_shared<Message>(); -- cgit v0.10.2-6-g49f6