QAbstractItemView: don't start editing on Ctrl-Click

Amends 17c1ebf8bfd254ff75cc55e335d1c1fb01da547f, after which dragEnabled
item views toggled selection on click rather than on press. If the edit
trigger included SelectedClicked at the same time, then Ctrl-Clicking a
selected item would start editing the item, instead of toggling
selection.

Fix this by ignoring clicks with modifier when evaluating whether
editing should start.

Extend the mouseSelection test case by including a column for the
editTrigger, and cover the respective combinations.

Fixes: QTBUG-111131
Change-Id: I9605f9b3d5a49e292551a34c3c4c7a5f9ecb2a89
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
(cherry picked from commit 557dcd8a87c6c6c77ccc71a85b1ec349c69eb4c4)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Volker Hilsheimer 2023-02-15 12:28:24 +01:00 committed by Qt Cherry-pick Bot
parent 2fef5514d3
commit d2ef5cd650
2 changed files with 46 additions and 2 deletions

View File

@ -1939,7 +1939,9 @@ void QAbstractItemView::mouseReleaseEvent(QMouseEvent *event)
} }
bool click = (index == d->pressedIndex && index.isValid() && !releaseFromDoubleClick); bool click = (index == d->pressedIndex && index.isValid() && !releaseFromDoubleClick);
bool selectedClicked = click && (event->button() == Qt::LeftButton) && d->pressedAlreadySelected; bool selectedClicked = click && d->pressedAlreadySelected
&& (event->button() == Qt::LeftButton)
&& (event->modifiers() == Qt::NoModifier);
EditTrigger trigger = (selectedClicked ? SelectedClicked : NoEditTriggers); EditTrigger trigger = (selectedClicked ? SelectedClicked : NoEditTriggers);
const bool edited = click && !d->pressClosedEditor ? edit(index, trigger, event) : false; const bool edited = click && !d->pressClosedEditor ? edit(index, trigger, event) : false;

View File

