From 66acca3316e3c333e4cc3abfc343e58e3516c8b4 Mon Sep 17 00:00:00 2001 From: Wang ChunLin Date: Thu, 15 Oct 2020 11:34:46 +0800 Subject: [PATCH] Fix invalid QSortFilterProxyModel::dataChanged parameters QSortFilterModel shouldn't forward dataChanged() when the source model changes data in columns that the filter model refuses Fixes: QTBUG-86850 Pick-to: 5.15 Change-Id: I26565d119d2aa36ea07b3de0c15f1b137bc002f8 Reviewed-by: David Faure --- .../itemmodels/qsortfilterproxymodel.cpp | 22 ++- .../tst_qsortfilterproxymodel.cpp | 166 ++++++++++++++++++ .../tst_qsortfilterproxymodel.h | 2 + 3 files changed, 181 insertions(+), 9 deletions(-) diff --git a/src/corelib/itemmodels/qsortfilterproxymodel.cpp b/src/corelib/itemmodels/qsortfilterproxymodel.cpp index fb09c22798..2777709a80 100644 --- a/src/corelib/itemmodels/qsortfilterproxymodel.cpp +++ b/src/corelib/itemmodels/qsortfilterproxymodel.cpp @@ -1404,15 +1404,19 @@ void QSortFilterProxyModelPrivate::_q_sourceDataChanged(const QModelIndex &sourc while (source_left_column < source_bottom_right.column() && m->proxy_columns.at(source_left_column) == -1) ++source_left_column; - const QModelIndex proxy_top_left = create_index( - proxy_start_row, m->proxy_columns.at(source_left_column), it); - int source_right_column = source_bottom_right.column(); - while (source_right_column > source_top_left.column() - && m->proxy_columns.at(source_right_column) == -1) - --source_right_column; - const QModelIndex proxy_bottom_right = create_index( - proxy_end_row, m->proxy_columns.at(source_right_column), it); - emit q->dataChanged(proxy_top_left, proxy_bottom_right, roles); + if (m->proxy_columns.at(source_left_column) != -1) { + const QModelIndex proxy_top_left = create_index( + proxy_start_row, m->proxy_columns.at(source_left_column), it); + int source_right_column = source_bottom_right.column(); + while (source_right_column > source_top_left.column() + && m->proxy_columns.at(source_right_column) == -1) + --source_right_column; + if (m->proxy_columns.at(source_right_column) != -1) { + const QModelIndex proxy_bottom_right = create_index( + proxy_end_row, m->proxy_columns.at(source_right_column), it); + emit q->dataChanged(proxy_top_left, proxy_bottom_right, roles); + } + } } } diff --git a/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.cpp b/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.cpp index 44da534246..ca97543ee7 100644 --- a/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.cpp +++ b/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.cpp @@ -2192,6 +2192,172 @@ void tst_QSortFilterProxyModel::changeSourceDataProxySendDataChanged_qtbug87781( QCOMPARE(afterDataChangedSpy.size(), 1); } +void tst_QSortFilterProxyModel::changeSourceDataProxyFilterSingleColumn() +{ + enum modelRow { Row0, Row1, RowCount }; + enum modelColumn { Column0, Column1, Column2, Column3, Column4, Column5, ColumnCount }; + + class FilterProxyModel : public QSortFilterProxyModel + { + public: + bool filterAcceptsColumn(int source_column, const QModelIndex &source_parent) const override { + Q_UNUSED(source_parent); + switch (source_column) { + case Column2: + case Column4: + return true; + default: + return false; + } + } + }; + + QStandardItemModel model; + FilterProxyModel proxy; + proxy.setSourceModel(&model); + model.insertRows(0, RowCount); + model.insertColumns(0, ColumnCount); + + QSignalSpy modelDataChangedSpy(&model, &QSortFilterProxyModel::dataChanged); + QSignalSpy proxyDataChangedSpy(&proxy, &FilterProxyModel::dataChanged); + + QVERIFY(modelDataChangedSpy.isValid()); + QVERIFY(proxyDataChangedSpy.isValid()); + + modelDataChangedSpy.clear(); + proxyDataChangedSpy.clear(); + model.setData(model.index(Row0, Column1), QStringLiteral("new data"), Qt::DisplayRole); + QCOMPARE(modelDataChangedSpy.size(), 1); + QCOMPARE(proxyDataChangedSpy.size(), 0); + + modelDataChangedSpy.clear(); + proxyDataChangedSpy.clear(); + model.setData(model.index(Row0, Column2), QStringLiteral("new data"), Qt::DisplayRole); + QCOMPARE(modelDataChangedSpy.size(), 1); + QCOMPARE(proxyDataChangedSpy.size(), 1); + + modelDataChangedSpy.clear(); + proxyDataChangedSpy.clear(); + model.setData(model.index(Row0, Column3), QStringLiteral("new data"), Qt::DisplayRole); + QCOMPARE(modelDataChangedSpy.size(), 1); + QCOMPARE(proxyDataChangedSpy.size(), 0); + + modelDataChangedSpy.clear(); + proxyDataChangedSpy.clear(); + model.setData(model.index(Row0, Column4), QStringLiteral("new data"), Qt::DisplayRole); + QCOMPARE(modelDataChangedSpy.size(), 1); + QCOMPARE(proxyDataChangedSpy.size(), 1); + + modelDataChangedSpy.clear(); + proxyDataChangedSpy.clear(); + model.setData(model.index(Row0, Column5), QStringLiteral("new data"), Qt::DisplayRole); + QCOMPARE(modelDataChangedSpy.size(), 1); + QCOMPARE(proxyDataChangedSpy.size(), 0); +} + +void tst_QSortFilterProxyModel::changeSourceDataProxyFilterMultipleColumns() +{ + class FilterProxyModel : public QSortFilterProxyModel + { + public: + bool filterAcceptsColumn(int source_column, const QModelIndex &source_parent) const override { + Q_UNUSED(source_parent); + switch (source_column) { + case 2: + case 4: + return true; + default: + return false; + } + } + }; + + class MyTableModel : public QAbstractTableModel + { + public: + explicit MyTableModel() = default; + int rowCount(const QModelIndex &parent = QModelIndex()) const override { + Q_UNUSED(parent) + return 10; + } + int columnCount(const QModelIndex &parent = QModelIndex()) const override { + Q_UNUSED(parent) + return 10; + } + QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override { + Q_UNUSED(index) + Q_UNUSED(role) + return QString("testData"); + } + + void testDataChanged(const int topLeftRow, const int topLeftColumn, const int bottomRightRow, const int bottomRightColumn) { + QModelIndex topLeft = index(topLeftRow, topLeftColumn); + QModelIndex bottomRight = index(bottomRightRow, bottomRightColumn); + QVERIFY(topLeft.isValid()); + QVERIFY(bottomRight.isValid()); + emit dataChanged(topLeft, bottomRight); + } + }; + + MyTableModel baseModel; + FilterProxyModel proxyModel; + + proxyModel.setSourceModel(&baseModel); + + QSignalSpy baseModelDataChangedSpy(&baseModel, &MyTableModel::dataChanged); + QSignalSpy proxyModelDataChangedSpy(&proxyModel, &FilterProxyModel::dataChanged); + + connect(&proxyModel, &FilterProxyModel::dataChanged, [=](const QModelIndex &topLeft, const QModelIndex &bottomRight) { + QVERIFY(topLeft.isValid()); + QVERIFY(bottomRight.isValid()); + + //make sure every element is valid + int topLeftRow = topLeft.row(); + int topLeftColumn = topLeft.column(); + int bottomRightRow = bottomRight.row(); + int bottomRightColumn = bottomRight.column(); + for (int row = topLeftRow; row <= bottomRightRow; ++row) { + for (int column = topLeftColumn; column <= bottomRightColumn; ++column) { + QModelIndex index = topLeft.model()->index(row, column); + QVERIFY(index.isValid()); + } + } + }); + + QVERIFY(baseModelDataChangedSpy.isValid()); + QVERIFY(proxyModelDataChangedSpy.isValid()); + + baseModelDataChangedSpy.clear(); + proxyModelDataChangedSpy.clear(); + baseModel.testDataChanged(0, 0, 1, 1); + QCOMPARE(baseModelDataChangedSpy.size(), 1); + QCOMPARE(proxyModelDataChangedSpy.size(), 0); + + baseModelDataChangedSpy.clear(); + proxyModelDataChangedSpy.clear(); + baseModel.testDataChanged(0, 0, 1, 2); + QCOMPARE(baseModelDataChangedSpy.size(), 1); + QCOMPARE(proxyModelDataChangedSpy.size(), 1); + + baseModelDataChangedSpy.clear(); + proxyModelDataChangedSpy.clear(); + baseModel.testDataChanged(0, 3, 1, 3); + QCOMPARE(baseModelDataChangedSpy.size(), 1); + QCOMPARE(proxyModelDataChangedSpy.size(), 0); + + baseModelDataChangedSpy.clear(); + proxyModelDataChangedSpy.clear(); + baseModel.testDataChanged(0, 3, 1, 5); + QCOMPARE(baseModelDataChangedSpy.size(), 1); + QCOMPARE(proxyModelDataChangedSpy.size(), 1); + + baseModelDataChangedSpy.clear(); + proxyModelDataChangedSpy.clear(); + baseModel.testDataChanged(0, 0, 1, 5); + QCOMPARE(baseModelDataChangedSpy.size(), 1); + QCOMPARE(proxyModelDataChangedSpy.size(), 1); +} + void tst_QSortFilterProxyModel::sortFilterRole() { QStandardItemModel model; diff --git a/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.h b/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.h index b0f4137082..3fca4aad28 100644 --- a/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.h +++ b/tests/auto/corelib/itemmodels/qsortfilterproxymodel_common/tst_qsortfilterproxymodel.h @@ -91,6 +91,8 @@ private slots: void changeSourceDataKeepsStableSorting_qtbug1548(); void changeSourceDataForwardsRoles_qtbug35440(); void changeSourceDataProxySendDataChanged_qtbug87781(); + void changeSourceDataProxyFilterSingleColumn(); + void changeSourceDataProxyFilterMultipleColumns(); void resortingDoesNotBreakTreeModels(); void dynamicFilterWithoutSort(); void sortFilterRole();