From fabfe76c1a6ced52cd9bcbe70a8ed20868fd04f1 Mon Sep 17 00:00:00 2001
From: Tobias Markmann <tm@ayena.de>
Date: Sun, 5 Apr 2015 13:10:48 +0200
Subject: Improve UX regarding room bookmark handling

Label the window for adding bookmarks as "Add Bookmark Details".
Allow modification of bookmarks from the cog menu in the chat window
and adjust the context menu item accordingly.

Test-Information:

Tested the bookmarks section of the "Chats" tab in the contact list
and the UX scenario using the cog menu that it works as expected.
Tested it on OS X 10.9.5 with Qt 5.4.1.

Change-Id: I80daf339fc86506db3d863decae4bcd892e3ea88

diff --git a/Swift/Controllers/Chat/ChatsManager.cpp b/Swift/Controllers/Chat/ChatsManager.cpp
index fe098de..3cd9169 100644
--- a/Swift/Controllers/Chat/ChatsManager.cpp
+++ b/Swift/Controllers/Chat/ChatsManager.cpp
@@ -6,62 +6,61 @@
 
 #include <Swift/Controllers/Chat/ChatsManager.h>
 
-#include <boost/bind.hpp>
 #include <boost/algorithm/string.hpp>
-#include <boost/smart_ptr/make_shared.hpp>
-#include <boost/archive/text_oarchive.hpp>
 #include <boost/archive/text_iarchive.hpp>
-#include <boost/serialization/optional.hpp>
-#include <boost/serialization/vector.hpp>
+#include <boost/archive/text_oarchive.hpp>
+#include <boost/bind.hpp>
 #include <boost/serialization/map.hpp>
-#include <boost/serialization/string.hpp>
+#include <boost/serialization/optional.hpp>
 #include <boost/serialization/split_free.hpp>
+#include <boost/serialization/string.hpp>
+#include <boost/serialization/vector.hpp>
+#include <boost/smart_ptr/make_shared.hpp>
 
+#include <Swiften/Avatars/AvatarManager.h>
+#include <Swiften/Base/Log.h>
 #include <Swiften/Base/foreach.h>
-#include <Swiften/Presence/PresenceSender.h>
+#include <Swiften/Client/ClientBlockListManager.h>
 #include <Swiften/Client/NickResolver.h>
-#include <Swiften/MUC/MUCManager.h>
+#include <Swiften/Client/StanzaChannel.h>
+#include <Swiften/Disco/DiscoServiceWalker.h>
 #include <Swiften/Elements/ChatState.h>
-#include <Swiften/Elements/MUCUserPayload.h>
 #include <Swiften/Elements/DeliveryReceipt.h>
 #include <Swiften/Elements/DeliveryReceiptRequest.h>
-#include <Swiften/MUC/MUCBookmarkManager.h>
-#include <Swiften/Avatars/AvatarManager.h>
 #include <Swiften/Elements/MUCInvitationPayload.h>
+#include <Swiften/Elements/MUCUserPayload.h>
+#include <Swiften/MUC/MUCBookmarkManager.h>
+#include <Swiften/MUC/MUCManager.h>
+#include <Swiften/Presence/PresenceSender.h>
 #include <Swiften/Roster/XMPPRoster.h>
-#include <Swiften/Client/ClientBlockListManager.h>
-#include <Swiften/Client/StanzaChannel.h>
+#include <Swiften/StringCodecs/Base64.h>
 #include <Swiften/VCards/VCardManager.h>
 
+#include <Swift/Controllers/Chat/AutoAcceptMUCInviteDecider.h>
 #include <Swift/Controllers/Chat/ChatController.h>
 #include <Swift/Controllers/Chat/ChatControllerBase.h>
-#include <Swift/Controllers/Chat/MUCSearchController.h>
-#include <Swift/Controllers/Chat/AutoAcceptMUCInviteDecider.h>
-#include <Swift/Controllers/XMPPEvents/EventController.h>
+#include <Swift/Controllers/Chat/ChatMessageParser.h>
 #include <Swift/Controllers/Chat/MUCController.h>
