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 (8021e2d5e7).
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 <david.faure@kdab.com>
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
This commit is contained in:
Christian Ehrlicher 2017-12-17 19:56:35 +01:00
parent 67b1fa48be
commit a02b371eb2
4 changed files with 42 additions and 12 deletions

View File

@ -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();

View File

@ -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)
};

View File

@ -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;

View File

@ -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());