[SCM] KDE Development Platform Libraries module packaging branch, master, updated. debian/4.14.2-4-3-g055a581
Lisandro Damián Nicanor Pérez
lisandro at moszumanska.debian.org
Mon Jan 19 17:44:47 UTC 2015
Gitweb-URL: http://git.debian.org/?p=pkg-kde/kde-sc/kde4libs.git;a=commitdiff;h=055a581
The following commit has been merged in the master branch:
commit 055a581d7e0710347bec8e6f28ac095c8450bffa
Author: Sandro Knauß <bugs at sandroknauss.de>
Date: Mon Jan 19 14:42:41 2015 -0300
Add a patch to fix the recursive filter proxy model.
The net result was people not beeing able to see their mail and
thus considering it was lost.
---
debian/changelog | 8 +
...KRecursiveFilterProxyModel-Fixed-the-model.diff | 331 +++++++++++++++++++++
debian/patches/series | 1 +
3 files changed, 340 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index 121f278..67272d0 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+kde4libs (4:4.14.2-5) unstable; urgency=medium
+
+ * Team upload.
+ * Add debian/patches/KRecursiveFilterProxyModel-Fixed-the-model.diff
+ (Closes: #775114)
+
+ -- Sandro Knauß <bugs at sandroknauss.de> Mon, 19 Jan 2015 14:12:25 +0100
+
kde4libs (4:4.14.2-4) unstable; urgency=medium
* Team upload.
diff --git a/debian/patches/KRecursiveFilterProxyModel-Fixed-the-model.diff b/debian/patches/KRecursiveFilterProxyModel-Fixed-the-model.diff
new file mode 100644
index 0000000..5ebfa53
--- /dev/null
+++ b/debian/patches/KRecursiveFilterProxyModel-Fixed-the-model.diff
@@ -0,0 +1,331 @@
+From a932980cc7babe69613b9c6ad98faa4ec368258e Mon Sep 17 00:00:00 2001
+From: Christian Mollekopf <chrigi_1 at fastmail.fm>
+Date: Tue, 9 Sep 2014 18:16:37 +0200
+Subject: [PATCH] KRecursiveFilterProxyModel: Fixed the model
+
+The model was not working properly and didn't include all items under
+some circumstances.
+This patch fixes the following scenarios in particular:
+
+* The change in sourceDataChanged is required to fix the shortcut condition.
+The idea is that if the parent is already part of the model (it must be if acceptRow returns true),
+we can directly invoke dataChanged on the parent, resulting in the changed index
+getting reevaluated. However, because the recursive filterAcceptsRow version was used
+the shortcut was also used when only the current index matches the filter and
+the parent index is in fact not yet in the model. In this case we failed to call
+dataChanged on the right index and thus the complete branch was never added to the model.
+
+* The change in refreshAscendantMapping is required to include indexes that were
+included by descendants. The intended way how this was supposed to work is that we
+traverse the tree upwards and find the last index that is not yet part of the model.
+We would then call dataChanged on that index causing it and its descendants to get reevaluated.
+However, acceptRow does not reflect wether an index is already in the model or not.
+Consider the following model:
+
+- A
+ - B
+ - C
+ - D
+
+
+If C is include in the model by default but D not and A & B only gets included due to C, we have the following model:
+- A
+ - B
+ - C
+ - D
+
+If we then call refreshAscendantsMapping on D it will not consider B as already being part of the model.
+This results in the toplevel index A being considered lastAscendant, and a call to dataChanged on A results in
+a reevaluation of A only, which is already in the model. Thus D never gets added to the model.
+
+Unfortunately there is no way to probe QSortFilterProxyModel for indexes that are
+already part of the model. Even the const mapFromSource internally creates a mapping when called,
+and thus instead of revealing indexes that are not yet part of the model, it silently
+creates a mapping (without issuing the relevant signals!).
+
+As the only possible workaround we have to issues dataChanged for all ancestors
+which is ignored for indexes that are not yet mapped, and results in a rowsInserted
+signal for the correct indexes. It also results in superfluous dataChanged signals,
+since we don't know when to stop, but at least we have a properly behaving model
+this way.
+---
+ kdeui/itemviews/krecursivefilterproxymodel.cpp | 17 +-
+ kdeui/tests/CMakeLists.txt | 1 +
+ kdeui/tests/krecursivefilterproxymodeltest.cpp | 221 +++++++++++++++++++++++++
+ 3 files changed, 227 insertions(+), 12 deletions(-)
+ create mode 100644 kdeui/tests/krecursivefilterproxymodeltest.cpp
+
+--- a/kdeui/itemviews/krecursivefilterproxymodel.cpp
++++ b/kdeui/itemviews/krecursivefilterproxymodel.cpp
+@@ -126,7 +126,7 @@ void KRecursiveFilterProxyModelPrivate::
+
+ QModelIndex source_parent = source_top_left.parent();
+
+- if (!source_parent.isValid() || q->filterAcceptsRow(source_parent.row(), source_parent.parent()))
++ if (!source_parent.isValid() || q->acceptRow(source_parent.row(), source_parent.parent()))
+ {
+ invokeDataChanged(source_top_left, source_bottom_right);
+ return;
+@@ -149,24 +149,17 @@ void KRecursiveFilterProxyModelPrivate::
+ void KRecursiveFilterProxyModelPrivate::refreshAscendantMapping(const QModelIndex &index, bool refreshAll)
+ {
+ Q_Q(KRecursiveFilterProxyModel);
+-
+ Q_ASSERT(index.isValid());
+- QModelIndex lastAscendant = index;
+- QModelIndex sourceAscendant = index.parent();
++
++ QModelIndex sourceAscendant = index;
+ // We got a matching descendant, so find the right place to insert the row.
+ // We need to tell the QSortFilterProxyModel that the first child between an existing row in the model
+ // has changed data so that it will get a mapping.
+- while(sourceAscendant.isValid() && !q->acceptRow(sourceAscendant.row(), sourceAscendant.parent()))
++ while(sourceAscendant.isValid())
+ {
+- if (refreshAll)
+- invokeDataChanged(sourceAscendant, sourceAscendant);
+-
+- lastAscendant = sourceAscendant;
++ invokeDataChanged(sourceAscendant, sourceAscendant);
+ sourceAscendant = sourceAscendant.parent();
+ }
+-
+- // Inform the model that its data changed so that it creates new mappings and finds the rows which now match the filter.
+- invokeDataChanged(lastAscendant, lastAscendant);
+ }
+
+ void KRecursiveFilterProxyModelPrivate::sourceRowsAboutToBeInserted(const QModelIndex &source_parent, int start, int end)
+--- a/kdeui/tests/CMakeLists.txt
++++ b/kdeui/tests/CMakeLists.txt
+@@ -82,6 +82,7 @@ KDEUI_PROXYMODEL_TESTS(
+ kdescendantsproxymodeltest
+ kselectionproxymodeltest
+ testmodelqueuedconnections
++ krecursivefilterproxymodeltest
+ )
+
+ KDEUI_EXECUTABLE_TESTS(
+--- /dev/null
++++ b/kdeui/tests/krecursivefilterproxymodeltest.cpp
+@@ -0,0 +1,221 @@
++/*
++ Copyright (c) 2014 Christian Mollekopf <mollekopf at kolabsys.com>
++
++ This library is free software; you can redistribute it and/or modify it
++ under the terms of the GNU Library General Public License as published by
++ the Free Software Foundation; either version 2 of the License, or (at your
++ option) any later version.
++
++ This library is distributed in the hope that it will be useful, but WITHOUT
++ ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
++ FITNESS FOR A PARTICULAR PURPOSE. See the GNU Library General Public
++ License for more details.
++
++ You should have received a copy of the GNU Library General Public License
++ along with this library; see the file COPYING.LIB. If not, write to the
++ Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
++ 02110-1301, USA.
++*/
++
++
++#include <qtest_kde.h>
++
++#include <krecursivefilterproxymodel.h>
++#include <QStandardItemModel>
++
++class ModelSignalSpy : public QObject {
++ Q_OBJECT
++public:
++ explicit ModelSignalSpy(QAbstractItemModel &model) {
++ connect(&model, SIGNAL(rowsInserted(QModelIndex, int, int)), this, SLOT(onRowsInserted(QModelIndex,int,int)));
++ connect(&model, SIGNAL(rowsRemoved(QModelIndex, int, int)), this, SLOT(onRowsRemoved(QModelIndex,int,int)));
++ connect(&model, SIGNAL(rowsMoved(QModelIndex, int, int, QModelIndex, int)), this, SLOT(onRowsMoved(QModelIndex,int,int, QModelIndex, int)));
++ connect(&model, SIGNAL(dataChanged(QModelIndex,QModelIndex)), this, SLOT(onDataChanged(QModelIndex,QModelIndex)));
++ connect(&model, SIGNAL(layoutChanged()), this, SLOT(onLayoutChanged()));
++ connect(&model, SIGNAL(modelReset()), this, SLOT(onModelReset()));
++ }
++
++ QStringList mSignals;
++ QModelIndex parent;
++ int start;
++ int end;
++
++public Q_SLOTS:
++ void onRowsInserted(QModelIndex p, int s, int e) {
++ mSignals << QLatin1String("rowsInserted");
++ parent = p;
++ start = s;
++ end = e;
++ }
++ void onRowsRemoved(QModelIndex p, int s, int e) {
++ mSignals << QLatin1String("rowsRemoved");
++ parent = p;
++ start = s;
++ end = e;
++ }
++ void onRowsMoved(QModelIndex,int,int,QModelIndex,int) {
++ mSignals << QLatin1String("rowsMoved");
++ }
++ void onDataChanged(QModelIndex,QModelIndex) {
++ mSignals << QLatin1String("dataChanged");
++ }
++ void onLayoutChanged() {
++ mSignals << QLatin1String("layoutChanged");
++ }
++ void onModelReset() {
++ mSignals << QLatin1String("modelReset");
++ }
++};
++
++class TestModel : public KRecursiveFilterProxyModel
++{
++ Q_OBJECT
++public:
++ virtual bool acceptRow(int sourceRow, const QModelIndex &sourceParent) const
++ {
++ // qDebug() << sourceModel()->index(sourceRow, 0, sourceParent).data().toString() << sourceModel()->index(sourceRow, 0, sourceParent).data(Qt::UserRole+1).toBool();
++ return sourceModel()->index(sourceRow, 0, sourceParent).data(Qt::UserRole+1).toBool();
++ }
++};
++
++static QModelIndex getIndex(char *string, const QAbstractItemModel &model)
++{
++ QModelIndexList list = model.match(model.index(0, 0), Qt::DisplayRole, QString::fromLatin1(string), 1, Qt::MatchRecursive);
++ if (list.isEmpty()) {
++ return QModelIndex();
++ }
++ return list.first();
++}
++
++class KRecursiveFilterProxyModelTest : public QObject
++{
++ Q_OBJECT
++private:
++
++private slots:
++ // Requires the acceptRow fix in sourceDataChanged to pass
++ // Test that we properly react to a data-changed signal in a descendant and include all required rows
++ void testDataChange()
++ {
++ QStandardItemModel model;
++ TestModel proxy;
++ proxy.setSourceModel(&model);
++
++ QStandardItem *row1 = new QStandardItem("row1");
++ row1->setData(false);
++ model.appendRow(row1);
++
++ QCOMPARE(getIndex("row1", proxy).isValid(), false);
++
++ QStandardItem *subchild = new QStandardItem("subchild");
++ subchild->setData(false);
++ {
++ QStandardItem *child = new QStandardItem("child");
++ child->setData(false);
++ child->appendRow(subchild);
++ row1->appendRow(child);
++ }
++
++ ModelSignalSpy spy(proxy);
++ subchild->setData(true);
++
++ QCOMPARE(getIndex("row1", proxy).isValid(), true);
++ QCOMPARE(getIndex("child", proxy).isValid(), true);
++ QCOMPARE(getIndex("subchild", proxy).isValid(), true);
++
++ QCOMPARE(spy.mSignals, QStringList() << QLatin1String("rowsInserted"));
++ }
++
++ void testInsert()
++ {
++ QStandardItemModel model;
++ TestModel proxy;
++ proxy.setSourceModel(&model);
++
++ QStandardItem *row1 = new QStandardItem("row1");
++ row1->setData(false);
++ model.appendRow(row1);
++
++ QStandardItem *child = new QStandardItem("child");
++ child->setData(false);
++ row1->appendRow(child);
++
++ QStandardItem *child2 = new QStandardItem("child2");
++ child2->setData(false);
++ child->appendRow(child2);
++
++ QCOMPARE(getIndex("row1", proxy).isValid(), false);
++ QCOMPARE(getIndex("child", proxy).isValid(), false);
++ QCOMPARE(getIndex("child2", proxy).isValid(), false);
++
++ ModelSignalSpy spy(proxy);
++ {
++ QStandardItem *subchild = new QStandardItem("subchild");
++ subchild->setData(true);
++ child2->appendRow(subchild);
++ }
++
++ QCOMPARE(getIndex("row1", proxy).isValid(), true);
++ QCOMPARE(spy.mSignals, QStringList() << QLatin1String("rowsInserted"));
++ QCOMPARE(spy.parent, QModelIndex());
++ }
++
++
++ // We want to get child2 into the model which is a descendant of child.
++ // child is already in the model from the neighbor2 branch. We must ensure dataChange is called on child,
++ // so child2 is included in the model.
++ void testNeighborPath()
++ {
++ QStandardItemModel model;
++ TestModel proxy;
++ proxy.setSourceModel(&model);
++
++ QStandardItem *row1 = new QStandardItem("row1");
++ row1->setData(false);
++ model.appendRow(row1);
++
++ QStandardItem *child = new QStandardItem("child");
++ child->setData(false);
++ row1->appendRow(child);
++
++ QStandardItem *child2 = new QStandardItem("child2");
++ child2->setData(false);
++ child->appendRow(child2);
++
++ {
++ QStandardItem *nb1 = new QStandardItem("neighbor");
++ nb1->setData(false);
++ child->appendRow(nb1);
++
++ QStandardItem *nb2 = new QStandardItem("neighbor2");
++ nb2->setData(true);
++ nb1->appendRow(nb2);
++ }
++
++ //These tests affect the test. It seems without them the mapping is not created in qsortfilterproxymodel, resulting in the item
++ //simply getting added later on. With these the model doesn't react to the added subchild as it should. Piece of crap.
++ QCOMPARE(getIndex("child2", proxy).isValid(), false);
++ QCOMPARE(getIndex("child", proxy).isValid(), true);
++ QCOMPARE(getIndex("neighbor", proxy).isValid(), true);
++ QCOMPARE(getIndex("neighbor2", proxy).isValid(), true);
++
++ ModelSignalSpy spy(proxy);
++
++ {
++ qDebug() << "inserting";
++ QStandardItem *subchild = new QStandardItem("subchild");
++ subchild->setData(true);
++ child2->appendRow(subchild);
++ }
++
++ QCOMPARE(getIndex("child2", proxy).isValid(), true);
++ QCOMPARE(getIndex("subchild", proxy).isValid(), true);
++ //The dataChanged signals are not intentional and cause by refreshAscendantMapping. Unfortunately we can't avoid them.
++ QCOMPARE(spy.mSignals, QStringList() << QLatin1String("rowsInserted") << QLatin1String("dataChanged") << QLatin1String("dataChanged"));
++ }
++
++};
++
++QTEST_KDEMAIN(KRecursiveFilterProxyModelTest, NoGUI)
++
++#include "krecursivefilterproxymodeltest.moc"
diff --git a/debian/patches/series b/debian/patches/series
index aaed415..70a4135 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -29,3 +29,4 @@ disable_bogus_test.diff
disable_bogus_tests
fix_solidlex_destroy_signature.patch
allow_cancel_ssl.diff
+KRecursiveFilterProxyModel-Fixed-the-model.diff
--
KDE Development Platform Libraries module packaging
More information about the pkg-kde-commits
mailing list