From dc5e8aa81c7083659e2ec3090489f34072ebc383 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Fri, 28 May 2021 13:03:34 +0200 Subject: QAbstractItemView: don't toggle extended selection on Ctrl+Press In ExtendedSelection mode, a Ctrl+Press might be both the start of a selection toggle, or the start of a Ctrl+Drag operation. If we already toggle on the press, then it's impossible to drag the existing selection while the Control key is pressed. Ignore Ctrl+Press events and let the corresponding release event toggle the selection. Adjust the relevant test cases accordingly. The QItemDelegate test case used a click+control event incorrectly, such an event doesn't change the clicked state and should not be eaten, and now it does change the selection, so fix the test. Task-number: QTBUG-59888 Change-Id: Ia76126e31c28bc97d3e93e54965bdb1d0b8ac6a4 Reviewed-by: Qt CI Bot Reviewed-by: Lars Knoll --- src/widgets/itemviews/qabstractitemview.cpp | 9 +++++++++ .../qabstractitemview/tst_qabstractitemview.cpp | 23 +++++++++++++++++++--- .../itemviews/qitemdelegate/tst_qitemdelegate.cpp | 14 +++++++++---- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/src/widgets/itemviews/qabstractitemview.cpp b/src/widgets/itemviews/qabstractitemview.cpp index 6799aa4ea6..f4f41e6f68 100644 --- a/src/widgets/itemviews/qabstractitemview.cpp +++ b/src/widgets/itemviews/qabstractitemview.cpp @@ -4028,6 +4028,11 @@ QItemSelectionModel::SelectionFlags QAbstractItemViewPrivate::extendedSelectionC return QItemSelectionModel::Clear; if (!index.isValid()) return QItemSelectionModel::NoUpdate; + // since the press might start a drag, deselect only on release + if (controlKeyPressed && !rightButtonPressed && pressedAlreadySelected + && dragEnabled && isIndexDragEnabled(index)) { + return QItemSelectionModel::NoUpdate; + } break; } case QEvent::MouseButtonRelease: { @@ -4040,6 +4045,10 @@ QItemSelectionModel::SelectionFlags QAbstractItemViewPrivate::extendedSelectionC || !index.isValid()) && state != QAbstractItemView::DragSelectingState && !shiftKeyPressed && !controlKeyPressed && (!rightButtonPressed || !index.isValid())) return QItemSelectionModel::ClearAndSelect|selectionBehaviorFlags(); + if (index == pressedIndex && controlKeyPressed && !rightButtonPressed + && dragEnabled && isIndexDragEnabled(index)) { + break; + } return QItemSelectionModel::NoUpdate; } case QEvent::KeyPress: { diff --git a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp index 37fa192a05..e7637f8013 100644 --- a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp +++ b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp @@ -2850,11 +2850,11 @@ void tst_QAbstractItemView::mouseSelection_data() // Extended selection: press with Ctrl toggles item QTest::addRow("Extended:Press,Toggle") << QAbstractItemView::ExtendedSelection << false << QList{SelectionEvent(SelectionEvent::Press, 3), - SelectionEvent(SelectionEvent::Press, Qt::ControlModifier, 3)} + SelectionEvent(SelectionEvent::Click, Qt::ControlModifier, 3)} << QList{}; QTest::addRow("Extended:Press,Add") << QAbstractItemView::ExtendedSelection << false << QList{SelectionEvent(SelectionEvent::Press, 1), - SelectionEvent(SelectionEvent::Press, Qt::ControlModifier, 3)} + SelectionEvent(SelectionEvent::Click, Qt::ControlModifier, 3)} << QList{1, 3}; // Extended selection: Shift creates a range between first and last pressed QTest::addRow("Extended:Press,Range") << QAbstractItemView::ExtendedSelection << false @@ -2871,6 +2871,15 @@ void tst_QAbstractItemView::mouseSelection_data() << QList{SelectionEvent(SelectionEvent::Press, 2), SelectionEvent(SelectionEvent::Move, 5)} << QList{2, 3, 4, 5}; + // Extended: Ctrl+Press-dragging extends the selection + QTest::addRow("Extended:Press,Drag;Ctrl-Press,Drag") << QAbstractItemView::ExtendedSelection << false + << QList{SelectionEvent(SelectionEvent::Press, 2), + SelectionEvent(SelectionEvent::Move, 5), + SelectionEvent(SelectionEvent::Release), + SelectionEvent(SelectionEvent::Press, Qt::ControlModifier, 6), + SelectionEvent(SelectionEvent::Move, Qt::ControlModifier, 8), + SelectionEvent(SelectionEvent::Release, Qt::ControlModifier, 8)} + << QList{2, 3, 4, 5, 6, 7, 8}; // Extended: Ctrl+Press-dragging in a selection should not deselect #QTBUG-59888 QTest::addRow("Extended:Ctrl-Drag selection") << QAbstractItemView::ExtendedSelection << true << QList{SelectionEvent(SelectionEvent::Click, 2), @@ -2880,6 +2889,15 @@ void tst_QAbstractItemView::mouseSelection_data() // two moves needed because of distance and state logic in QAbstractItemView SelectionEvent(SelectionEvent::Move, Qt::ControlModifier, 6)} << QList{2, 3, 4, 5}; + // Extended: Ctrl+Press-dragging with a selection extends, then drags #QTBUG-59888 + QTest::addRow("Extended:Ctrl-Drag selection") << QAbstractItemView::ExtendedSelection << true + << QList{SelectionEvent(SelectionEvent::Click, 2), + SelectionEvent(SelectionEvent::Click, Qt::ShiftModifier, 5), + SelectionEvent(SelectionEvent::Press, Qt::ControlModifier, 6), + SelectionEvent(SelectionEvent::Move, Qt::ControlModifier, 7), + // two moves needed because of distance and state logic in 7QAbstractItemView + SelectionEvent(SelectionEvent::Move, Qt::ControlModifier, 8)} + << QList{2, 3, 4, 5, 6}; } void tst_QAbstractItemView::mouseSelection() @@ -2950,7 +2968,6 @@ void tst_QAbstractItemView::mouseSelection() actualSelected << index.row(); QEXPECT_FAIL("Multi:Press-Drag selection", "QTBUG-59889", Continue); - QEXPECT_FAIL("Extended:Ctrl-Drag selection", "QTBUG-59889", Continue); QCOMPARE(actualSelected, selectedRows); } diff --git a/tests/auto/widgets/itemviews/qitemdelegate/tst_qitemdelegate.cpp b/tests/auto/widgets/itemviews/qitemdelegate/tst_qitemdelegate.cpp index 25212373a4..8d68ca7ac9 100644 --- a/tests/auto/widgets/itemviews/qitemdelegate/tst_qitemdelegate.cpp +++ b/tests/auto/widgets/itemviews/qitemdelegate/tst_qitemdelegate.cpp @@ -1394,6 +1394,7 @@ void tst_QItemDelegate::QTBUG4435_keepSelectionOnCheck() } QTableView view; view.setModel(&model); + view.setSelectionMode(QAbstractItemView::MultiSelection); view.setItemDelegate(new TestItemDelegate(&view)); view.show(); view.selectAll(); @@ -1404,11 +1405,16 @@ void tst_QItemDelegate::QTBUG4435_keepSelectionOnCheck() option.features = QStyleOptionViewItem::HasDisplay | QStyleOptionViewItem::HasCheckIndicator; option.checkState = Qt::CheckState(model.index(0, 0).data(Qt::CheckStateRole).toInt()); const int checkMargin = qApp->style()->pixelMetric(QStyle::PM_FocusFrameHMargin, 0, 0) + 1; - QPoint pos = qApp->style()->subElementRect(QStyle::SE_ItemViewItemCheckIndicator, &option, 0).center() - + QPoint(checkMargin, 0); - QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::ControlModifier, pos); - QTRY_VERIFY(view.selectionModel()->isColumnSelected(0, QModelIndex())); + QRect checkRect = qApp->style()->subElementRect(QStyle::SE_ItemViewItemCheckIndicator, &option, 0); + checkRect.translate(checkMargin, 0); + // click into the check mark checks, but doesn't change selection + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, checkRect.center()); QCOMPARE(model.item(0)->checkState(), Qt::Checked); + QTRY_VERIFY(view.selectionModel()->isColumnSelected(0, QModelIndex())); + // click outside the check mark doesn't check, and changes selection + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, + checkRect.center() + QPoint(checkRect.width(), 0)); + QTRY_VERIFY(!view.selectionModel()->isColumnSelected(0, QModelIndex())); } void tst_QItemDelegate::comboBox() -- cgit v1.2.3