-#include <Swift/Controllers/UIEvents/RequestChatUIEvent.h>
-#include <Swift/Controllers/UIEvents/CreateImpromptuMUCUIEvent.h>
-#include <Swift/Controllers/UIEvents/JoinMUCUIEvent.h>
-#include <Swift/Controllers/UIEvents/RequestJoinMUCUIEvent.h>
+#include <Swift/Controllers/Chat/MUCSearchController.h>
+#include <Swift/Controllers/Chat/UserSearchController.h>
+#include <Swift/Controllers/FileTransfer/FileTransferController.h>
+#include <Swift/Controllers/FileTransfer/FileTransferOverview.h>
+#include <Swift/Controllers/ProfileSettingsProvider.h>
+#include <Swift/Controllers/SettingConstants.h>
+#include <Swift/Controllers/Settings/SettingsProvider.h>
 #include <Swift/Controllers/UIEvents/AddMUCBookmarkUIEvent.h>
-#include <Swift/Controllers/UIEvents/RemoveMUCBookmarkUIEvent.h>
+#include <Swift/Controllers/UIEvents/CreateImpromptuMUCUIEvent.h>
 #include <Swift/Controllers/UIEvents/EditMUCBookmarkUIEvent.h>
 #include <Swift/Controllers/UIEvents/InviteToMUCUIEvent.h>
+#include <Swift/Controllers/UIEvents/JoinMUCUIEvent.h>
+#include <Swift/Controllers/UIEvents/RemoveMUCBookmarkUIEvent.h>
+#include <Swift/Controllers/UIEvents/RequestChatUIEvent.h>
+#include <Swift/Controllers/UIEvents/RequestJoinMUCUIEvent.h>
 #include <Swift/Controllers/UIInterfaces/ChatListWindowFactory.h>
 #include <Swift/Controllers/UIInterfaces/JoinMUCWindow.h>
 #include <Swift/Controllers/UIInterfaces/JoinMUCWindowFactory.h>
-#include <Swift/Controllers/FileTransfer/FileTransferController.h>
-#include <Swift/Controllers/FileTransfer/FileTransferOverview.h>
-#include <Swift/Controllers/ProfileSettingsProvider.h>
-#include <Swift/Controllers/Settings/SettingsProvider.h>
-#include <Swift/Controllers/SettingConstants.h>
 #include <Swift/Controllers/WhiteboardManager.h>
-#include <Swift/Controllers/Chat/ChatMessageParser.h>
-#include <Swift/Controllers/Chat/UserSearchController.h>
-#include <Swiften/Disco/DiscoServiceWalker.h>
-#include <Swiften/Client/ClientBlockListManager.h>
-#include <Swiften/StringCodecs/Base64.h>
-#include <Swiften/Base/Log.h>
+#include <Swift/Controllers/XMPPEvents/EventController.h>
 
 BOOST_CLASS_VERSION(Swift::ChatListWindow::Chat, 1)
 
@@ -811,7 +810,7 @@ MUC::ref ChatsManager::handleJoinMUCRequest(const JID &mucJID, const boost::opti
 			chatWindowFactoryAdapter = new SingleChatWindowFactoryAdapter(reuseChatwindow);
 		}
 		boost::shared_ptr<ChatMessageParser> chatMessageParser = boost::make_shared<ChatMessageParser>(emoticons_, highlightManager_->getRules(), true); /* a message parser that knows this is a room/MUC (not a chat) */
