AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QStringListModel: fix moveRows()

Browse Source 
QStringListMode::moveRows() had an issue when the destination was before
the source row.

Change-Id: Icf64e5b4cdd6a39faf3ba4ccc3883196b247ccbd
Reviewed-by: Luca Beldi <v.ronin@yahoo.it>
Reviewed-by: David Faure <david.faure@kdab.com>
This commit is contained in:
Christian Ehrlicher 2019-12-02 21:04:09 +01:00
parent be9398c8d4
commit 4e04132264
2 changed files with 36 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
src/corelib/itemmodels/qstringlistmodel.cpp
View File

@ -301,24 +301,23 @@ bool QStringListModel::moveRows(const QModelIndex &sourceParent, int sourceRow,
{ {
if (sourceRow < 0 if (sourceRow < 0
|| sourceRow + count - 1 >= rowCount(sourceParent) || sourceRow + count - 1 >= rowCount(sourceParent)
|| destinationChild <= 0 || destinationChild < 0
|| destinationChild > rowCount(destinationParent) || destinationChild > rowCount(destinationParent)
|| sourceRow == destinationChild
|| sourceRow == destinationChild - 1 || sourceRow == destinationChild - 1
|| count <= 0) { || count <= 0
|| sourceParent.isValid()
|| destinationParent.isValid()) {
return false; return false;
} }
if (!beginMoveRows(QModelIndex(), sourceRow, sourceRow + count - 1, QModelIndex(), destinationChild)) if (!beginMoveRows(QModelIndex(), sourceRow, sourceRow + count - 1, QModelIndex(), destinationChild))
return false; return false;
/*
QList::move assumes that the second argument is the index where the item will end up to int fromRow = sourceRow;
i.e. the valid range for that argument is from 0 to QList::size()-1 if (destinationChild < sourceRow)
QAbstractItemModel::moveRows when source and destinations have the same parent assumes that fromRow += count - 1;
the item will end up being in the row BEFORE the one indicated by destinationChild else
i.e. the valid range for that argument is from 1 to QList::size() destinationChild--;
For this reason we remove 1 from destinationChild when using it inside QList
*/
destinationChild--;
const int fromRow = destinationChild < sourceRow ? (sourceRow + count - 1) : sourceRow;
while (count--) while (count--)
lst.move(fromRow, destinationChild); lst.move(fromRow, destinationChild);
endMoveRows(); endMoveRows();

53
tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp
View File

@ -103,28 +103,25 @@ void tst_QStringListModel::moveRowsInvalid_data()
QTest::addColumn<QModelIndex>("destinationParent"); QTest::addColumn<QModelIndex>("destinationParent");
QTest::addColumn<int>("destination"); QTest::addColumn<int>("destination");
QStringListModel* tempModel = new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this); const auto createModel = [this]() {
QTest::addRow("destination_equal_source") << tempModel << QModelIndex() << 0 << 1 << QModelIndex() << 1; return new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this);
tempModel = new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this); };
QTest::addRow("count_equal_0") << tempModel << QModelIndex() << 0 << 0 << QModelIndex() << 2; constexpr int rowCount = 6;
tempModel = new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this);
QTest::addRow("destination_equal_source") << createModel() << QModelIndex() << 0 << 1 << QModelIndex() << 0;
QTest::addRow("count_equal_0") << createModel() << QModelIndex() << 0 << 0 << QModelIndex() << 2;
QStringListModel *tempModel = createModel();
QTest::addRow("move_child") << tempModel << tempModel->index(0, 0) << 0 << 1 << QModelIndex() << 2; QTest::addRow("move_child") << tempModel << tempModel->index(0, 0) << 0 << 1 << QModelIndex() << 2;
tempModel = new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this); tempModel = createModel();
QTest::addRow("move_to_child") << tempModel << QModelIndex() << 0 << 1 << tempModel->index(0, 0) << 2; QTest::addRow("move_to_child") << tempModel << QModelIndex() << 0 << 1 << tempModel->index(0, 0) << 2;
tempModel = new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this); QTest::addRow("negative_count") << createModel() << QModelIndex() << 0 << -1 << QModelIndex() << 2;
QTest::addRow("negative_count") << tempModel << QModelIndex() << 0 << -1 << QModelIndex() << 2; QTest::addRow("negative_source_row") << createModel() << QModelIndex() << -1 << 1 << QModelIndex() << 2;
tempModel = new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this); QTest::addRow("negative_destination_row") << createModel() << QModelIndex() << 0 << 1 << QModelIndex() << -1;
QTest::addRow("negative_source_row") << tempModel << QModelIndex() << -1 << 1 << QModelIndex() << 2; QTest::addRow("source_row_equal_rowCount") << createModel() << QModelIndex() << rowCount << 1 << QModelIndex() << 1;
tempModel = new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this); QTest::addRow("source_row_equal_destination_row") << createModel() << QModelIndex() << 2 << 1 << QModelIndex() << 2;
QTest::addRow("negative_destination_row") << tempModel << QModelIndex() << 0 << 1 << QModelIndex() << -1; QTest::addRow("source_row_equal_destination_row_plus_1") << createModel() << QModelIndex() << 2 << 1 << QModelIndex() << 3;
tempModel = new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this); QTest::addRow("destination_row_greater_rowCount") << createModel() << QModelIndex() << 0 << 1 << QModelIndex() << rowCount + 1;
QTest::addRow("source_row_equal_rowCount") << tempModel << QModelIndex() << tempModel->rowCount() << 1 << QModelIndex() << 1; QTest::addRow("move_row_within_source_range") << createModel() << QModelIndex() << 0 << 3 << QModelIndex() << 2;
tempModel = new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this);
QTest::addRow("destination_row_greater_rowCount") << tempModel << QModelIndex() << 0 << 1 << QModelIndex() << tempModel->rowCount() + 1;
tempModel = new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this);
QTest::addRow("move_row_within_source_range") << tempModel << QModelIndex() << 0 << 3 << QModelIndex() << 2;
tempModel = new QStringListModel(QStringList{"A", "B", "C", "D", "E", "F"}, this);
QTest::addRow("destination_row_before_0") << tempModel << QModelIndex() << 1 << 1 << QModelIndex() << 0;
} }
void tst_QStringListModel::moveRowsInvalid() void tst_QStringListModel::moveRowsInvalid()
@ -155,20 +152,20 @@ void tst_QStringListModel::moveRows_data()
QTest::newRow("1_Item_from_top_to_middle") << 0 << 1 << 3 << QStringList{"B", "C", "A", "D", "E", "F"}; QTest::newRow("1_Item_from_top_to_middle") << 0 << 1 << 3 << QStringList{"B", "C", "A", "D", "E", "F"};
QTest::newRow("1_Item_from_top_to_bottom") << 0 << 1 << 6 << QStringList{"B", "C", "D", "E", "F", "A"}; QTest::newRow("1_Item_from_top_to_bottom") << 0 << 1 << 6 << QStringList{"B", "C", "D", "E", "F", "A"};
QTest::newRow("1_Item_from_middle_to_top") << 2 << 1 << 1 << QStringList{"C", "A", "B", "D", "E", "F"}; QTest::newRow("1_Item_from_middle_to_top") << 2 << 1 << 0 << QStringList{"C", "A", "B", "D", "E", "F"};
QTest::newRow("1_Item_from_bottom_to_middle") << 5 << 1 << 3 << QStringList{"A", "B", "F", "C", "D", "E"}; QTest::newRow("1_Item_from_bottom_to_middle") << 5 << 1 << 2 << QStringList{"A", "B", "F", "C", "D", "E"};
QTest::newRow("1_Item_from_bottom to_top") << 5 << 1 << 1 << QStringList{"F", "A", "B", "C", "D", "E"}; QTest::newRow("1_Item_from_bottom to_top") << 5 << 1 << 0 << QStringList{"F", "A", "B", "C", "D", "E"};
QTest::newRow("1_Item_from_middle_to_bottom") << 2 << 1 << 6 << QStringList{"A", "B", "D", "E", "F", "C"}; QTest::newRow("1_Item_from_middle_to_bottom") << 2 << 1 << 6 << QStringList{"A", "B", "D", "E", "F", "C"};
QTest::newRow("1_Item_from_middle_to_middle_before") << 2 << 1 << 1 << QStringList{"C", "A", "B", "D", "E", "F"}; QTest::newRow("1_Item_from_middle_to_middle_before") << 2 << 1 << 1 << QStringList{"A", "C", "B", "D", "E", "F"};
QTest::newRow("1_Item_from_middle_to_middle_after") << 2 << 1 << 4 << QStringList{"A", "B", "D", "C", "E", "F"}; QTest::newRow("1_Item_from_middle_to_middle_after") << 2 << 1 << 4 << QStringList{"A", "B", "D", "C", "E", "F"};
QTest::newRow("2_Items_from_top_to_middle") << 0 << 2 << 3 << QStringList{"C", "A", "B", "D", "E", "F"}; QTest::newRow("2_Items_from_top_to_middle") << 0 << 2 << 3 << QStringList{"C", "A", "B", "D", "E", "F"};
QTest::newRow("2_Items_from_top_to_bottom") << 0 << 2 << 6 << QStringList{"C", "D", "E", "F", "A", "B"}; QTest::newRow("2_Items_from_top_to_bottom") << 0 << 2 << 6 << QStringList{"C", "D", "E", "F", "A", "B"};
QTest::newRow("2_Items_from_middle_to_top") << 2 << 2 << 1 << QStringList{"C", "D", "A", "B", "E", "F"}; QTest::newRow("2_Items_from_middle_to_top") << 2 << 2 << 0 << QStringList{"C", "D", "A", "B", "E", "F"};
QTest::newRow("2_Items_from_bottom_to_middle") << 4 << 2 << 3 << QStringList{"A", "B", "E", "F", "C", "D"}; QTest::newRow("2_Items_from_bottom_to_middle") << 4 << 2 << 2 << QStringList{"A", "B", "E", "F", "C", "D"};
QTest::newRow("2_Items_from_bottom_to_top") << 4 << 2 << 1 << QStringList{"E", "F", "A", "B", "C", "D"}; QTest::newRow("2_Items_from_bottom_to_top") << 4 << 2 << 0 << QStringList{"E", "F", "A", "B", "C", "D"};
QTest::newRow("2_Items_from_middle_to_bottom") << 2 << 2 << 6 << QStringList{"A", "B", "E", "F", "C", "D"}; QTest::newRow("2_Items_from_middle_to_bottom") << 2 << 2 << 6 << QStringList{"A", "B", "E", "F", "C", "D"};
QTest::newRow("2_Items_from_middle_to_middle_before") << 3 << 2 << 2 << QStringList{"A", "D", "E", "B", "C", "F"}; QTest::newRow("2_Items_from_middle_to_middle_before") << 3 << 2 << 1 << QStringList{"A", "D", "E", "B", "C", "F"};
QTest::newRow("2_Items_from_middle_to_middle_after") << 1 << 2 << 5 << QStringList{"A", "D", "E", "B", "C", "F"}; QTest::newRow("2_Items_from_middle_to_middle_after") << 1 << 2 << 5 << QStringList{"A", "D", "E", "B", "C", "F"};
} }