Fix: Stale custom context menu entries (#3301658)

Added a new ContextActionRemove protobuf message type triggering the clientside removal of a context menu entry. Since the server only remembers the action identifier and doesn't have a notion of where it is applied to on the client it is only possible to remove all entries for an identifier at the same time at this point. The removal is triggered when overriding a new handler (clientside visibility might have changed), remove a handler and in error conditions.

5 files changed, 51 insertions, 2 deletions

diff --git a/src/Message.h b/src/Message.h
index dac8534ff..9f007c84d 100644
--- a/src/Message.h
+++ b/src/Message.h
@@ -52,6 +52,7 @@
 	MUMBLE_MH_MSG(QueryUsers) \
 	MUMBLE_MH_MSG(CryptSetup) \
 	MUMBLE_MH_MSG(ContextActionAdd) \
+	MUMBLE_MH_MSG(ContextActionRemove) \
 	MUMBLE_MH_MSG(ContextAction) \
 	MUMBLE_MH_MSG(UserList) \
 	MUMBLE_MH_MSG(VoiceTarget) \
diff --git a/src/Mumble.proto b/src/Mumble.proto
index b8ce7c51a..19a955de8 100644
--- a/src/Mumble.proto
+++ b/src/Mumble.proto
@@ -201,6 +201,10 @@ message ContextActionAdd {
 	optional uint32 context = 3;
 }
 
+message ContextActionRemove {
+	required string action = 1;
+}
+
 message ContextAction {
 	optional uint32 session = 1;
 	optional uint32 channel_id = 2;
diff --git a/src/mumble/Messages.cpp b/src/mumble/Messages.cpp
index 3270332b2..505d22770 100644
--- a/src/mumble/Messages.cpp
+++ b/src/mumble/Messages.cpp
@@ -641,6 +641,24 @@ void MainWindow::msgContextActionAdd(const MumbleProto::ContextActionAdd &msg) {
 		qlChannelActions.append(a);
 }
 
+void MainWindow::msgContextActionRemove(const MumbleProto::ContextActionRemove &msg) {
+	QString action = u8(msg.action());
+
+	QSet<QAction *> qs;
+	qs += qlServerActions.toSet();
+	qs += qlChannelActions.toSet();
+	qs += qlUserActions.toSet();
+
+	foreach(QAction *a, qs) {
+		if (a->data() == action) {
+			qlServerActions.removeOne(a);
+			qlChannelActions.removeOne(a);
+			qlUserActions.removeOne(a);
+			delete a;
+		}
+	}
+}
+
 void MainWindow::msgVersion(const MumbleProto::Version &msg) {
 	if (msg.has_version())
 		g.sh->uiVersion = msg.version();
diff --git a/src/murmur/Messages.cpp b/src/murmur/Messages.cpp
index 9023e8e24..fe29fdc7d 100644
--- a/src/murmur/Messages.cpp
+++ b/src/murmur/Messages.cpp
@@ -1375,6 +1375,9 @@ void Server::msgCryptSetup(ServerUser *uSource, MumbleProto::CryptSetup &msg) {
 void Server::msgContextActionAdd(ServerUser *, MumbleProto::ContextActionAdd &) {
 }
 
+void Server::msgContextActionRemove(ServerUser *, MumbleProto::ContextActionRemove &) {
+}
+
 void Server::msgContextAction(ServerUser *uSource, MumbleProto::ContextAction &msg) {
 	MSG_SETUP(ServerUser::Authenticated);
 
diff --git a/src/murmur/MurmurIce.cpp b/src/murmur/MurmurIce.cpp
index 2447090ca..c52b188b6 100644
--- a/src/murmur/MurmurIce.cpp
+++ b/src/murmur/MurmurIce.cpp
@@ -495,6 +495,13 @@ void MurmurIce::contextAction(const ::User *pSrc, const QString &action, unsigne
 	} catch (...) {
 		qCritical("Registered Ice ServerContextCallback %s on server %d, session %d, action %s failed", qPrintable(QString::fromStdString(communicator->proxyToString(prx))), s->iServerNum, pSrc->uiSession, qPrintable(action));
 		qmUser.remove(action);
+
+		// Remove clientside entry
+		MumbleProto::ContextActionRemove mpcar;
+		mpcar.set_action(u8(action));
+		ServerUser *su = s->qhUsers.value(session);
+		if (su)
+			s->sendMessage(su, mpcar);
 	}
 }
 
@@ -1024,6 +1031,13 @@ static void impl_Server_addContextCallback(const Murmur::AMD_Server_addContextCa
 
 	try {
 		const Murmur::ServerContextCallbackPrx &oneway = Murmur::ServerContextCallbackPrx::checkedCast(cbptr->ice_oneway()->ice_connectionCached(false)->ice_timeout(5000));
+		if (qmPrx.contains(u8(action))) {
+			// Since the server has no notion of the ctx part of the context action
+			// make sure we remove them all clientside when overriding an old callback
+			MumbleProto::ContextActionRemove mpcar;
+			mpcar.set_action(action);
+			server->sendMessage(user, mpcar);
+		}
 		qmPrx.insert(u8(action), oneway);
 		cb->ice_response();
 	} catch (...) {
@@ -1047,9 +1061,18 @@ static void impl_Server_removeContextCallback(const Murmur::AMD_Server_removeCon
 		const Murmur::ServerContextCallbackPrx &oneway = Murmur::ServerContextCallbackPrx::uncheckedCast(cbptr->ice_oneway()->ice_connectionCached(false)->ice_timeout(5000));
 
 		foreach(int session, qmPrx.keys()) {
-			QMap<QString, ::Murmur::ServerContextCallbackPrx> qm = qmPrx[session];
-			foreach(const QString &act, qm.keys(oneway))
+			ServerUser *user = server->qhUsers.value(session);
+			QMap<QString, ::Murmur::ServerContextCallbackPrx> &qm = qmPrx[session];
+			foreach(const QString &act, qm.keys(oneway)) {
 				qm.remove(act);
+
+				// Ask clients to remove the clientside callbacks
+				if (user) {
+					MumbleProto::ContextActionRemove mpcar;
+					mpcar.set_action(u8(act));
+					server->sendMessage(user, mpcar);
+				}
+			}
 		}
 
 		cb->ice_response();