Bug#859633: unblock: kitemmodels/5.28.0-2
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
Dear release team,
I've backported three upstream fixes in kitemmmodels for stretch that I
consider important enough:
Fix assert (in beginRemoveRows) when deselecting an empty child of a selected
child in korganizer (24db696)
-> affects korganizer
KExtraColumnsProxyModel: Persist model indexes after emitting layoutChange,
not before (f55b055)
-> affects the moving folder operations in kmail
Update proxies for recently realised class of bugs (fdc69d0)
-> follows the same logic in other parts of the code
I uploaded 5.28.0-2 with these, and it has already successfully built in all
the release architectures.
I'm attaching the corresponding debdiff.
Happy hacking,
Please unblock package kitemmodels
unblock kitemmodels/5.28.0-2
-- System Information:
Debian Release: 9.0
APT prefers unstable-debug
APT policy: (500, 'unstable-debug'), (500, 'testing-debug'), (500, 'testing'), (500, 'stable'), (50, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386, armhf
Kernel: Linux 4.9.0-1-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash
Init: systemd (via /run/systemd/system)
diff -Nru kitemmodels-5.28.0/debian/changelog kitemmodels-5.28.0/debian/changelog
--- kitemmodels-5.28.0/debian/changelog 2016-11-18 16:01:50.000000000 +0100
+++ kitemmodels-5.28.0/debian/changelog 2017-04-04 18:03:26.000000000 +0200
@@ -1,3 +1,14 @@
+kitemmodels (5.28.0-2) unstable; urgency=medium
+
+ * Add new upstream patch:
+ Fix-assert-in-beginRemoveRows-when-deselecting-an-empty-c.patch
+ * Add new upstream patch:
+ KExtraColumnsProxyModel-Persist-model-indexes-after-emitt.patch
+ * Add new upstream patch:
+ Update-proxies-for-recently-realised-class-of-bugs.patch
+
+ -- Maximiliano Curia <maxy@debian.org> Tue, 04 Apr 2017 18:03:26 +0200
+
kitemmodels (5.28.0-1) unstable; urgency=medium
[ Automatic packaging ]
diff -Nru kitemmodels-5.28.0/debian/patches/Fix-assert-in-beginRemoveRows-when-deselecting-an-empty-c.patch kitemmodels-5.28.0/debian/patches/Fix-assert-in-beginRemoveRows-when-deselecting-an-empty-c.patch
--- kitemmodels-5.28.0/debian/patches/Fix-assert-in-beginRemoveRows-when-deselecting-an-empty-c.patch 1970-01-01 01:00:00.000000000 +0100
+++ kitemmodels-5.28.0/debian/patches/Fix-assert-in-beginRemoveRows-when-deselecting-an-empty-c.patch 2017-04-04 18:03:26.000000000 +0200
@@ -0,0 +1,79 @@
+From: David Faure <faure@kde.org>
+Date: Fri, 16 Dec 2016 10:23:09 +0100
+Subject: Fix assert (in beginRemoveRows) when deselecting an empty child of a
+ selected child in korganizer
+
+After
+ int proxyEndRemove = proxyStartRemove;
+ proxyEndRemove += rc; was adding 0 (empty root)
+and then --proxyEndRemove; was making us end up with proxyEndRemove < proxyStartRemove.
+
+REVIEW: 129657
+---
+ autotests/kselectionproxymodeltest.cpp | 14 ++++++++++++++
+ src/kselectionproxymodel.cpp | 6 +++---
+ 2 files changed, 17 insertions(+), 3 deletions(-)
+
+diff --git a/autotests/kselectionproxymodeltest.cpp b/autotests/kselectionproxymodeltest.cpp
+index 483e7c4..da8ce13 100644
+--- a/autotests/kselectionproxymodeltest.cpp
++++ b/autotests/kselectionproxymodeltest.cpp
+@@ -310,6 +310,13 @@ void KSelectionProxyModelTest::deselection_data()
+ << 1
+ << QStringList{"4"} << 5;
+ ++testNumber;
++
++ QTest::newRow(QByteArray("test" + QByteArray::number(testNumber)).data())
++ << static_cast<int>(KSelectionProxyModel::ChildrenOfExactSelection)
++ << QStringList{"6", "7"} << 1
++ << 0
++ << QStringList{"7"} << 1;
++ ++testNumber;
+ }
+
+ void KSelectionProxyModelTest::deselection()
+@@ -604,6 +611,13 @@ void KSelectionProxyModelTest::removeRows_data()
+ << 1
+ << QStringList{"9", "9"} << 2;
+ ++testNumber;
++
++ QTest::newRow(QByteArray("test" + QByteArray::number(testNumber)).data())
++ << static_cast<int>(kspm_mode) << connectSelectionModelFirst << false
++ << QStringList{"6", "8", "11"} << 4
++ << 0
++ << QStringList{"8", "8"} << 4;
++ ++testNumber;
+ }
+ }
+
+diff --git a/src/kselectionproxymodel.cpp b/src/kselectionproxymodel.cpp
+index 0f57c70..873e974 100644
+--- a/src/kselectionproxymodel.cpp
++++ b/src/kselectionproxymodel.cpp
+@@ -1176,7 +1176,7 @@ QPair<int, int> KSelectionProxyModelPrivate::beginRemoveRows(const QModelIndex &
+ }
+
+ --proxyEndRemove;
+- if (proxyEndRemove >= 0) {
++ if (proxyEndRemove >= proxyStartRemove) {
+ return qMakePair(proxyStartRemove, proxyEndRemove);
+ }
+ return qMakePair(-1, -1);
+@@ -1750,7 +1750,7 @@ void KSelectionProxyModelPrivate::removeSelectionFromProxy(const QItemSelection
+ }
+
+ --proxyEndRemove;
+- if (proxyEndRemove >= 0) {
++ if (proxyEndRemove >= proxyStartRemove) {
+ q->beginRemoveRows(QModelIndex(), proxyStartRemove, proxyEndRemove);
+
+ rootIt = m_rootIndexList.erase(rootRemoveStart, rootIt);
+@@ -1953,7 +1953,7 @@ void KSelectionProxyModelPrivate::insertSelectionIntoProxy(const QItemSelection
+
+ if (rowCount == 0) {
+ // Even if the newindex doesn't have any children to put into the model yet,
+- // We still need to make sure it's future children are inserted into the model.
++ // We still need to make sure its future children are inserted into the model.
+ m_rootIndexList.insert(rootListRow, newIndex);
+ if (!m_resetting || m_layoutChanging) {
+ emit q->rootIndexAdded(newIndex);
diff -Nru kitemmodels-5.28.0/debian/patches/KExtraColumnsProxyModel-Persist-model-indexes-after-emitt.patch kitemmodels-5.28.0/debian/patches/KExtraColumnsProxyModel-Persist-model-indexes-after-emitt.patch
--- kitemmodels-5.28.0/debian/patches/KExtraColumnsProxyModel-Persist-model-indexes-after-emitt.patch 1970-01-01 01:00:00.000000000 +0100
+++ kitemmodels-5.28.0/debian/patches/KExtraColumnsProxyModel-Persist-model-indexes-after-emitt.patch 2017-04-04 18:03:26.000000000 +0200
@@ -0,0 +1,65 @@
+From: David Faure <faure@kde.org>
+Date: Tue, 20 Dec 2016 11:16:35 +0100
+Subject: KExtraColumnsProxyModel: Persist model indexes after emitting
+ layoutChange, not before
+
+Same fix as Stephen Kelly's fix for QIdentityProxyModel in https://codereview.qt-project.org/180390
+
+I tried to write a unittest but QStandardItemModel doesn't support moveRow,
+and we don't have all the infrastructure here to force emitting a layoutChange
+with specific parents like in the Qt unittests.
+
+This commit, added to the 3 fixes in Qt, fixes crashes when moving folders in kmail.
+
+CCMAIL: steveire@gmail.com
+---
+ src/kextracolumnsproxymodel.cpp | 29 +++++++++++++++--------------
+ 1 file changed, 15 insertions(+), 14 deletions(-)
+
+diff --git a/src/kextracolumnsproxymodel.cpp b/src/kextracolumnsproxymodel.cpp
+index 53235b8..3626d26 100644
+--- a/src/kextracolumnsproxymodel.cpp
++++ b/src/kextracolumnsproxymodel.cpp
+@@ -271,6 +271,21 @@ int KExtraColumnsProxyModel::proxyColumnForExtraColumn(int extraColumn) const
+ void KExtraColumnsProxyModelPrivate::_ec_sourceLayoutAboutToBeChanged(const QList<QPersistentModelIndex> &sourceParents, QAbstractItemModel::LayoutChangeHint hint)
+ {
+ Q_Q(KExtraColumnsProxyModel);
++
++ QList<QPersistentModelIndex> parents;
++ parents.reserve(sourceParents.size());
++ foreach (const QPersistentModelIndex &parent, sourceParents) {
++ if (!parent.isValid()) {
++ parents << QPersistentModelIndex();
++ continue;
++ }
++ const QModelIndex mappedParent = q->mapFromSource(parent);
++ Q_ASSERT(mappedParent.isValid());
++ parents << mappedParent;
++ }
++
++ emit q->layoutAboutToBeChanged(parents, hint);
++
+ const QModelIndexList persistentIndexList = q->persistentIndexList();
+ layoutChangePersistentIndexes.reserve(persistentIndexList.size());
+ layoutChangeProxyColumns.reserve(persistentIndexList.size());
+@@ -287,20 +302,6 @@ void KExtraColumnsProxyModelPrivate::_ec_sourceLayoutAboutToBeChanged(const QLis
+ Q_ASSERT(srcPersistentIndex.isValid());
+ layoutChangePersistentIndexes << srcPersistentIndex;
+ }
+-
+- QList<QPersistentModelIndex> parents;
+- parents.reserve(sourceParents.size());
+- foreach (const QPersistentModelIndex &parent, sourceParents) {
+- if (!parent.isValid()) {
+- parents << QPersistentModelIndex();
+- continue;
+- }
+- const QModelIndex mappedParent = q->mapFromSource(parent);
+- Q_ASSERT(mappedParent.isValid());
+- parents << mappedParent;
+- }
+-
+- emit q->layoutAboutToBeChanged(parents, hint);
+ }
+
+ void KExtraColumnsProxyModelPrivate::_ec_sourceLayoutChanged(const QList<QPersistentModelIndex> &sourceParents, QAbstractItemModel::LayoutChangeHint hint)
diff -Nru kitemmodels-5.28.0/debian/patches/series kitemmodels-5.28.0/debian/patches/series
--- kitemmodels-5.28.0/debian/patches/series 1970-01-01 01:00:00.000000000 +0100
+++ kitemmodels-5.28.0/debian/patches/series 2017-04-04 18:03:26.000000000 +0200
@@ -0,0 +1,3 @@
+Fix-assert-in-beginRemoveRows-when-deselecting-an-empty-c.patch
+KExtraColumnsProxyModel-Persist-model-indexes-after-emitt.patch
+Update-proxies-for-recently-realised-class-of-bugs.patch
diff -Nru kitemmodels-5.28.0/debian/patches/Update-proxies-for-recently-realised-class-of-bugs.patch kitemmodels-5.28.0/debian/patches/Update-proxies-for-recently-realised-class-of-bugs.patch
--- kitemmodels-5.28.0/debian/patches/Update-proxies-for-recently-realised-class-of-bugs.patch 1970-01-01 01:00:00.000000000 +0100
+++ kitemmodels-5.28.0/debian/patches/Update-proxies-for-recently-realised-class-of-bugs.patch 2017-04-04 18:03:26.000000000 +0200
@@ -0,0 +1,110 @@
+From: Stephen Kelly <steveire@gmail.com>
+Date: Tue, 20 Dec 2016 22:40:22 +0000
+Subject: Update proxies for recently realised class of bugs.
+
+Store persistent indexes for updating after emitting any signals which
+can invoke user code.
+---
+ src/kconcatenaterowsproxymodel.cpp | 22 +++++++++++-----------
+ src/kdescendantsproxymodel.cpp | 4 ++--
+ src/kselectionproxymodel.cpp | 18 +++++++++---------
+ 3 files changed, 22 insertions(+), 22 deletions(-)
+
+diff --git a/src/kconcatenaterowsproxymodel.cpp b/src/kconcatenaterowsproxymodel.cpp
+index f3b0c10..465c09b 100644
+--- a/src/kconcatenaterowsproxymodel.cpp
++++ b/src/kconcatenaterowsproxymodel.cpp
+@@ -303,17 +303,6 @@ void KConcatenateRowsProxyModelPrivate::slotDataChanged(const QModelIndex &from,
+
+ void KConcatenateRowsProxyModelPrivate::slotSourceLayoutAboutToBeChanged(const QList<QPersistentModelIndex> &sourceParents, QAbstractItemModel::LayoutChangeHint hint)
+ {
+- const QModelIndexList persistentIndexList = q->persistentIndexList();
+- layoutChangePersistentIndexes.reserve(persistentIndexList.size());
+-
+- foreach (const QPersistentModelIndex &proxyPersistentIndex, persistentIndexList) {
+- proxyIndexes << proxyPersistentIndex;
+- Q_ASSERT(proxyPersistentIndex.isValid());
+- const QPersistentModelIndex srcPersistentIndex = q->mapToSource(proxyPersistentIndex);
+- Q_ASSERT(srcPersistentIndex.isValid());
+- layoutChangePersistentIndexes << srcPersistentIndex;
+- }
+-
+ QList<QPersistentModelIndex> parents;
+ parents.reserve(sourceParents.size());
+ foreach (const QPersistentModelIndex &parent, sourceParents) {
+@@ -327,6 +316,17 @@ void KConcatenateRowsProxyModelPrivate::slotSourceLayoutAboutToBeChanged(const Q
+ }
+
+ emit q->layoutAboutToBeChanged(parents, hint);
++
++ const QModelIndexList persistentIndexList = q->persistentIndexList();
++ layoutChangePersistentIndexes.reserve(persistentIndexList.size());
++
++ foreach (const QPersistentModelIndex &proxyPersistentIndex, persistentIndexList) {
++ proxyIndexes << proxyPersistentIndex;
++ Q_ASSERT(proxyPersistentIndex.isValid());
++ const QPersistentModelIndex srcPersistentIndex = q->mapToSource(proxyPersistentIndex);
++ Q_ASSERT(srcPersistentIndex.isValid());
++ layoutChangePersistentIndexes << srcPersistentIndex;
++ }
+ }
+
+ void KConcatenateRowsProxyModelPrivate::slotSourceLayoutChanged(const QList<QPersistentModelIndex> &sourceParents, QAbstractItemModel::LayoutChangeHint hint)
+diff --git a/src/kdescendantsproxymodel.cpp b/src/kdescendantsproxymodel.cpp
+index 810d3f1..5f758ff 100644
+--- a/src/kdescendantsproxymodel.cpp
++++ b/src/kdescendantsproxymodel.cpp
+@@ -901,6 +901,8 @@ void KDescendantsProxyModelPrivate::sourceLayoutAboutToBeChanged()
+ return;
+ }
+
++ q->layoutAboutToBeChanged();
++
+ QPersistentModelIndex srcPersistentIndex;
+ Q_FOREACH (const QPersistentModelIndex &proxyPersistentIndex, q->persistentIndexList()) {
+ m_proxyIndexes << proxyPersistentIndex;
+@@ -909,8 +911,6 @@ void KDescendantsProxyModelPrivate::sourceLayoutAboutToBeChanged()
+ Q_ASSERT(srcPersistentIndex.isValid());
+ m_layoutChangePersistentIndexes << srcPersistentIndex;
+ }
+-
+- q->layoutAboutToBeChanged();
+ }
+
+ void KDescendantsProxyModelPrivate::sourceLayoutChanged()
+diff --git a/src/kselectionproxymodel.cpp b/src/kselectionproxymodel.cpp
+index 873e974..d0ff4a1 100644
+--- a/src/kselectionproxymodel.cpp
++++ b/src/kselectionproxymodel.cpp
+@@ -767,15 +767,6 @@ void KSelectionProxyModelPrivate::sourceLayoutAboutToBeChanged()
+
+ emit q->layoutAboutToBeChanged();
+
+- QPersistentModelIndex srcPersistentIndex;
+- Q_FOREACH (const QPersistentModelIndex &proxyPersistentIndex, q->persistentIndexList()) {
+- m_proxyIndexes << proxyPersistentIndex;
+- Q_ASSERT(proxyPersistentIndex.isValid());
+- srcPersistentIndex = q->mapToSource(proxyPersistentIndex);
+- Q_ASSERT(srcPersistentIndex.isValid());
+- m_layoutChangePersistentIndexes << srcPersistentIndex;
+- }
+-
+ QItemSelection selection;
+ Q_FOREACH (const QModelIndex &rootIndex, m_rootIndexList) {
+ // This will be optimized later.
+@@ -786,6 +777,15 @@ void KSelectionProxyModelPrivate::sourceLayoutAboutToBeChanged()
+ selection = kNormalizeSelection(selection);
+ emit q->rootSelectionAboutToBeRemoved(selection);
+
++ QPersistentModelIndex srcPersistentIndex;
++ Q_FOREACH (const QPersistentModelIndex &proxyPersistentIndex, q->persistentIndexList()) {
++ m_proxyIndexes << proxyPersistentIndex;
++ Q_ASSERT(proxyPersistentIndex.isValid());
++ srcPersistentIndex = q->mapToSource(proxyPersistentIndex);
++ Q_ASSERT(srcPersistentIndex.isValid());
++ m_layoutChangePersistentIndexes << srcPersistentIndex;
++ }
++
+ m_rootIndexList.clear();
+ }
+
Reply to: