From a02b371eb2f34bf198c95cf0caa1fbad639f96bb Mon Sep 17 00:00:00 2001 From: Christian Ehrlicher Date: Sun, 17 Dec 2017 19:56:35 +0100 Subject: QHeaderView: properly connect rows/columnsMoved QHeaderViewPrivate reimplemented _q_layoutChanged() to handle changes of rows/columns via layoutChanged/layoutAboutToBeChanged. This worked fine for Qt4 but since Qt5 only the special signals rowsAboutToBeMoved/ rowsMoved are used for this (8021e2d5e7ccd09146896f788441c116f2ca6159). With this change, QAbstractItemViewPrivate::_q_rows/columnsMoved() is calling the virtual function _q_layoutChanged(). This resulted in a wrong call of QHP::_q_layoutChanged() for a horizontal header when a row changed and for a vertical header during a column change. In the end this can lead to an unhide of hidden sections. Task-number: QTBUG-54610 Change-Id: Ide4bfc5b24a97746fd1e5af82d3ba08257149157 Reviewed-by: David Faure Reviewed-by: Olivier Goffart (Woboq GmbH) --- src/widgets/itemviews/qheaderview.cpp | 28 +++++++++++++++------- src/widgets/itemviews/qheaderview.h | 3 ++- src/widgets/itemviews/qheaderview_p.h | 4 ++-- .../widgets/itemviews/qtreeview/tst_qtreeview.cpp | 19 ++++++++++++++- 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/widgets/itemviews/qheaderview.cpp b/src/widgets/itemviews/qheaderview.cpp index c7966f624f..abd5997ea1 100644 --- a/src/widgets/itemviews/qheaderview.cpp +++ b/src/widgets/itemviews/qheaderview.cpp @@ -361,7 +361,9 @@ void QHeaderView::setModel(QAbstractItemModel *model) QObject::disconnect(d->model, SIGNAL(columnsRemoved(QModelIndex,int,int)), this, SLOT(_q_sectionsRemoved(QModelIndex,int,int))); QObject::disconnect(d->model, SIGNAL(columnsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), - this, SLOT(_q_layoutAboutToBeChanged())); + this, SLOT(_q_sectionsAboutToBeChanged())); + QObject::disconnect(d->model, SIGNAL(columnsMoved(QModelIndex,int,int,QModelIndex,int)), + this, SLOT(_q_sectionsChanged())); } else { QObject::disconnect(d->model, SIGNAL(rowsInserted(QModelIndex,int,int)), this, SLOT(sectionsInserted(QModelIndex,int,int))); @@ -370,12 +372,16 @@ void QHeaderView::setModel(QAbstractItemModel *model) QObject::disconnect(d->model, SIGNAL(rowsRemoved(QModelIndex,int,int)), this, SLOT(_q_sectionsRemoved(QModelIndex,int,int))); QObject::disconnect(d->model, SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), - this, SLOT(_q_layoutAboutToBeChanged())); + this, SLOT(_q_sectionsAboutToBeChanged())); + QObject::disconnect(d->model, SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)), + this, SLOT(_q_sectionsChanged())); } QObject::disconnect(d->model, SIGNAL(headerDataChanged(Qt::Orientation,int,int)), this, SLOT(headerDataChanged(Qt::Orientation,int,int))); QObject::disconnect(d->model, SIGNAL(layoutAboutToBeChanged()), - this, SLOT(_q_layoutAboutToBeChanged())); + this, SLOT(_q_sectionsAboutToBeChanged())); + QObject::disconnect(d->model, SIGNAL(layoutChanged()), + this, SLOT(_q_sectionsChanged())); } if (model && model != QAbstractItemModelPrivate::staticEmptyModel()) { @@ -387,7 +393,9 @@ void QHeaderView::setModel(QAbstractItemModel *model) QObject::connect(model, SIGNAL(columnsRemoved(QModelIndex,int,int)), this, SLOT(_q_sectionsRemoved(QModelIndex,int,int))); QObject::connect(model, SIGNAL(columnsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), - this, SLOT(_q_layoutAboutToBeChanged())); + this, SLOT(_q_sectionsAboutToBeChanged())); + QObject::connect(model, SIGNAL(columnsMoved(QModelIndex,int,int,QModelIndex,int)), + this, SLOT(_q_sectionsChanged())); } else { QObject::connect(model, SIGNAL(rowsInserted(QModelIndex,int,int)), this, SLOT(sectionsInserted(QModelIndex,int,int))); @@ -396,12 +404,16 @@ void QHeaderView::setModel(QAbstractItemModel *model) QObject::connect(model, SIGNAL(rowsRemoved(QModelIndex,int,int)), this, SLOT(_q_sectionsRemoved(QModelIndex,int,int))); QObject::connect(model, SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), - this, SLOT(_q_layoutAboutToBeChanged())); + this, SLOT(_q_sectionsAboutToBeChanged())); + QObject::connect(model, SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)), + this, SLOT(_q_sectionsChanged())); } QObject::connect(model, SIGNAL(headerDataChanged(Qt::Orientation,int,int)), this, SLOT(headerDataChanged(Qt::Orientation,int,int))); QObject::connect(model, SIGNAL(layoutAboutToBeChanged()), - this, SLOT(_q_layoutAboutToBeChanged())); + this, SLOT(_q_sectionsAboutToBeChanged())); + QObject::connect(model, SIGNAL(layoutChanged()), + this, SLOT(_q_sectionsChanged())); } d->state = QHeaderViewPrivate::NoClear; @@ -2062,7 +2074,7 @@ void QHeaderViewPrivate::_q_sectionsRemoved(const QModelIndex &parent, viewport->update(); } -void QHeaderViewPrivate::_q_layoutAboutToBeChanged() +void QHeaderViewPrivate::_q_sectionsAboutToBeChanged() { //if there is no row/column we can't have mapping for columns //because no QModelIndex in the model would be valid @@ -2082,7 +2094,7 @@ void QHeaderViewPrivate::_q_layoutAboutToBeChanged() : model->index(logicalIndex(i), 0, root)); } -void QHeaderViewPrivate::_q_layoutChanged() +void QHeaderViewPrivate::_q_sectionsChanged() { Q_Q(QHeaderView); viewport->update(); diff --git a/src/widgets/itemviews/qheaderview.h b/src/widgets/itemviews/qheaderview.h index b6554c0730..ec00d16d10 100644 --- a/src/widgets/itemviews/qheaderview.h +++ b/src/widgets/itemviews/qheaderview.h @@ -250,7 +250,8 @@ protected: private: Q_PRIVATE_SLOT(d_func(), void _q_sectionsRemoved(const QModelIndex &parent, int logicalFirst, int logicalLast)) - Q_PRIVATE_SLOT(d_func(), void _q_layoutAboutToBeChanged()) + Q_PRIVATE_SLOT(d_func(), void _q_sectionsAboutToBeChanged()) + Q_PRIVATE_SLOT(d_func(), void _q_sectionsChanged()) Q_DECLARE_PRIVATE(QHeaderView) Q_DISABLE_COPY(QHeaderView) }; diff --git a/src/widgets/itemviews/qheaderview_p.h b/src/widgets/itemviews/qheaderview_p.h index 8fc8b88aa5..3b8e61af75 100644 --- a/src/widgets/itemviews/qheaderview_p.h +++ b/src/widgets/itemviews/qheaderview_p.h @@ -120,8 +120,8 @@ public: void updateHiddenSections(int logicalFirst, int logicalLast); void resizeSections(QHeaderView::ResizeMode globalMode, bool useGlobalMode = false); void _q_sectionsRemoved(const QModelIndex &,int,int); - void _q_layoutAboutToBeChanged(); - void _q_layoutChanged() override; + void _q_sectionsAboutToBeChanged(); + void _q_sectionsChanged(); bool isSectionSelected(int section) const; bool isFirstVisibleSection(int section) const; diff --git a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp index f870605d7d..5293ba487a 100644 --- a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp +++ b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp @@ -312,6 +312,12 @@ public: return QVariant(); } + void simulateMoveRows() + { + beginMoveRows(QModelIndex(), 0, 0, QModelIndex(), 2); + endMoveRows(); + } + void removeLastRow() { beginRemoveRows(QModelIndex(), rows - 1, rows - 1); @@ -1327,6 +1333,17 @@ void tst_QTreeView::columnHidden() for (int c = 0; c < model.columnCount(); ++c) QCOMPARE(view.isColumnHidden(c), true); view.update(); + + // QTBUG 54610 + // QAbstractItemViewPrivate::_q_layoutChanged() is called on + // rows/columnMoved and because this function is virtual, + // QHeaderViewPrivate::_q_layoutChanged() was called and unhided + // all sections because QHeaderViewPrivate::_q_layoutAboutToBeChanged() + // could not fill persistentHiddenSections (and is not needed) + view.hideColumn(model.cols - 1); + QCOMPARE(view.isColumnHidden(model.cols - 1), true); + model.simulateMoveRows(); + QCOMPARE(view.isColumnHidden(model.cols - 1), true); } void tst_QTreeView::rowHidden() @@ -4601,7 +4618,7 @@ void tst_QTreeView::fetchMoreOnScroll() tw.setModel(&im); tw.show(); tw.expandAll(); - QTest::qWaitForWindowActive(&tw); + QVERIFY(QTest::qWaitForWindowActive(&tw)); // Now we can allow the fetch to happen im.canFetchReady = true; tw.verticalScrollBar()->setValue(tw.verticalScrollBar()->maximum()); -- cgit v1.2.3