QQuickTableView: support unselecting cells

As it stood, SelectionRectangle and TableView didn't support
unselecting cells by ctrl-dragging or ctrl-clicking a selected
cell. This is considered a bug. This patch will therefore make
sure that we support doing that.

In order to implement this, we need to provide the held keyboard
modifiers to startSelection(), so that TableView can choose if the
selected cells should become selected or unselected in the
selection model.

By making this change, it also became clear that some of the auto
tests didn't call startSelection() before setSelectionStartPos().
We therefore now also assert that we always do this, and fix up the
failing auto tests that violated this rule.

Fixes: QTBUG-121143
Pick-to: 6.7 6.6 6.5
Change-Id: Icd58b551234f3c6145165771de599e46b62014dc
Reviewed-by: Santhosh Kumar <santhosh.kumar.selvaraj@qt.io>

4 files changed, 62 insertions, 18 deletions

diff --git a/src/quick/items/qquickselectable_p.h b/src/quick/items/qquickselectable_p.h
index 475660ab2d..dbee64c49e 100644
--- a/src/quick/items/qquickselectable_p.h
+++ b/src/quick/items/qquickselectable_p.h
@@ -30,7 +30,7 @@ public:
 
     virtual QQuickItem *selectionPointerHandlerTarget() const = 0;
 
-    virtual bool startSelection(const QPointF &pos) = 0;
+    virtual bool startSelection(const QPointF &pos, Qt::KeyboardModifiers modifiers) = 0;
     virtual void setSelectionStartPos(const QPointF &pos) = 0;
     virtual void setSelectionEndPos(const QPointF &pos) = 0;
     virtual void clearSelection() = 0;
diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index 4d971525d4..6203f6f3c1 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -1663,10 +1663,15 @@ QQuickItem *QQuickTableViewPrivate::selectionPointerHandlerTarget() const
     return const_cast<QQuickTableView *>(q_func())->contentItem();
 }
 