-		controller = new MUCController(jid_, muc, password, nick, stanzaChannel_, iqRouter_, reuseChatwindow ? chatWindowFactoryAdapter : chatWindowFactory_, presenceOracle_, avatarManager_, uiEventStream_, false, timerFactory_, eventController_, entityCapsProvider_, roster_, historyController_, mucRegistry_, highlightManager_, clientBlockListManager_, chatMessageParser, isImpromptu, autoAcceptMUCInviteDecider_, vcardManager_);
+		controller = new MUCController(jid_, muc, password, nick, stanzaChannel_, iqRouter_, reuseChatwindow ? chatWindowFactoryAdapter : chatWindowFactory_, presenceOracle_, avatarManager_, uiEventStream_, false, timerFactory_, eventController_, entityCapsProvider_, roster_, historyController_, mucRegistry_, highlightManager_, clientBlockListManager_, chatMessageParser, isImpromptu, autoAcceptMUCInviteDecider_, vcardManager_, mucBookmarkManager_);
 		if (chatWindowFactoryAdapter) {
 			/* The adapters are only passed to chat windows, which are deleted in their
 			 * controllers' dtor, which are deleted in ChatManager's dtor. The adapters
diff --git a/Swift/Controllers/Chat/MUCController.cpp b/Swift/Controllers/Chat/MUCController.cpp
index 2706981..054f896 100644
--- a/Swift/Controllers/Chat/MUCController.cpp
+++ b/Swift/Controllers/Chat/MUCController.cpp
@@ -6,6 +6,8 @@
 
 #include <Swift/Controllers/Chat/MUCController.h>
 
+#include <algorithm>
+
 #include <boost/bind.hpp>
 #include <boost/regex.hpp>
 #include <boost/algorithm/string.hpp>
@@ -21,6 +23,8 @@
 #include <Swiften/Disco/EntityCapsProvider.h>
 #include <Swiften/Elements/Delay.h>
 #include <Swiften/MUC/MUC.h>
+#include <Swiften/MUC/MUCBookmark.h>
+#include <Swiften/MUC/MUCBookmarkManager.h>
 #include <Swiften/Network/Timer.h>
 #include <Swiften/Network/TimerFactory.h>
 #include <Swiften/Roster/XMPPRoster.h>
@@ -33,15 +37,15 @@
 #include <Swift/Controllers/Roster/ContactRosterItem.h>
 #include <Swift/Controllers/Roster/GroupRosterItem.h>
 #include <Swift/Controllers/Roster/ItemOperations/SetAvatar.h>
-#include <Swift/Controllers/Roster/ItemOperations/SetPresence.h>
 #include <Swift/Controllers/Roster/ItemOperations/SetMUC.h>
+#include <Swift/Controllers/Roster/ItemOperations/SetPresence.h>
 #include <Swift/Controllers/Roster/Roster.h>
 #include <Swift/Controllers/Roster/RosterVCardProvider.h>
 #include <Swift/Controllers/UIEvents/InviteToMUCUIEvent.h>
 #include <Swift/Controllers/UIEvents/RequestAddUserDialogUIEvent.h>
+#include <Swift/Controllers/UIEvents/RequestChangeBlockStateUIEvent.h>
 #include <Swift/Controllers/UIEvents/RequestChatUIEvent.h>
 #include <Swift/Controllers/UIEvents/RequestInviteToMUCUIEvent.h>
-#include <Swift/Controllers/UIEvents/RequestChangeBlockStateUIEvent.h>
 #include <Swift/Controllers/UIEvents/ShowProfileForRosterItemUIEvent.h>
 #include <Swift/Controllers/UIEvents/UIEventStream.h>
 #include <Swift/Controllers/UIInterfaces/ChatWindow.h>
@@ -51,7 +55,18 @@
 #define MUC_JOIN_WARNING_TIMEOUT_MILLISECONDS 60000
 
 namespace Swift {
-	
+
+class MUCBookmarkPredicate {
+	public:
+		MUCBookmarkPredicate(const JID& mucJID) : roomJID_(mucJID) { }
+		bool operator()(const MUCBookmark& operand) {
+			return operand.getRoom() == roomJID_;
+		}
+
+	private:
+		JID roomJID_;
+};
+
 /**
  * The controller does not gain ownership of the stanzaChannel, nor the factory.
  */
@@ -78,8 +93,9 @@ MUCController::MUCController (
 		boost::shared_ptr<ChatMessageParser> chatMessageParser,
 		bool isImpromptu,
 		AutoAcceptMUCInviteDecider* autoAcceptMUCInviteDecider,
-		VCardManager* vcardManager) :
-	ChatControllerBase(self, stanzaChannel, iqRouter, chatWindowFactory, muc->getJID(), presenceOracle, avatarManager, useDelayForLatency, uiEventStream, eventController, timerFactory, entityCapsProvider, historyController, mucRegistry, highlightManager, chatMessageParser, autoAcceptMUCInviteDecider), muc_(muc), nick_(nick), desiredNick_(nick), password_(password), renameCounter_(0), isImpromptu_(isImpromptu), isImpromptuAlreadyConfigured_(false), clientBlockListManager_(clientBlockListManager) {
+		VCardManager* vcardManager,
+		MUCBookmarkManager* mucBookmarkManager) :
+	ChatControllerBase(self, stanzaChannel, iqRouter, chatWindowFactory, muc->getJID(), presenceOracle, avatarManager, useDelayForLatency, uiEventStream, eventController, timerFactory, entityCapsProvider, historyController, mucRegistry, highlightManager, chatMessageParser, autoAcceptMUCInviteDecider), muc_(muc), nick_(nick), desiredNick_(nick), password_(password), renameCounter_(0), isImpromptu_(isImpromptu), isImpromptuAlreadyConfigured_(false), clientBlockListManager_(clientBlockListManager), mucBookmarkManager_(mucBookmarkManager) {
 	parting_ = true;
 	joined_ = false;
 	lastWasPresence_ = false;
@@ -137,6 +153,20 @@ MUCController::MUCController (
 	} 
 	handleBareJIDCapsChanged(muc->getJID());
 	eventStream_->onUIEvent.connect(boost::bind(&MUCController::handleUIEvent, this, _1));
+
+
+	// setup handling of MUC bookmark changes
+	mucBookmarkManagerBookmarkAddedConnection_ = (mucBookmarkManager_->onBookmarkAdded.connect(boost::bind(&MUCController::handleMUCBookmarkAdded, this, _1)));
+	mucBookmarkManagerBookmarkRemovedConnection_ = (mucBookmarkManager_->onBookmarkRemoved.connect(boost::bind(&MUCController::handleMUCBookmarkRemoved, this, _1)));
+
+	std::vector<MUCBookmark> mucBookmarks = mucBookmarkManager_->getBookmarks();
+	std::vector<MUCBookmark>::iterator bookmarkIterator = std::find_if(mucBookmarks.begin(), mucBookmarks.end(), MUCBookmarkPredicate(muc->getJID()));
+	if (bookmarkIterator != mucBookmarks.end()) {
+		updateChatWindowBookmarkStatus(*bookmarkIterator);
+	}
+	else {
+		updateChatWindowBookmarkStatus(boost::optional<MUCBookmark>());
+	}
 }
 
 MUCController::~MUCController() {
@@ -1142,4 +1172,31 @@ void MUCController::setAvailableServerFeatures(boost::shared_ptr<DiscoInfo> info
 	}
 }
 
+void MUCController::handleMUCBookmarkAdded(const MUCBookmark& bookmark) {
+	if (bookmark.getRoom() == muc_->getJID()) {
+		updateChatWindowBookmarkStatus(bookmark);
+	}
+}
+
+void MUCController::handleMUCBookmarkRemoved(const MUCBookmark& bookmark) {
+	if (bookmark.getRoom() == muc_->getJID()) {
+		updateChatWindowBookmarkStatus(boost::optional<MUCBookmark>());
+	}
+}
+
+void MUCController::updateChatWindowBookmarkStatus(const boost::optional<MUCBookmark>& bookmark) {
+	assert(chatWindow_);
+	if (bookmark) {
+		if (bookmark->getAutojoin()) {
+			chatWindow_->setBookmarkState(ChatWindow::RoomAutoJoined);
+		}
+		else {
+			chatWindow_->setBookmarkState(ChatWindow::RoomBookmarked);
+		}
+	}
+	else {
+		chatWindow_->setBookmarkState(ChatWindow::RoomNotBookmarked);
+	}
+}
+
 }
