From f568bfce641f52b4641b5d8281c99742f1ae6f40 Mon Sep 17 00:00:00 2001 From: Christian Ehrlicher Date: Sun, 23 Dec 2018 17:59:18 +0100 Subject: [PATCH] QTreeView: fix keyboard navigation when first or last item is disabled The keyboard navigation did not consider the disabled state when trying to find the new index under all circumstances. This lead to a non-working PageUp/Down/Home/End navigation when the first or last item was disabled or hidden. Fix it by explicitly checking if the calculated item is hidden/enabled and skip it in this case. Fixes: QTBUG-44746 Fixes: QTBUG-34832 Change-Id: Ifa3b64a405e67b792db5db9d186d426fcfe183fb Reviewed-by: David Faure --- src/widgets/itemviews/qtreeview.cpp | 30 +++++++++++++++++-- src/widgets/itemviews/qtreeview_p.h | 2 ++ .../itemviews/qtreeview/tst_qtreeview.cpp | 24 +++++++++++++++ .../itemviews/qtreewidget/tst_qtreewidget.cpp | 28 +++++++++++++++++ 4 files changed, 81 insertions(+), 3 deletions(-) diff --git a/src/widgets/itemviews/qtreeview.cpp b/src/widgets/itemviews/qtreeview.cpp index f37da86539..3fad6ca3df 100644 --- a/src/widgets/itemviews/qtreeview.cpp +++ b/src/widgets/itemviews/qtreeview.cpp @@ -2301,9 +2301,9 @@ QModelIndex QTreeView::moveCursor(CursorAction cursorAction, Qt::KeyboardModifie case MovePageDown: return d->modelIndex(d->pageDown(vi), current.column()); case MoveHome: - return d->model->index(0, current.column(), d->root); + return d->modelIndex(d->itemForKeyHome(), current.column()); case MoveEnd: - return d->modelIndex(d->viewItems.count() - 1, current.column()); + return d->modelIndex(d->itemForKeyEnd(), current.column()); } return current; } @@ -3422,7 +3422,11 @@ int QTreeViewPrivate::pageUp(int i) const int index = itemAtCoordinate(coordinateForItem(i) - viewport->height()); while (isItemHiddenOrDisabled(index)) index--; - return index == -1 ? 0 : index; + if (index == -1) + index = 0; + while (isItemHiddenOrDisabled(index)) + index++; + return index >= viewItems.count() ? 0 : index; } int QTreeViewPrivate::pageDown(int i) const @@ -3430,6 +3434,26 @@ int QTreeViewPrivate::pageDown(int i) const int index = itemAtCoordinate(coordinateForItem(i) + viewport->height()); while (isItemHiddenOrDisabled(index)) index++; + if (index == -1 || index >= viewItems.count()) + index = viewItems.count() - 1; + while (isItemHiddenOrDisabled(index)) + index--; + return index == -1 ? viewItems.count() - 1 : index; +} + +int QTreeViewPrivate::itemForKeyHome() const +{ + int index = 0; + while (isItemHiddenOrDisabled(index)) + index++; + return index >= viewItems.count() ? 0 : index; +} + +int QTreeViewPrivate::itemForKeyEnd() const +{ + int index = viewItems.count() - 1; + while (isItemHiddenOrDisabled(index)) + index--; return index == -1 ? viewItems.count() - 1 : index; } diff --git a/src/widgets/itemviews/qtreeview_p.h b/src/widgets/itemviews/qtreeview_p.h index 0a0e7aae87..9666a9f8c2 100644 --- a/src/widgets/itemviews/qtreeview_p.h +++ b/src/widgets/itemviews/qtreeview_p.h @@ -140,6 +140,8 @@ public: int pageUp(int item) const; int pageDown(int item) const; + int itemForKeyHome() const; + int itemForKeyEnd() const; int itemHeight(int item) const; int indentationForItem(int item) const; diff --git a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp index e452efff07..ece2e9a220 100644 --- a/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp +++ b/tests/auto/widgets/itemviews/qtreeview/tst_qtreeview.cpp @@ -4135,6 +4135,30 @@ void tst_QTreeView::keyboardNavigationWithDisabled() QCOMPARE(view.currentIndex(), model.index(12, 0)); QTest::keyClick(view.viewport(), Qt::Key_Up); QCOMPARE(view.currentIndex(), model.index(6, 0)); + // QTBUG-44746 - when first/last item is disabled, + // Key_PageUp/Down/Home/End will not work as expected. + model.item(0)->setEnabled(false); + model.item(1)->setEnabled(true); + model.item(2)->setEnabled(true); + model.item(model.rowCount() - 1)->setEnabled(false); + model.item(model.rowCount() - 2)->setEnabled(true); + model.item(model.rowCount() - 3)->setEnabled(true); + // PageUp + view.setCurrentIndex(model.index(2, 0)); + QCOMPARE(view.currentIndex(), model.index(2, 0)); + QTest::keyClick(view.viewport(), Qt::Key_PageUp); + QCOMPARE(view.currentIndex(), model.index(1, 0)); + // PageDown + view.setCurrentIndex(model.index(model.rowCount() - 3, 0)); + QCOMPARE(view.currentIndex(), model.index(model.rowCount() - 3, 0)); + QTest::keyClick(view.viewport(), Qt::Key_PageDown); + QCOMPARE(view.currentIndex(), model.index(model.rowCount() - 2, 0)); + // Key_Home + QTest::keyClick(view.viewport(), Qt::Key_Home); + QCOMPARE(view.currentIndex(), model.index(1, 0)); + // Key_End + QTest::keyClick(view.viewport(), Qt::Key_End); + QCOMPARE(view.currentIndex(), model.index(model.rowCount() - 2, 0)); } class RemoveColumnOne : public QSortFilterProxyModel diff --git a/tests/auto/widgets/itemviews/qtreewidget/tst_qtreewidget.cpp b/tests/auto/widgets/itemviews/qtreewidget/tst_qtreewidget.cpp index ccae4ad626..74a2cc71ac 100644 --- a/tests/auto/widgets/itemviews/qtreewidget/tst_qtreewidget.cpp +++ b/tests/auto/widgets/itemviews/qtreewidget/tst_qtreewidget.cpp @@ -99,6 +99,7 @@ private slots: void insertTopLevelItems_data(); void insertTopLevelItems(); void keyboardNavigation(); + void keyboardNavigationWithHidden(); void scrollToItem(); void setSortingEnabled(); void match(); @@ -1575,6 +1576,33 @@ void tst_QTreeWidget::keyboardNavigation() } } +void tst_QTreeWidget::keyboardNavigationWithHidden() +{ + QTreeWidget tw; + for (int i = 0; i < 1000; ++i) + tw.addTopLevelItem(new QTreeWidgetItem({QString::number(i), QStringLiteral("second col")})); + // QTBUG-34832 - when first/last item is hidden, + // Key_PageUp/Down/Home/End will not work as expected. + tw.topLevelItem(0)->setHidden(true); + tw.topLevelItem(tw.model()->rowCount() - 1)->setHidden(true); + // PageUp + tw.setCurrentIndex(tw.model()->index(2, 0)); + QCOMPARE(tw.currentIndex(), tw.model()->index(2, 0)); + QTest::keyClick(tw.viewport(), Qt::Key_PageUp); + QCOMPARE(tw.currentIndex(), tw.model()->index(1, 0)); + // PageDown + tw.setCurrentIndex(tw.model()->index(tw.model()->rowCount() - 3, 0)); + QCOMPARE(tw.currentIndex(), tw.model()->index(tw.model()->rowCount() - 3, 0)); + QTest::keyClick(tw.viewport(), Qt::Key_PageDown); + QCOMPARE(tw.currentIndex(), tw.model()->index(tw.model()->rowCount() - 2, 0)); + // Key_Home + QTest::keyClick(tw.viewport(), Qt::Key_Home); + QCOMPARE(tw.currentIndex(), tw.model()->index(1, 0)); + // Key_End + QTest::keyClick(tw.viewport(), Qt::Key_End); + QCOMPARE(tw.currentIndex(), tw.model()->index(tw.model()->rowCount() - 2, 0)); +} + void tst_QTreeWidget::scrollToItem() { // Check if all parent nodes of the item found are expanded.