From 055e384668d38987a0054169e4feb43645d9735b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= <joerg@thalheim.io>
Date: Sun, 26 Apr 2026 12:49:46 +0200
Subject: [PATCH] core/model: make diffUpdate handle reordering without
 duplicating rows

diffUpdate inserted an element at its target index without removing it
from its previous index, so a pure permutation like [d,a,b,c] ->
[a,b,c,d] left every misplaced element in the model twice and the
removal pass never reclaimed them because their pointer is still in
newValues. Any caller that re-sorts an existing list (dbusmenu
re-layout, desktopentry re-scan) can hit this. Found via a downstream
niri workspace backend where a monitor hot-plug re-sorts the model.
---
 src/core/model.hpp            |  4 +++
 src/core/test/CMakeLists.txt  |  1 +
 src/core/test/objectmodel.cpp | 47 +++++++++++++++++++++++++++++++++++
 src/core/test/objectmodel.hpp | 12 +++++++++
 4 files changed, 64 insertions(+)
 create mode 100644 src/core/test/objectmodel.cpp
 create mode 100644 src/core/test/objectmodel.hpp

diff --git a/src/core/model.hpp b/src/core/model.hpp
index 0e88025..37b0447 100644
--- a/src/core/model.hpp
+++ b/src/core/model.hpp
@@ -138,6 +138,10 @@ public:
 		qsizetype oi = 0;
 		for (auto* object: newValues) {
 			if (this->mValuesList.length() == oi || this->mValuesList.at(oi) != object) {
+				// object may already be present further down (reorder case);
+				// drop the old row first so the same pointer is not inserted twice.
+				auto old = this->mValuesList.indexOf(object, oi);
+				if (old != -1) this->removeAt(old);
 				this->insertObject(object, oi);
 			}
 
diff --git a/src/core/test/CMakeLists.txt b/src/core/test/CMakeLists.txt
index 4e66c62..b35b42d 100644
--- a/src/core/test/CMakeLists.txt
+++ b/src/core/test/CMakeLists.txt
@@ -8,3 +8,4 @@ qs_test(transformwatcher transformwatcher.cpp)
 qs_test(ringbuffer ringbuf.cpp)
 qs_test(scriptmodel scriptmodel.cpp)
 qs_test(stacklist stacklist.cpp)
+qs_test(objectmodel objectmodel.cpp)
diff --git a/src/core/test/objectmodel.cpp b/src/core/test/objectmodel.cpp
new file mode 100644
index 0000000..b9b1d62
--- /dev/null
+++ b/src/core/test/objectmodel.cpp
@@ -0,0 +1,47 @@
+#include "objectmodel.hpp"
+
+#include <qlist.h>
+#include <qobject.h>
+#include <qtest.h>
+#include <qtestcase.h>
+
+#include "../model.hpp"
+
+void TestObjectModel::diffUpdateInsertRemove() {
+	QObject a;
+	QObject b;
+	QObject c;
+	QObject d;
+
+	auto model = ObjectModel<QObject>(nullptr);
+	model.insertObject(&a);
+	model.insertObject(&b);
+	model.insertObject(&c);
+	QCOMPARE(model.valueList(), (QList<QObject*> {&a, &b, &c}));
+
+	// drop b, append d — relative order of survivors unchanged
+	model.diffUpdate({&a, &c, &d});
+	QCOMPARE(model.valueList(), (QList<QObject*> {&a, &c, &d}));
+}
+
+void TestObjectModel::diffUpdateReorder() {
+	QObject a;
+	QObject b;
+	QObject c;
+	QObject d;
+
+	auto model = ObjectModel<QObject>(nullptr);
+	model.insertObject(&d);
+	model.insertObject(&a);
+	model.insertObject(&b);
+	model.insertObject(&c);
+	QCOMPARE(model.valueList(), (QList<QObject*> {&d, &a, &b, &c}));
+
+	// pure permutation: same elements, different order. Previously turned
+	// [d,a,b,c] into [a,b,c,d,a,b,c] because the old row was never removed
+	// before inserting at the new index.
+	model.diffUpdate({&a, &b, &c, &d});
+	QCOMPARE(model.valueList(), (QList<QObject*> {&a, &b, &c, &d}));
+}
+
+QTEST_MAIN(TestObjectModel);
diff --git a/src/core/test/objectmodel.hpp b/src/core/test/objectmodel.hpp
new file mode 100644
index 0000000..e3a10f9
--- /dev/null
+++ b/src/core/test/objectmodel.hpp
@@ -0,0 +1,12 @@
+#pragma once
+
+#include <qobject.h>
+#include <qtmetamacros.h>
+
+class TestObjectModel: public QObject {
+	Q_OBJECT;
+
+private slots:
+	static void diffUpdateInsertRemove();
+	static void diffUpdateReorder();
+};