diff --git a/Swift/Controllers/Chat/MUCController.h b/Swift/Controllers/Chat/MUCController.h
index 7c24ae2..36edbdd 100644
--- a/Swift/Controllers/Chat/MUCController.h
+++ b/Swift/Controllers/Chat/MUCController.h
@@ -6,20 +6,20 @@
 
 #pragma once
 
+#include <map>
 #include <set>
 #include <string>
-#include <map>
 
 #include <boost/shared_ptr.hpp>
 #include <boost/signals/connection.hpp>
 
 #include <Swiften/Base/boost_bsignals.h>
-#include <Swiften/Network/Timer.h>
-#include <Swiften/Elements/Message.h>
 #include <Swiften/Elements/DiscoInfo.h>
+#include <Swiften/Elements/MUCOccupant.h>
+#include <Swiften/Elements/Message.h>
 #include <Swiften/JID/JID.h>
 #include <Swiften/MUC/MUC.h>
-#include <Swiften/Elements/MUCOccupant.h>
+#include <Swiften/Network/Timer.h>
 
 #include <Swift/Controllers/Chat/ChatControllerBase.h>
 #include <Swift/Controllers/Roster/RosterItem.h>
@@ -40,6 +40,8 @@ namespace Swift {
 	class VCardManager;
 	class RosterVCardProvider;
 	class ClientBlockListManager;
+	class MUCBookmarkManager;
+	class MUCBookmark;
 
 	enum JoinPart {Join, Part, JoinThenPart, PartThenJoin};
 
@@ -51,7 +53,7 @@ namespace Swift {
 
 	class MUCController : public ChatControllerBase {
 		public:
-			MUCController(const JID& self, MUC::ref muc, const boost::optional<std::string>& password, const std::string &nick, StanzaChannel* stanzaChannel, IQRouter* iqRouter, ChatWindowFactory* chatWindowFactory, PresenceOracle* presenceOracle, AvatarManager* avatarManager, UIEventStream* events, bool useDelayForLatency, TimerFactory* timerFactory, EventController* eventController, EntityCapsProvider* entityCapsProvider, XMPPRoster* roster, HistoryController* historyController, MUCRegistry* mucRegistry, HighlightManager* highlightManager, ClientBlockListManager* clientBlockListManager, boost::shared_ptr<ChatMessageParser> chatMessageParser, bool isImpromptu, AutoAcceptMUCInviteDecider* autoAcceptMUCInviteDecider, VCardManager* vcardManager);
+			MUCController(const JID& self, MUC::ref muc, const boost::optional<std::string>& password, const std::string &nick, StanzaChannel* stanzaChannel, IQRouter* iqRouter, ChatWindowFactory* chatWindowFactory, PresenceOracle* presenceOracle, AvatarManager* avatarManager, UIEventStream* events, bool useDelayForLatency, TimerFactory* timerFactory, EventController* eventController, EntityCapsProvider* entityCapsProvider, XMPPRoster* roster, HistoryController* historyController, MUCRegistry* mucRegistry, HighlightManager* highlightManager, ClientBlockListManager* clientBlockListManager, boost::shared_ptr<ChatMessageParser> chatMessageParser, bool isImpromptu, AutoAcceptMUCInviteDecider* autoAcceptMUCInviteDecider, VCardManager* vcardManager, MUCBookmarkManager* mucBookmarkManager);
 			virtual ~MUCController();
 			boost::signal<void ()> onUserLeft;
 			boost::signal<void ()> onUserJoined;
@@ -137,6 +139,10 @@ namespace Swift {
 			void handleUnblockUserRequest();
 			void handleBlockingStateChanged();
 
+			void handleMUCBookmarkAdded(const MUCBookmark& bookmark);
+			void handleMUCBookmarkRemoved(const MUCBookmark& bookmark);
+			void updateChatWindowBookmarkStatus(const boost::optional<MUCBookmark>& bookmark);
+
 		private:
 			MUC::ref muc_;
 			UIEventStream* events_;
@@ -169,6 +175,10 @@ namespace Swift {
 			boost::bsignals::scoped_connection blockingOnItemRemovedConnection_;
 
 			boost::optional<ChatWindow::AlertID> blockedContactAlert_;
+
+			MUCBookmarkManager* mucBookmarkManager_;
+			boost::bsignals::scoped_connection mucBookmarkManagerBookmarkAddedConnection_;
+			boost::bsignals::scoped_connection mucBookmarkManagerBookmarkRemovedConnection_;
 	};
 }
 
diff --git a/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp b/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp
index 0b7b793..cc4045a 100644
--- a/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp
+++ b/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp
@@ -4,10 +4,10 @@
  * See the COPYING file for more information.
  */
 
-#include <cppunit/extensions/HelperMacros.h>
-#include <cppunit/extensions/TestFactoryRegistry.h>
 #include <boost/algorithm/string.hpp>
 
+#include <cppunit/extensions/HelperMacros.h>
+#include <cppunit/extensions/TestFactoryRegistry.h>
 #include <hippomocks.h>
 
 #include <Swiften/Avatars/NullAvatarManager.h>
@@ -19,6 +19,7 @@
 #include <Swiften/Crypto/PlatformCryptoProvider.h>
 #include <Swiften/Disco/DummyEntityCapsProvider.h>
 #include <Swiften/Elements/MUCUserPayload.h>
+#include <Swiften/MUC/MUCBookmarkManager.h>
 #include <Swiften/MUC/UnitTest/MockMUC.h>
 #include <Swiften/Network/TimerFactory.h>
 #include <Swiften/Presence/DirectedPresenceSender.h>
@@ -88,11 +89,13 @@ public:
 		vcardStorage_ = new VCardMemoryStorage(crypto_.get());
 		vcardManager_ = new VCardManager(self_, iqRouter_, vcardStorage_);
 		clientBlockListManager_ = new ClientBlockListManager(iqRouter_);
-		controller_ = new MUCController (self_, muc_, boost::optional<std::string>(), nick_, stanzaChannel_, iqRouter_, chatWindowFactory_, presenceOracle_, avatarManager_, uiEventStream_, false, timerFactory, eventController_, entityCapsProvider_, NULL, NULL, mucRegistry_, highlightManager_, clientBlockListManager_, chatMessageParser_, false, NULL, vcardManager_);
+		mucBookmarkManager_ = new MUCBookmarkManager(iqRouter_);
+		controller_ = new MUCController (self_, muc_, boost::optional<std::string>(), nick_, stanzaChannel_, iqRouter_, chatWindowFactory_, presenceOracle_, avatarManager_, uiEventStream_, false, timerFactory, eventController_, entityCapsProvider_, NULL, NULL, mucRegistry_, highlightManager_, clientBlockListManager_, chatMessageParser_, false, NULL, vcardManager_, mucBookmarkManager_);
 	}
 
 	void tearDown() {
 		delete controller_;
+		delete mucBookmarkManager_;
 		delete clientBlockListManager_;
 		delete vcardManager_;
 		delete vcardStorage_;
@@ -438,6 +441,7 @@ private:
 	VCardManager* vcardManager_;
 	VCardMemoryStorage* vcardStorage_;
 	ClientBlockListManager* clientBlockListManager_;
+	MUCBookmarkManager* mucBookmarkManager_;
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(MUCControllerTest);
diff --git a/Swift/Controllers/UIInterfaces/ChatWindow.h b/Swift/Controllers/UIInterfaces/ChatWindow.h
index 8d0026c..765f16a 100644
--- a/Swift/Controllers/UIInterfaces/ChatWindow.h
+++ b/Swift/Controllers/UIInterfaces/ChatWindow.h
@@ -6,22 +6,22 @@
 
 #pragma once
 
-#include <vector>
 #include <string>
+#include <vector>
 
+#include <boost/date_time/posix_time/posix_time.hpp>
+#include <boost/make_shared.hpp>
 #include <boost/optional.hpp>
 #include <boost/shared_ptr.hpp>
-#include <boost/make_shared.hpp>
-#include <boost/date_time/posix_time/posix_time.hpp>
 
 #include <Swiften/Base/boost_bsignals.h>
-#include <Swiften/Elements/SecurityLabelsCatalog.h>
 #include <Swiften/Elements/ChatState.h>
 #include <Swiften/Elements/Form.h>
 #include <Swiften/Elements/MUCOccupant.h>
+#include <Swiften/Elements/SecurityLabelsCatalog.h>
 #include <Swiften/MUC/MUCBookmark.h>
-#include <Swift/Controllers/HighlightManager.h>
 
+#include <Swift/Controllers/HighlightManager.h>
 
 namespace Swift {
 	class AvatarManager;
@@ -95,6 +95,7 @@ namespace Swift {
 			enum Direction { UnknownDirection, DefaultDirection };
 			enum MUCType { StandardMUC, ImpromptuMUC };
 			enum TimestampBehaviour { KeepTimestamp, UpdateTimestamp };
+			enum RoomBookmarkState { RoomNotBookmarked, RoomBookmarked, RoomAutoJoined };
 
 			ChatWindow() {}
 			virtual ~ChatWindow() {}
@@ -157,6 +158,7 @@ namespace Swift {
 			virtual void setBlockingState(BlockingState state) = 0;
 			virtual void setCanInitiateImpromptuChats(bool supportsImpromptu) = 0;
 			virtual void showBookmarkWindow(const MUCBookmark& bookmark) = 0;
+			virtual void setBookmarkState(RoomBookmarkState bookmarkState) = 0;
 
 			/**
 			 * A handle that uniquely identities an alert message.
diff --git a/Swift/Controllers/UnitTest/MockChatWindow.h b/Swift/Controllers/UnitTest/MockChatWindow.h
index 1fdd340..2798be6 100644
--- a/Swift/Controllers/UnitTest/MockChatWindow.h
+++ b/Swift/Controllers/UnitTest/MockChatWindow.h
@@ -8,9 +8,9 @@
 
 #include <boost/shared_ptr.hpp>
 
-#include <Swift/Controllers/UIInterfaces/ChatWindow.h>
 #include <Swiften/Base/foreach.h>
 
+#include <Swift/Controllers/UIInterfaces/ChatWindow.h>
 
 namespace Swift {
 	class MockChatWindow : public ChatWindow {
@@ -80,6 +80,7 @@ namespace Swift {
 			virtual void setBlockingState(BlockingState) {}
 			virtual void setCanInitiateImpromptuChats(bool /*supportsImpromptu*/) {}
 			virtual void showBookmarkWindow(const MUCBookmark& /*bookmark*/) {}
+			virtual void setBookmarkState(RoomBookmarkState) {}
 
 			std::string bodyFromMessage(const ChatMessage& message) {
 				boost::shared_ptr<ChatTextMessagePart> text;
diff --git a/Swift/QtUI/QtAddBookmarkWindow.cpp b/Swift/QtUI/QtAddBookmarkWindow.cpp
index a272aa2..3596657 100644
--- a/Swift/QtUI/QtAddBookmarkWindow.cpp
+++ b/Swift/QtUI/QtAddBookmarkWindow.cpp
@@ -1,20 +1,19 @@
 /*
- * Copyright (c) 2010-2014 Isode Limited.
+ * Copyright (c) 2010-2015 Isode Limited.
  * All rights reserved.
  * See the COPYING file for more information.
  */
 
 #include "QtAddBookmarkWindow.h"
 
-#include <qdebug.h>
-
 namespace Swift {
 QtAddBookmarkWindow::QtAddBookmarkWindow(UIEventStream* eventStream) : eventStream_(eventStream) {
-
+	setWindowTitle(tr("Add Bookmark Details"));
 }
 
 QtAddBookmarkWindow::QtAddBookmarkWindow(UIEventStream* eventStream, const MUCBookmark& bookmark) : eventStream_(eventStream) {
 	createFormFromBookmark(bookmark);
+	setWindowTitle(tr("Add Bookmark Details"));
 }
 
 bool QtAddBookmarkWindow::commit() {
diff --git a/Swift/QtUI/QtChatWindow.cpp b/Swift/QtUI/QtChatWindow.cpp
index f25c033..b1b9f83 100644
--- a/Swift/QtUI/QtChatWindow.cpp
+++ b/Swift/QtUI/QtChatWindow.cpp
@@ -10,7 +10,6 @@
 #include <boost/lexical_cast.hpp>
 #include <boost/smart_ptr/make_shared.hpp>
 
-#include <qdebug.h>
 #include <QApplication>
 #include <QBoxLayout>
 #include <QCloseEvent>
@@ -34,7 +33,8 @@
 #include <QTime>
 #include <QToolButton>
 #include <QUrl>
-#include <QMimeData>
+
+#include <qdebug.h>
 
 #include <Swiften/Base/Log.h>
 
@@ -42,25 +42,26 @@
 #include <Swift/Controllers/Roster/Roster.h>
 #include <Swift/Controllers/Roster/RosterItem.h>
 #include <Swift/Controllers/Settings/SettingsProvider.h>
-#include <Swift/Controllers/UIEvents/UIEventStream.h>
-#include <Swift/Controllers/UIEvents/SendFileUIEvent.h>
 #include <Swift/Controllers/UIEvents/JoinMUCUIEvent.h>
+#include <Swift/Controllers/UIEvents/SendFileUIEvent.h>
+#include <Swift/Controllers/UIEvents/UIEventStream.h>
 
 #include <SwifTools/TabComplete.h>
 
-#include <Swift/QtUI/Roster/QtOccupantListWidget.h>
 #include <Swift/QtUI/QtAddBookmarkWindow.h>
+#include <Swift/QtUI/QtEditBookmarkWindow.h>
 #include <Swift/QtUI/QtPlainChatView.h>
-#include <Swift/QtUI/QtSettingsProvider.h>
 #include <Swift/QtUI/QtScaledAvatarCache.h>
+#include <Swift/QtUI/QtSettingsProvider.h>
 #include <Swift/QtUI/QtTextEdit.h>
 #include <Swift/QtUI/QtUISettingConstants.h>
 #include <Swift/QtUI/QtUtilities.h>
 #include <Swift/QtUI/QtWebKitChatView.h>
+#include <Swift/QtUI/Roster/QtOccupantListWidget.h>
 
 namespace Swift {
 
-QtChatWindow::QtChatWindow(const QString& contact, QtChatTheme* theme, UIEventStream* eventStream, SettingsProvider* settings, const std::map<std::string, std::string>& emoticons) : QtTabbable(), id_(Q2PSTRING(contact)), contact_(contact), nextAlertId_(0), eventStream_(eventStream), blockingState_(BlockingUnsupported), isMUC_(false), supportsImpromptuChat_(false) {
+QtChatWindow::QtChatWindow(const QString& contact, QtChatTheme* theme, UIEventStream* eventStream, SettingsProvider* settings, const std::map<std::string, std::string>& emoticons) : QtTabbable(), id_(Q2PSTRING(contact)), contact_(contact), nextAlertId_(0), eventStream_(eventStream), blockingState_(BlockingUnsupported), isMUC_(false), supportsImpromptuChat_(false), roomBookmarkState_(RoomNotBookmarked) {
 	settings_ = settings;
 	unreadCount_ = 0;
 	isOnline_ = true;
@@ -524,9 +525,6 @@ void QtChatWindow::updateTitleWithUnreadCount() {
 	emit titleUpdated();
 }
 
-
-
-
 void QtChatWindow::flash() {
 	emit requestFlash();
 }
@@ -725,8 +723,16 @@ void QtChatWindow::handleActionButtonClicked() {
 		}
 	}
 
-	QAction* bookmark = contextMenu.addAction(tr("Add bookmark..."));
-	bookmark->setEnabled(isOnline_);
+	QAction* bookmark = NULL;
+	if (isMUC_) {
+		if (roomBookmarkState_ == RoomNotBookmarked) {
+			bookmark = contextMenu.addAction(tr("Bookmark this room..."));
+		}
+		else {
+			bookmark = contextMenu.addAction(tr("Edit bookmark..."));
+		}
+		bookmark->setEnabled(isOnline_);
+	}
 
 	QAction* result = contextMenu.exec(QCursor::pos());
 	if (result == NULL) {
@@ -797,8 +803,14 @@ void QtChatWindow::setCanInitiateImpromptuChats(bool supportsImpromptu) {
 }
 
 void QtChatWindow::showBookmarkWindow(const MUCBookmark& bookmark) {
-	QtAddBookmarkWindow* window = new QtAddBookmarkWindow(eventStream_, bookmark);
-	window->show();
+	if (roomBookmarkState_ == RoomNotBookmarked) {
+		QtAddBookmarkWindow* window = new QtAddBookmarkWindow(eventStream_, bookmark);
+		window->show();
+	}
+	else {
+		QtEditBookmarkWindow* window = new QtEditBookmarkWindow(eventStream_, bookmark);
+		window->show();
+	}
 }
 
 std::string QtChatWindow::getID() const {
@@ -879,7 +891,6 @@ void QtChatWindow::setFileTransferStatus(std::string id, const FileTransferState
 	messageLog_->setFileTransferStatus(id, state, msg);
 }
 
-
 std::string QtChatWindow::addWhiteboardRequest(bool senderIsSelf) {
 	handleAppendedToLog();
 	return messageLog_->addWhiteboardRequest(contact_, senderIsSelf);
@@ -901,4 +912,8 @@ void QtChatWindow::setMessageReceiptState(const std::string& id, ChatWindow::Rec
 	messageLog_->setMessageReceiptState(id, state);
 }
 
+void QtChatWindow::setBookmarkState(RoomBookmarkState bookmarkState) {
+	roomBookmarkState_ = bookmarkState;
+}
+
 }
diff --git a/Swift/QtUI/QtChatWindow.h b/Swift/QtUI/QtChatWindow.h
index 5c6fa2a..06c6064 100644
--- a/Swift/QtUI/QtChatWindow.h
+++ b/Swift/QtUI/QtChatWindow.h
@@ -14,10 +14,10 @@
 #include <QString>
 #include <QTextCursor>
 
-#include <SwifTools/LastLineTracker.h>
-
 #include <Swift/Controllers/UIInterfaces/ChatWindow.h>
 
+#include <SwifTools/LastLineTracker.h>
+
 #include <Swift/QtUI/ChatSnippet.h>
 #include <Swift/QtUI/QtAffiliationEditor.h>
 #include <Swift/QtUI/QtEmoticonsGrid.h>
@@ -25,7 +25,6 @@
 #include <Swift/QtUI/QtSwiftUtil.h>
 #include <Swift/QtUI/QtTabbable.h>
 
-
 class QTextEdit;
 class QLineEdit;
 class QComboBox;
@@ -133,6 +132,7 @@ namespace Swift {
 			void setBlockingState(BlockingState state);
 			virtual void setCanInitiateImpromptuChats(bool supportsImpromptu);
 			virtual void showBookmarkWindow(const MUCBookmark& bookmark);
+			virtual void setBookmarkState(RoomBookmarkState bookmarkState);
 			virtual std::string getID() const;
 
 		public slots:
@@ -225,6 +225,7 @@ namespace Swift {
 			bool impromptu_;
 			bool isMUC_;
 			bool supportsImpromptuChat_;
+			RoomBookmarkState roomBookmarkState_;
 			QMenu* emoticonsMenu_;
 	};
 }
-- 
cgit v0.10.2-6-g49f6