-bool QQuickTableViewPrivate::startSelection(const QPointF &pos)
+bool QQuickTableViewPrivate::startSelection(const QPointF &pos, Qt::KeyboardModifiers modifiers)
 {
     Q_Q(QQuickTableView);
-    Q_UNUSED(pos);
+    if (!selectionModel) {
+        if (warnNoSelectionModel)
+            qmlWarning(q_func()) << "Cannot start selection: no SelectionModel assigned!";
+        warnNoSelectionModel = false;
+        return false;
+    }
 
     if (selectionBehavior == QQuickTableView::SelectionDisabled) {
         qmlWarning(q) << "Cannot start selection: TableView.selectionBehavior == TableView.SelectionDisabled";
@@ -1684,6 +1689,18 @@ bool QQuickTableViewPrivate::startSelection(const QPointF &pos)
     else if (selectionModel)
         existingSelection = selectionModel->selection();
 
+    // If pos is on top of an unselected cell, we start a session where the user selects which
+    // cells to become selected. Otherwise, if pos is on top of an already selected cell and
+    // ctrl is being held, we start a session where the user selects which selected cells to
+    // become unselected.
+    selectionFlag = QItemSelectionModel::Select;
+    if (modifiers & Qt::ControlModifier) {
+        QPoint startCell = clampedCellAtPos(pos);
+        const QModelIndex startIndex = q->index(startCell.y(), startCell.x());
+        if (selectionModel->isSelected(startIndex))
+            selectionFlag = QItemSelectionModel::Deselect;
+    }
+
     selectionStartCell = QPoint(-1, -1);
     selectionEndCell = QPoint(-1, -1);
     q->closeEditor();
@@ -1693,6 +1710,7 @@ bool QQuickTableViewPrivate::startSelection(const QPointF &pos)
 void QQuickTableViewPrivate::setSelectionStartPos(const QPointF &pos)
 {
     Q_Q(QQuickTableView);
+    Q_ASSERT(selectionFlag != QItemSelectionModel::NoUpdate);
     if (loadedItems.isEmpty())
         return;
     if (!selectionModel) {
@@ -1749,6 +1767,7 @@ void QQuickTableViewPrivate::setSelectionStartPos(const QPointF &pos)
 
 void QQuickTableViewPrivate::setSelectionEndPos(const QPointF &pos)
 {
+    Q_ASSERT(selectionFlag != QItemSelectionModel::NoUpdate);
     if (loadedItems.isEmpty())
         return;
     if (!selectionModel) {
@@ -1854,11 +1873,19 @@ void QQuickTableViewPrivate::updateSelection(const QRect &oldSelection, const QR
         deselect.merge(QItemSelection(startIndex, endIndex), QItemSelectionModel::Select);
     }
 
-    // Don't clear the selection that existed before the user started a new selection block
-    deselect.merge(existingSelection, QItemSelectionModel::Deselect);
-
-    selectionModel->select(deselect, QItemSelectionModel::Deselect);
-    selectionModel->select(select, QItemSelectionModel::Select);
+    if (selectionFlag == QItemSelectionModel::Select) {
+        // Don't clear the selection that existed before the user started a new selection block
+        deselect.merge(existingSelection, QItemSelectionModel::Deselect);
+        selectionModel->select(deselect, QItemSelectionModel::Deselect);
+        selectionModel->select(select, QItemSelectionModel::Select);
+    } else if (selectionFlag == QItemSelectionModel::Deselect){
+        QItemSelection oldSelection = existingSelection;
+        oldSelection.merge(select, QItemSelectionModel::Deselect);
+        selectionModel->select(oldSelection, QItemSelectionModel::Select);
+        selectionModel->select(select, QItemSelectionModel::Deselect);
+    } else {
+        Q_UNREACHABLE();
+    }
 }
 
 void QQuickTableViewPrivate::cancelSelectionTracking()
@@ -1867,6 +1894,7 @@ void QQuickTableViewPrivate::cancelSelectionTracking()
     selectionStartCell = QPoint(-1, -1);
     selectionEndCell = QPoint(-1, -1);
     existingSelection.clear();
+    selectionFlag = QItemSelectionModel::NoUpdate;
     if (selectableCallbackFunction)
         selectableCallbackFunction(QQuickSelectable::CallBackFlag::CancelSelection);
 }
@@ -5053,7 +5081,6 @@ bool QQuickTableViewPrivate::setCurrentIndexFromKeyEvent(QKeyEvent *e)
 
     const QModelIndex currentIndex = selectionModel->currentIndex();
     const QPoint currentCell = q->cellAtIndex(currentIndex);
-    const bool select = (e->modifiers() & Qt::ShiftModifier) && (e->key() != Qt::Key_Backtab);
 
     if (!q->activeFocusOnTab()) {
         switch (e->key()) {
@@ -5085,21 +5112,30 @@ bool QQuickTableViewPrivate::setCurrentIndexFromKeyEvent(QKeyEvent *e)
     }
 
     auto beginMoveCurrentIndex = [&](){
-        if (!select) {
+        const bool shouldSelect = (e->modifiers() & Qt::ShiftModifier) && (e->key() != Qt::Key_Backtab);
+        const bool startNewSelection = selectionRectangle().isEmpty();
+        if (!shouldSelect) {
             clearSelection();
-        } else if (selectionRectangle().isEmpty()) {
+            cancelSelectionTracking();
+        } else if (startNewSelection) {
+            // Try to start a new selection if no selection exists from before.
+            // The startSelection() call is theoretically allowed to refuse, although this
+            // is less likely when starting a selection using the keyboard.
             const int serializedStartIndex = modelIndexToCellIndex(selectionModel->currentIndex());
             if (loadedItems.contains(serializedStartIndex)) {
                 const QRectF startGeometry = loadedItems.value(serializedStartIndex)->geometry();
-                setSelectionStartPos(startGeometry.center());
-                if (selectableCallbackFunction)
-                    selectableCallbackFunction(QQuickSelectable::CallBackFlag::SelectionRectangleChanged);
+                if (startSelection(startGeometry.center(), Qt::ShiftModifier)) {
+                    setSelectionStartPos(startGeometry.center());
+                    if (selectableCallbackFunction)
+                        selectableCallbackFunction(QQuickSelectable::CallBackFlag::SelectionRectangleChanged);
+                }
             }
         }
     };
 
     auto endMoveCurrentIndex = [&](const QPoint &cell){
-        if (select) {
+        const bool isSelecting = selectionFlag != QItemSelectionModel::NoUpdate;
+        if (isSelecting) {
             if (polishScheduled)
                 forceLayout(true);
             const int serializedEndIndex = modelIndexAtCell(cell);
diff --git a/src/quick/items/qquicktableview_p_p.h b/src/quick/items/qquicktableview_p_p.h
index 1f0c8549a5..867e03485a 100644
--- a/src/quick/items/qquicktableview_p_p.h
+++ b/src/quick/items/qquicktableview_p_p.h
@@ -360,6 +360,7 @@ public:
     QPointer<QItemSelectionModel> selectionModel;
     QQuickTableView::SelectionBehavior selectionBehavior = QQuickTableView::SelectCells;
     QQuickTableView::SelectionMode selectionMode = QQuickTableView::ExtendedSelection;
+    QItemSelectionModel::SelectionFlag selectionFlag = QItemSelectionModel::NoUpdate;
     std::function<void(CallBackFlag)> selectableCallbackFunction;
     bool inSelectionModelUpdate = false;
 
@@ -582,7 +583,7 @@ public:
 
     // QQuickSelectable
     QQuickItem *selectionPointerHandlerTarget() const override;
-    bool startSelection(const QPointF &pos) override;
+    bool startSelection(const QPointF &pos, Qt::KeyboardModifiers modifiers) override;
     void setSelectionStartPos(const QPointF &pos) override;
     void setSelectionEndPos(const QPointF &pos) override;
     void clearSelection() override;
diff --git a/src/quick/items/qquicktreeview.cpp b/src/quick/items/qquicktreeview.cpp
index f2613b4f48..033884d58d 100644
--- a/src/quick/items/qquicktreeview.cpp
+++ b/src/quick/items/qquicktreeview.cpp
@@ -369,8 +369,15 @@ void QQuickTreeViewPrivate::updateSelection(const QRect &oldSelection, const QRe
             deselect.merge(QItemSelection(index, index), QItemSelectionModel::Select);
     }
 
-    selectionModel->select(deselect, QItemSelectionModel::Deselect);
-    selectionModel->select(select, QItemSelectionModel::Select);
+    if (selectionFlag == QItemSelectionModel::Select) {
+        selectionModel->select(deselect, QItemSelectionModel::Deselect);
+        selectionModel->select(select, QItemSelectionModel::Select);
+    } else {
+        QItemSelection oldSelection = existingSelection;
+        oldSelection.merge(select, QItemSelectionModel::Deselect);
+        selectionModel->select(oldSelection, QItemSelectionModel::Select);
+        selectionModel->select(select, QItemSelectionModel::Deselect);
+    }
 }
 
 QQuickTreeView::QQuickTreeView(QQuickItem *parent)