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 <qt_ci_bot@qt-project.org>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

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<int>{};
     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()