@ -2847,32 +2847,40 @@ void tst_QAbstractItemView::mouseSelection_data()
{ {
QTest::addColumn<QAbstractItemView::SelectionMode>("selectionMode"); QTest::addColumn<QAbstractItemView::SelectionMode>("selectionMode");
QTest::addColumn<bool>("dragEnabled"); QTest::addColumn<bool>("dragEnabled");
QTest::addColumn<QAbstractItemView::EditTrigger>("editTrigger");
QTest::addColumn<QList<SelectionEvent>>("selectionEvents"); QTest::addColumn<QList<SelectionEvent>>("selectionEvents");
QTest::addColumn<QList<int>>("selectedRows"); QTest::addColumn<QList<int>>("selectedRows");
// single selection mode - always one row selected, modifiers ignored // single selection mode - always one row selected, modifiers ignored
QTest::addRow("Single:Press") << QAbstractItemView::SingleSelection << false QTest::addRow("Single:Press") << QAbstractItemView::SingleSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 1)} << QList{SelectionEvent(SelectionEvent::Press, 1)}
<< QList{1}; << QList{1};
QTest::addRow("Single:Click") << QAbstractItemView::SingleSelection << false QTest::addRow("Single:Click") << QAbstractItemView::SingleSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent{SelectionEvent::Click, 1}} << QList{SelectionEvent{SelectionEvent::Click, 1}}
<< QList{1}; << QList{1};
QTest::addRow("Single:Press+Drag") << QAbstractItemView::SingleSelection << false QTest::addRow("Single:Press+Drag") << QAbstractItemView::SingleSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 1), << QList{SelectionEvent(SelectionEvent::Press, 1),
SelectionEvent{SelectionEvent::Move, 2}, SelectionEvent{SelectionEvent::Move, 2},
SelectionEvent{SelectionEvent::Release}} SelectionEvent{SelectionEvent::Release}}
<< QList{2}; << QList{2};
QTest::addRow("Single:Shift+Click") << QAbstractItemView::SingleSelection << false QTest::addRow("Single:Shift+Click") << QAbstractItemView::SingleSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent{SelectionEvent::Click, Qt::ShiftModifier, 2}} << QList{SelectionEvent{SelectionEvent::Click, Qt::ShiftModifier, 2}}
<< QList{2}; << QList{2};
QTest::addRow("Single:Press;Ctrl+Press") << QAbstractItemView::SingleSelection << false QTest::addRow("Single:Press;Ctrl+Press") << QAbstractItemView::SingleSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent{SelectionEvent::Press, 3}, << QList{SelectionEvent{SelectionEvent::Press, 3},
SelectionEvent{SelectionEvent::Press, Qt::ControlModifier, 3}} SelectionEvent{SelectionEvent::Press, Qt::ControlModifier, 3}}
<< QList{3}; << QList{3};
QTest::addRow("Single:Ctrl+Click") << QAbstractItemView::SingleSelection << false QTest::addRow("Single:Ctrl+Click") << QAbstractItemView::SingleSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent{SelectionEvent::Click, Qt::ControlModifier, 3}} << QList{SelectionEvent{SelectionEvent::Click, Qt::ControlModifier, 3}}
<< QList{3}; << QList{3};
QTest::addRow("Single:Click;Ctrl+Click") << QAbstractItemView::SingleSelection << false QTest::addRow("Single:Click;Ctrl+Click") << QAbstractItemView::SingleSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent{SelectionEvent::Click, 3}, << QList{SelectionEvent{SelectionEvent::Click, 3},
SelectionEvent{SelectionEvent::Click, Qt::ControlModifier, 3}} SelectionEvent{SelectionEvent::Click, Qt::ControlModifier, 3}}
<< QList<int>{}; << QList<int>{};
@ -2880,30 +2888,37 @@ void tst_QAbstractItemView::mouseSelection_data()
// multi selection mode - selection toggles on press, selection can be drag-extended // multi selection mode - selection toggles on press, selection can be drag-extended
// modifiers ignored // modifiers ignored
QTest::addRow("Multi:Press") << QAbstractItemView::MultiSelection << false QTest::addRow("Multi:Press") << QAbstractItemView::MultiSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 1)} << QList{SelectionEvent(SelectionEvent::Press, 1)}
<< QList{1}; << QList{1};
QTest::addRow("Multi:Press twice") << QAbstractItemView::MultiSelection << false QTest::addRow("Multi:Press twice") << QAbstractItemView::MultiSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 1), << QList{SelectionEvent(SelectionEvent::Press, 1),
SelectionEvent(SelectionEvent::Press, 1)} SelectionEvent(SelectionEvent::Press, 1)}
<< QList<int>{}; << QList<int>{};
QTest::addRow("Multi:Press twice with drag enabled") << QAbstractItemView::MultiSelection << true QTest::addRow("Multi:Press twice with drag enabled") << QAbstractItemView::MultiSelection << true
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Click, 1), << QList{SelectionEvent(SelectionEvent::Click, 1),
SelectionEvent(SelectionEvent::Press, 1)} SelectionEvent(SelectionEvent::Press, 1)}
<< QList{1}; << QList{1};
QTest::addRow("Multi:Press and click with drag enabled") << QAbstractItemView::MultiSelection << true QTest::addRow("Multi:Press and click with drag enabled") << QAbstractItemView::MultiSelection << true
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 1), << QList{SelectionEvent(SelectionEvent::Press, 1),
SelectionEvent(SelectionEvent::Click, 1)} SelectionEvent(SelectionEvent::Click, 1)}
<< QList<int>{}; << QList<int>{};
QTest::addRow("Multi:Press,Press") << QAbstractItemView::MultiSelection << false QTest::addRow("Multi:Press,Press") << QAbstractItemView::MultiSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 2), << QList{SelectionEvent(SelectionEvent::Press, 2),
SelectionEvent(SelectionEvent::Press, 3)} SelectionEvent(SelectionEvent::Press, 3)}
<< QList{2, 3}; << QList{2, 3};
QTest::addRow("Multi:Press,Drag") << QAbstractItemView::MultiSelection << false QTest::addRow("Multi:Press,Drag") << QAbstractItemView::MultiSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 1), << QList{SelectionEvent(SelectionEvent::Press, 1),
SelectionEvent(SelectionEvent::Move, 5), SelectionEvent(SelectionEvent::Move, 5),
SelectionEvent(SelectionEvent::Release)} SelectionEvent(SelectionEvent::Release)}
<< QList{1, 2, 3, 4, 5}; << QList{1, 2, 3, 4, 5};
QTest::addRow("Multi:Press,Drag,Deselect") << QAbstractItemView::MultiSelection << false QTest::addRow("Multi:Press,Drag,Deselect") << QAbstractItemView::MultiSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 1), << QList{SelectionEvent(SelectionEvent::Press, 1),
SelectionEvent(SelectionEvent::Move, 5), SelectionEvent(SelectionEvent::Move, 5),
SelectionEvent(SelectionEvent::Release), SelectionEvent(SelectionEvent::Release),
@ -2911,6 +2926,7 @@ void tst_QAbstractItemView::mouseSelection_data()
<< QList{1, 2, 4, 5}; << QList{1, 2, 4, 5};
// drag-select a few indices; then drag-select a larger area that includes the first // drag-select a few indices; then drag-select a larger area that includes the first
QTest::addRow("Multi:Press,Drag;Surround") << QAbstractItemView::MultiSelection << false QTest::addRow("Multi:Press,Drag;Surround") << QAbstractItemView::MultiSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 3), << QList{SelectionEvent(SelectionEvent::Press, 3),
SelectionEvent(SelectionEvent::Move, 5), SelectionEvent(SelectionEvent::Move, 5),
SelectionEvent(SelectionEvent::Release), SelectionEvent(SelectionEvent::Release),
@ -2920,6 +2936,7 @@ void tst_QAbstractItemView::mouseSelection_data()
<< QList{1, 2, 3, 4, 5, 6, 7, 8}; << QList{1, 2, 3, 4, 5, 6, 7, 8};
// drag-select a few indices; then try to select more starting with the last -> not working // drag-select a few indices; then try to select more starting with the last -> not working
QTest::addRow("Multi:Press,Drag;Expand") << QAbstractItemView::MultiSelection << false QTest::addRow("Multi:Press,Drag;Expand") << QAbstractItemView::MultiSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 3), << QList{SelectionEvent(SelectionEvent::Press, 3),
SelectionEvent(SelectionEvent::Move, 5), SelectionEvent(SelectionEvent::Move, 5),
SelectionEvent(SelectionEvent::Release), SelectionEvent(SelectionEvent::Release),
@ -2930,6 +2947,7 @@ void tst_QAbstractItemView::mouseSelection_data()
// Multi: Press-dragging a selection should not deselect #QTBUG-59888 // Multi: Press-dragging a selection should not deselect #QTBUG-59888
QTest::addRow("Multi:Press-Drag selection") << QAbstractItemView::MultiSelection << true QTest::addRow("Multi:Press-Drag selection") << QAbstractItemView::MultiSelection << true
// with drag'n'drop enabled, we cannot drag a selection // with drag'n'drop enabled, we cannot drag a selection
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Click, 2), << QList{SelectionEvent(SelectionEvent::Click, 2),
SelectionEvent(SelectionEvent::Click, 3), SelectionEvent(SelectionEvent::Click, 3),
SelectionEvent(SelectionEvent::Click, 4), SelectionEvent(SelectionEvent::Click, 4),
@ -2942,42 +2960,51 @@ void tst_QAbstractItemView::mouseSelection_data()
// Extended selection: Press selects a single item // Extended selection: Press selects a single item
QTest::addRow("Extended:Press") << QAbstractItemView::ExtendedSelection << false QTest::addRow("Extended:Press") << QAbstractItemView::ExtendedSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 3)} << QList{SelectionEvent(SelectionEvent::Press, 3)}
<< QList{3}; << QList{3};
QTest::addRow("Extended:Press twice") << QAbstractItemView::ExtendedSelection << false QTest::addRow("Extended:Press twice") << QAbstractItemView::ExtendedSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 3), << QList{SelectionEvent(SelectionEvent::Press, 3),
SelectionEvent(SelectionEvent::Press, 3)} SelectionEvent(SelectionEvent::Press, 3)}
<< QList{3}; << QList{3};
QTest::addRow("Extended:Press,Press") << QAbstractItemView::ExtendedSelection << false QTest::addRow("Extended:Press,Press") << QAbstractItemView::ExtendedSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 2), << QList{SelectionEvent(SelectionEvent::Press, 2),
SelectionEvent(SelectionEvent::Press, 3)} SelectionEvent(SelectionEvent::Press, 3)}
<< QList{3}; << QList{3};
// Extended selection: press with Ctrl toggles item // Extended selection: press with Ctrl toggles item
QTest::addRow("Extended:Press,Toggle") << QAbstractItemView::ExtendedSelection << false QTest::addRow("Extended:Press,Toggle") << QAbstractItemView::ExtendedSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 3), << QList{SelectionEvent(SelectionEvent::Press, 3),
SelectionEvent(SelectionEvent::Click, Qt::ControlModifier, 3)} SelectionEvent(SelectionEvent::Click, Qt::ControlModifier, 3)}
<< QList<int>{}; << QList<int>{};
QTest::addRow("Extended:Press,Add") << QAbstractItemView::ExtendedSelection << false QTest::addRow("Extended:Press,Add") << QAbstractItemView::ExtendedSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 1), << QList{SelectionEvent(SelectionEvent::Press, 1),
SelectionEvent(SelectionEvent::Click, Qt::ControlModifier, 3)} SelectionEvent(SelectionEvent::Click, Qt::ControlModifier, 3)}
<< QList{1, 3}; << QList{1, 3};
// Extended selection: Shift creates a range between first and last pressed // Extended selection: Shift creates a range between first and last pressed
QTest::addRow("Extended:Press,Range") << QAbstractItemView::ExtendedSelection << false QTest::addRow("Extended:Press,Range") << QAbstractItemView::ExtendedSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 1), << QList{SelectionEvent(SelectionEvent::Press, 1),
SelectionEvent(SelectionEvent::Press, Qt::ShiftModifier, 5)} SelectionEvent(SelectionEvent::Press, Qt::ShiftModifier, 5)}
<< QList{1, 2, 3, 4, 5}; << QList{1, 2, 3, 4, 5};
QTest::addRow("Extended:Press,Range,Fix Range") << QAbstractItemView::ExtendedSelection << false QTest::addRow("Extended:Press,Range,Fix Range") << QAbstractItemView::ExtendedSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 1), << QList{SelectionEvent(SelectionEvent::Press, 1),
SelectionEvent(SelectionEvent::Press, Qt::ShiftModifier, 5), SelectionEvent(SelectionEvent::Press, Qt::ShiftModifier, 5),
SelectionEvent(SelectionEvent::Press, Qt::ShiftModifier, 3)} SelectionEvent(SelectionEvent::Press, Qt::ShiftModifier, 3)}
<< QList{1, 2, 3}; << QList{1, 2, 3};
// Extended: dragging extends the selection // Extended: dragging extends the selection
QTest::addRow("Extended:Press,Drag") << QAbstractItemView::ExtendedSelection << false QTest::addRow("Extended:Press,Drag") << QAbstractItemView::ExtendedSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 2), << QList{SelectionEvent(SelectionEvent::Press, 2),
SelectionEvent(SelectionEvent::Move, 5)} SelectionEvent(SelectionEvent::Move, 5)}
<< QList{2, 3, 4, 5}; << QList{2, 3, 4, 5};
// Extended: Ctrl+Press-dragging extends the selection // Extended: Ctrl+Press-dragging extends the selection
QTest::addRow("Extended:Press,Drag;Ctrl-Press,Drag") << QAbstractItemView::ExtendedSelection << false QTest::addRow("Extended:Press,Drag;Ctrl-Press,Drag") << QAbstractItemView::ExtendedSelection << false
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Press, 2), << QList{SelectionEvent(SelectionEvent::Press, 2),
SelectionEvent(SelectionEvent::Move, 5), SelectionEvent(SelectionEvent::Move, 5),
SelectionEvent(SelectionEvent::Release), SelectionEvent(SelectionEvent::Release),
@ -2987,6 +3014,7 @@ void tst_QAbstractItemView::mouseSelection_data()
<< QList{2, 3, 4, 5, 6, 7, 8}; << QList{2, 3, 4, 5, 6, 7, 8};
// Extended: Ctrl+Press-dragging in a selection should not deselect #QTBUG-59888 // Extended: Ctrl+Press-dragging in a selection should not deselect #QTBUG-59888
QTest::addRow("Extended:Ctrl-Drag selection") << QAbstractItemView::ExtendedSelection << true QTest::addRow("Extended:Ctrl-Drag selection") << QAbstractItemView::ExtendedSelection << true
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Click, 2), << QList{SelectionEvent(SelectionEvent::Click, 2),
SelectionEvent(SelectionEvent::Click, Qt::ShiftModifier, 5), SelectionEvent(SelectionEvent::Click, Qt::ShiftModifier, 5),
SelectionEvent(SelectionEvent::Press, Qt::ControlModifier, 3), SelectionEvent(SelectionEvent::Press, Qt::ControlModifier, 3),
@ -2996,6 +3024,7 @@ void tst_QAbstractItemView::mouseSelection_data()
<< QList{2, 3, 4, 5}; << QList{2, 3, 4, 5};
// Extended: Ctrl+Press-dragging with a selection extends, then drags #QTBUG-59888 // Extended: Ctrl+Press-dragging with a selection extends, then drags #QTBUG-59888
QTest::addRow("Extended:Ctrl-Drag selection") << QAbstractItemView::ExtendedSelection << true QTest::addRow("Extended:Ctrl-Drag selection") << QAbstractItemView::ExtendedSelection << true
<< QAbstractItemView::NoEditTriggers
<< QList{SelectionEvent(SelectionEvent::Click, 2), << QList{SelectionEvent(SelectionEvent::Click, 2),
SelectionEvent(SelectionEvent::Click, Qt::ShiftModifier, 5), SelectionEvent(SelectionEvent::Click, Qt::ShiftModifier, 5),
SelectionEvent(SelectionEvent::Press, Qt::ControlModifier, 6), SelectionEvent(SelectionEvent::Press, Qt::ControlModifier, 6),
@ -3003,12 +3032,24 @@ void tst_QAbstractItemView::mouseSelection_data()
// two moves needed because of distance and state logic in 7QAbstractItemView // two moves needed because of distance and state logic in 7QAbstractItemView
SelectionEvent(SelectionEvent::Move, Qt::ControlModifier, 8)} SelectionEvent(SelectionEvent::Move, Qt::ControlModifier, 8)}
<< QList{2, 3, 4, 5, 6}; << QList{2, 3, 4, 5, 6};
// Extended: when drag is enabled, click with Ctrl toggles item instead of editing # QTBUG-111131
QTest::addRow("Extended:Click,Toggle,editable") << QAbstractItemView::ExtendedSelection << false
<< QAbstractItemView::SelectedClicked
<< QList{SelectionEvent(SelectionEvent::Click, 3),
SelectionEvent(SelectionEvent::Click, Qt::ControlModifier, 3)}
<< QList<int>{};
QTest::addRow("Extended:Click,Toggle,dragable,editable") << QAbstractItemView::ExtendedSelection << true
<< QAbstractItemView::SelectedClicked
<< QList{SelectionEvent(SelectionEvent::Click, 3),
SelectionEvent(SelectionEvent::Click, Qt::ControlModifier, 3)}
<< QList<int>{};
} }
void tst_QAbstractItemView::mouseSelection() void tst_QAbstractItemView::mouseSelection()
{ {
QFETCH(QAbstractItemView::SelectionMode, selectionMode); QFETCH(QAbstractItemView::SelectionMode, selectionMode);
QFETCH(bool, dragEnabled); QFETCH(bool, dragEnabled);
QFETCH(QAbstractItemView::EditTrigger, editTrigger);
QFETCH(QList<SelectionEvent>, selectionEvents); QFETCH(QList<SelectionEvent>, selectionEvents);
QFETCH(QList<int>, selectedRows); QFETCH(QList<int>, selectedRows);
@ -3017,7 +3058,7 @@ void tst_QAbstractItemView::mouseSelection()
for (int i = 0; i < 10; ++i) { for (int i = 0; i < 10; ++i) {
QStandardItem *item = new QStandardItem(QString("item %0").arg(i)); QStandardItem *item = new QStandardItem(QString("item %0").arg(i));
item->setDragEnabled(dragEnabled); item->setDragEnabled(dragEnabled);
item->setEditable(false); item->setEditable(editTrigger != QAbstractItemView::NoEditTriggers);
parentItem->appendRow(item); parentItem->appendRow(item);
} }
@ -3027,6 +3068,7 @@ void tst_QAbstractItemView::mouseSelection()
view->setModel(&model); view->setModel(&model);
view->setDragEnabled(dragEnabled); view->setDragEnabled(dragEnabled);
view->setSelectionMode(selectionMode); view->setSelectionMode(selectionMode);
view->setEditTriggers(editTrigger);
view->show(); view->show();
QVERIFY(QTest::qWaitForWindowActive(view)); QVERIFY(QTest::qWaitForWindowActive(view));