[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#859633: marked as done (unblock: kitemmodels/5.28.0-2)



Your message dated Wed, 05 Apr 2017 12:19:00 +0000
with message-id <05e98f89-8b6b-71b5-b1d3-e33a6ffded12@thykier.net>
and subject line Re: Bug#859633: unblock: kitemmodels/5.28.0-2
has caused the Debian Bug report #859633,
regarding unblock: kitemmodels/5.28.0-2
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
859633: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=859633
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message ---
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();
+ }
+ 

--- End Message ---
--- Begin Message ---
Maximiliano Curia:
> 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
> 
> [...]

Unblocked, thanks.

~Niels

--- End Message ---

Reply to: