summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorOlivier Goffart <ogoffart@trolltech.com>2009-06-10 16:42:35 (GMT)
committerOlivier Goffart <ogoffart@trolltech.com>2009-06-11 10:57:35 (GMT)
commite0be99974d5d66f548543df9b5b919c8e81ade62 (patch)
tree598eb58063f035968b3c8d918eb7f6194cf47cb0
parenta6eac57ee46c150aabd5cff331d6859e7999680f (diff)
downloadQt-e0be99974d5d66f548543df9b5b919c8e81ade62.zip
Qt-e0be99974d5d66f548543df9b5b919c8e81ade62.tar.gz
Qt-e0be99974d5d66f548543df9b5b919c8e81ade62.tar.bz2
Invalid QPersistentIndexes after QStandardItem::takeRow
We need the parent of each potential QPersistentModelIndex in order to cleanup when removing the rows. They need not to change in order QSortFilterProxyModel maping to be still valid. takeRow must not change the internal data before calling beginRemoveRow. Same thing for takeColumn Task-number: 255652 Reviewed-by: Thierry Reviewed-by: Leo
-rw-r--r--src/gui/itemviews/qstandarditemmodel.cpp36
-rw-r--r--tests/auto/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp99
-rw-r--r--tests/auto/qstandarditemmodel/tst_qstandarditemmodel.cpp49
3 files changed, 148 insertions, 36 deletions
diff --git a/src/gui/itemviews/qstandarditemmodel.cpp b/src/gui/itemviews/qstandarditemmodel.cpp
index 9d0f796..30daed2 100644
--- a/src/gui/itemviews/qstandarditemmodel.cpp
+++ b/src/gui/itemviews/qstandarditemmodel.cpp
@@ -1742,18 +1742,21 @@ QList<QStandardItem*> QStandardItem::takeRow(int row)
Q_D(QStandardItem);
if ((row < 0) || (row >= rowCount()))
return QList<QStandardItem*>();
+ if (d->model)
+ d->model->d_func()->rowsAboutToBeRemoved(this, row, row);
QList<QStandardItem*> items;
int index = d->childIndex(row, 0);
- for (int column = 0; column < d->columnCount(); ++column) {
- QStandardItem *ch = d->children.at(index);
- if (ch) {
+ int col_count = d->columnCount();
+ for (int column = 0; column < col_count; ++column) {
+ QStandardItem *ch = d->children.at(index + column);
+ if (ch)
ch->d_func()->setParentAndModel(0, 0);
- d->children.replace(index, 0);
- }
items.append(ch);
- ++index;
}
- removeRow(row);
+ d->children.remove(index, col_count);
+ d->rows--;
+ if (d->model)
+ d->model->d_func()->rowsRemoved(this, row, 1);
return items;
}
@@ -1769,18 +1772,21 @@ QList<QStandardItem*> QStandardItem::takeColumn(int column)
Q_D(QStandardItem);
if ((column < 0) || (column >= columnCount()))
return QList<QStandardItem*>();
+ if (d->model)
+ d->model->d_func()->columnsAboutToBeRemoved(this, column, column);
QList<QStandardItem*> items;
- int index = d->childIndex(0, column);
- for (int row = 0; row < d->rowCount(); ++row) {
+
+ for (int row = d->rowCount() - 1; row >= 0; --row) {
+ int index = d->childIndex(row, column);
QStandardItem *ch = d->children.at(index);
- if (ch) {
+ if (ch)
ch->d_func()->setParentAndModel(0, 0);
- d->children.replace(index, 0);
- }
- items.append(ch);
- index += d->columnCount();
+ d->children.remove(index);
+ items.prepend(ch);
}
- removeColumn(column);
+ d->columns--;
+ if (d->model)
+ d->model->d_func()->columnsRemoved(this, column, 1);
return items;
}
diff --git a/tests/auto/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp b/tests/auto/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
index d7c231f..274b177 100644
--- a/tests/auto/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
+++ b/tests/auto/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
@@ -134,6 +134,7 @@ private slots:
void task250023_fetchMore();
void task251296_hiddenChildren();
void task252507_mapFromToSource();
+ void task255652_removeRowsRecursive();
protected:
void buildHierarchy(const QStringList &data, QAbstractItemModel *model);
@@ -390,7 +391,7 @@ void tst_QSortFilterProxyModel::sort()
QModelIndex index = m_proxy->index(row, 0, QModelIndex());
QCOMPARE(m_proxy->data(index, Qt::DisplayRole).toString(), initial.at(row));
}
-
+
}
void tst_QSortFilterProxyModel::sortHierarchy_data()
@@ -411,7 +412,7 @@ void tst_QSortFilterProxyModel::sortHierarchy_data()
<< static_cast<int>(Qt::AscendingOrder)
<< (QStringList() << "a" << "<" << "b" << "<" << "c" << ">" << ">")
<< (QStringList() << "a" << "<" << "b" << "<" << "c" << ">" << ">");
-
+
#if 1
QTest::newRow("hierarchical ascending")
<< static_cast<int>(Qt::AscendingOrder)
@@ -571,7 +572,7 @@ void tst_QSortFilterProxyModel::insertRows()
QModelIndex index = m_proxy->index(row, 0, QModelIndex());
QCOMPARE(m_proxy->data(index, Qt::DisplayRole).toString(), initial.at(row));
}
-
+
// insert the row
m_proxy->insertRows(position, insert.count(), QModelIndex());
QCOMPARE(m_model->rowCount(QModelIndex()), expected.count());
@@ -615,7 +616,7 @@ void tst_QSortFilterProxyModel::prependRow()
QStandardItem sub2("sub2");
sub2.appendRow(new QStandardItem("sub3"));
item.insertRow(0, &sub2);
-
+
QModelIndex index_sub2 = proxy.mapFromSource(model.indexFromItem(&sub2));
QCOMPARE(sub2.rowCount(), proxy.rowCount(index_sub2));
@@ -626,7 +627,7 @@ void tst_QSortFilterProxyModel::prependRow()
/*
void tst_QSortFilterProxyModel::insertColumns_data()
{
-
+
}
void tst_QSortFilterProxyModel::insertColumns()
@@ -923,7 +924,7 @@ void tst_QSortFilterProxyModel::removeRows()
// prepare model
foreach (QString s, initial)
model.appendRow(new QStandardItem(s));
-
+
// remove the rows
QCOMPARE(proxy.removeRows(position, count, QModelIndex()), success);
QCOMPARE(model.rowCount(QModelIndex()), expectedSource.count());
@@ -1164,10 +1165,10 @@ void tst_QSortFilterProxyModel::removeColumns()
proxy.setSourceModel(&model);
if (!filter.isEmpty())
proxy.setFilterRegExp(QRegExp(filter));
-
+
// prepare model
model.setHorizontalHeaderLabels(initial);
-
+
// remove the columns
QCOMPARE(proxy.removeColumns(position, count, QModelIndex()), success);
QCOMPARE(model.columnCount(QModelIndex()), expectedSource.count());
@@ -1266,7 +1267,7 @@ void tst_QSortFilterProxyModel::filter_data()
<< "golf"
<< "quebec"
<< "foxtrot"
- << "india"
+ << "india"
<< "romeo"
<< "november"
<< "oskar"
@@ -1274,9 +1275,9 @@ void tst_QSortFilterProxyModel::filter_data()
<< "kilo"
<< "whiskey"
<< "mike"
- << "papa"
+ << "papa"
<< "sierra"
- << "xray"
+ << "xray"
<< "viktor")
<< (QStringList()
<< "delta"
@@ -1334,7 +1335,7 @@ void tst_QSortFilterProxyModel::filterHierarchy_data()
<< "foo" << "boo" << "baz" << "moo" << "laa" << "haa")
<< (QStringList()
<< "foo" << "boo" << "moo");
-
+
QTest::newRow("simple hierarchy") << "b.*z"
<< (QStringList() << "baz" << "<" << "boz" << "<" << "moo" << ">" << ">")
<< (QStringList() << "baz" << "<" << "boz" << ">");
@@ -1671,7 +1672,7 @@ void tst_QSortFilterProxyModel::removeSourceRows()
QSignalSpy insertSpy(&proxy, SIGNAL(rowsInserted(QModelIndex, int, int)));
QSignalSpy aboutToRemoveSpy(&proxy, SIGNAL(rowsAboutToBeRemoved(QModelIndex, int, int)));
QSignalSpy aboutToInsertSpy(&proxy, SIGNAL(rowsAboutToBeInserted(QModelIndex, int, int)));
-
+
model.removeRows(start, count, QModelIndex());
QCOMPARE(aboutToRemoveSpy.count(), expectedRemovedProxyIntervals.count());
@@ -1827,7 +1828,7 @@ void tst_QSortFilterProxyModel::changeFilter()
QFETCH(IntPairList, finalRemoveIntervals);
QFETCH(IntPairList, insertIntervals);
QFETCH(QStringList, finalProxyItems);
-
+
QStandardItemModel model;
QSortFilterProxyModel proxy;
@@ -1880,7 +1881,7 @@ void tst_QSortFilterProxyModel::changeFilter()
#ifdef Q_OS_IRIX
QEXPECT_FAIL("filter (2)", "Not reliable on IRIX", Abort);
-#endif
+#endif
QCOMPARE(finalInsertSpy.count(), insertIntervals.count());
for (int i = 0; i < finalInsertSpy.count(); ++i) {
QList<QVariant> args = finalInsertSpy.at(i);
@@ -2068,7 +2069,7 @@ void tst_QSortFilterProxyModel::sortFilterRole()
model.setData(index, sourceItems.at(i).first, Qt::DisplayRole);
model.setData(index, sourceItems.at(i).second, Qt::UserRole);
}
-
+
proxy.setFilterRegExp("2");
QCOMPARE(proxy.rowCount(), 0); // Qt::DisplayRole is default role
@@ -2358,7 +2359,7 @@ void tst_QSortFilterProxyModel::sourceInsertRows()
model.insertColumns(0, 1, parent);
model.insertRows(0, 1, parent);
}
-
+
model.insertRows(0, 1, QModelIndex());
model.insertRows(0, 1, QModelIndex());
@@ -2462,7 +2463,7 @@ void tst_QSortFilterProxyModel::task236755_hiddenColumns()
QVERIFY(view.isColumnHidden(0));
}
-void tst_QSortFilterProxyModel::task247867_insertRowsSort()
+void tst_QSortFilterProxyModel::task247867_insertRowsSort()
{
QStandardItemModel model(2,2);
QSortFilterProxyModel proxyModel;
@@ -2567,7 +2568,7 @@ void tst_QSortFilterProxyModel::task248868_dynamicSorting()
QStringList initial2 = initial;
initial2.replaceInStrings("bateau", "girafe");
model1.setStringList(initial2); //this will cause a reset
-
+
QStringList expected2 = initial2;
expected2.sort();
@@ -2648,7 +2649,7 @@ class QtTestModel: public QAbstractItemModel
{
if (!idx.isValid())
return QVariant();
-
+
if (role == Qt::DisplayRole) {
if (idx.row() < 0 || idx.column() < 0 || idx.column() >= cols || idx.row() >= rows) {
wrongIndex = true;
@@ -2756,5 +2757,63 @@ void tst_QSortFilterProxyModel::task252507_mapFromToSource()
QCOMPARE(proxy.mapFromSource(proxy.index(6, 2)), QModelIndex());
}
+static QStandardItem *addEntry(QStandardItem* pParent, const QString &description)
+{
+ QStandardItem* pItem = new QStandardItem(description);
+ pParent->appendRow(pItem);
+ return pItem;
+}
+
+
+void tst_QSortFilterProxyModel::task255652_removeRowsRecursive()
+{
+ QStandardItemModel pModel;
+ QStandardItem *pItem1 = new QStandardItem("root");
+ pModel.appendRow(pItem1);
+ QList<QStandardItem *> items;
+
+ QStandardItem *pItem11 = addEntry(pItem1,"Sub-heading");
+ items << pItem11;
+ QStandardItem *pItem111 = addEntry(pItem11,"A");
+ items << pItem111;
+ items << addEntry(pItem111,"A1");
+ items << addEntry(pItem111,"A2");
+ QStandardItem *pItem112 = addEntry(pItem11,"B");
+ items << pItem112;
+ items << addEntry(pItem112,"B1");
+ items << addEntry(pItem112,"B2");
+ QStandardItem *pItem1123 = addEntry(pItem112,"B3");
+ items << pItem1123;
+ items << addEntry(pItem1123,"B3-");
+
+ QSortFilterProxyModel proxy;
+ proxy.setSourceModel(&pModel);
+
+ QList<QPersistentModelIndex> sourceIndexes;
+ QList<QPersistentModelIndex> proxyIndexes;
+ foreach (QStandardItem *item, items) {
+ QModelIndex idx = item->index();
+ sourceIndexes << idx;
+ proxyIndexes << proxy.mapFromSource(idx);
+ }
+
+ foreach (const QPersistentModelIndex &pidx, sourceIndexes)
+ QVERIFY(pidx.isValid());
+ foreach (const QPersistentModelIndex &pidx, proxyIndexes)
+ QVERIFY(pidx.isValid());
+
+ QList<QStandardItem*> itemRow = pItem1->takeRow(0);
+
+ QCOMPARE(itemRow.count(), 1);
+ QCOMPARE(itemRow.first(), pItem11);
+
+ foreach (const QPersistentModelIndex &pidx, sourceIndexes)
+ QVERIFY(!pidx.isValid());
+ foreach (const QPersistentModelIndex &pidx, proxyIndexes)
+ QVERIFY(!pidx.isValid());
+
+ delete pItem11;
+}
+
QTEST_MAIN(tst_QSortFilterProxyModel)
#include "tst_qsortfilterproxymodel.moc"
diff --git a/tests/auto/qstandarditemmodel/tst_qstandarditemmodel.cpp b/tests/auto/qstandarditemmodel/tst_qstandarditemmodel.cpp
index 84bda92..8067554 100644
--- a/tests/auto/qstandarditemmodel/tst_qstandarditemmodel.cpp
+++ b/tests/auto/qstandarditemmodel/tst_qstandarditemmodel.cpp
@@ -135,6 +135,7 @@ private slots:
void rootItemFlags();
void treeDragAndDrop();
+ void removeRowsAndColumns();
private:
QAbstractItemModel *m_model;
@@ -1403,7 +1404,7 @@ bool tst_QStandardItemModel::compareItems(QStandardItem *item1, QStandardItem *i
return true;
if (!item1 || !item2)
return false;
- if (item1->text() != item2->text()){
+ if (item1->text() != item2->text()){
qDebug() << item1->text() << item2->text();
return false;
}
@@ -1606,6 +1607,52 @@ void tst_QStandardItemModel::treeDragAndDrop()
}
}
+void tst_QStandardItemModel::removeRowsAndColumns()
+{
+#define VERIFY_MODEL \
+ for (int c = 0; c < col_list.count(); c++) \
+ for (int r = 0; r < row_list.count(); r++) \
+ QCOMPARE(model.item(r,c)->text() , row_list[r] + "x" + col_list[c]);
+
+ QVector<QString> row_list = QString("1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20").split(',').toVector();
+ QVector<QString> col_list = row_list;
+ QStandardItemModel model;
+ for (int c = 0; c < col_list.count(); c++)
+ for (int r = 0; r < row_list.count(); r++)
+ model.setItem(r, c, new QStandardItem(row_list[r] + "x" + col_list[c]));
+ VERIFY_MODEL
+
+ row_list.remove(3);
+ model.removeRow(3);
+ VERIFY_MODEL
+
+ col_list.remove(5);
+ model.removeColumn(5);
+ VERIFY_MODEL
+
+ row_list.remove(2, 5);
+ model.removeRows(2, 5);
+ VERIFY_MODEL
+
+ col_list.remove(1, 6);
+ model.removeColumns(1, 6);
+ VERIFY_MODEL
+
+ QList<QStandardItem *> row_taken = model.takeRow(6);
+ QCOMPARE(row_taken.count(), col_list.count());
+ for (int c = 0; c < col_list.count(); c++)
+ QCOMPARE(row_taken[c]->text() , row_list[6] + "x" + col_list[c]);
+ row_list.remove(6);
+ VERIFY_MODEL
+
+ QList<QStandardItem *> col_taken = model.takeColumn(10);
+ QCOMPARE(col_taken.count(), row_list.count());
+ for (int r = 0; r < row_list.count(); r++)
+ QCOMPARE(col_taken[r]->text() , row_list[r] + "x" + col_list[10]);
+ col_list.remove(10);
+ VERIFY_MODEL
+}
+
QTEST_MAIN(tst_QStandardItemModel)
#include "tst_qstandarditemmodel.moc"