QAIV: Don't open editor on release when press closed editor

A mouse press that transfers focus from an open editor back to the view
will close the editor. To prevent that the corresponding release then
opens the same editor again we need to know that the closeEditor call
was caused by the mouse press. Since Qt first generates the focusOut
event, and then delivers the mouse press, we have to start a zero-timer
to check whether we are in the same event delivery process. If so, ignore
the corresponding release.

Add test case that simulates that chain of events.

Fixes: QTBUG-20456
Pick-to: 6.2 6.1
Change-Id: I28fa32bfbc776db207c594c329961f575ae58ea9
Reviewed-by: Jan Arve Sæther <[email protected]>

src/widgets/itemviews/qabstractitemview.cpp

@@ -86,6 +86,7 @@ QAbstractItemViewPrivate::QAbstractItemViewPrivate()
         selectionMode(QAbstractItemView::ExtendedSelection),
         selectionBehavior(QAbstractItemView::SelectItems),
         currentlyCommittingEditor(nullptr),
+        pressClosedEditor(false),
         pressedModifiers(Qt::NoModifier),
         pressedPosition(QPoint(-1, -1)),
         pressedAlreadySelected(false),
@@ -1777,6 +1778,9 @@ void QAbstractItemView::mousePressEvent(QMouseEvent *event)
     QPoint pos = event->position().toPoint();
     QPersistentModelIndex index = indexAt(pos);
 
+    // this is the mouse press event that closed the last editor (via focus event)
+    d->pressClosedEditor = d->pressClosedEditorWatcher.isActive() && d->lastEditedIndex == index;
+
     if (!d->selectionModel
         || (d->state == EditingState && d->hasEditor(index)))
         return;
@@ -1935,16 +1939,17 @@ void QAbstractItemView::mouseReleaseEvent(QMouseEvent *event)
     bool click = (index == d->pressedIndex && index.isValid() && !releaseFromDoubleClick);
     bool selectedClicked = click && (event->button() == Qt::LeftButton) && d->pressedAlreadySelected;
     EditTrigger trigger = (selectedClicked ? SelectedClicked : NoEditTriggers);
-    const bool edited = click ? edit(index, trigger, event) : false;
+    const bool edited = click && !d->pressClosedEditor ? edit(index, trigger, event) : false;
 
     d->ctrlDragSelectionFlag = QItemSelectionModel::NoUpdate;
 
     if (d->selectionModel && d->noSelectionOnMousePress) {
         d->noSelectionOnMousePress = false;
-        if (!edited)
+        if (!edited && !d->pressClosedEditor)
             d->selectionModel->select(index, selectionCommand(index, event));
     }
 
+    d->pressClosedEditor = false;
     setState(NoState);
 
     if (click) {
@@ -2584,6 +2589,8 @@ void QAbstractItemView::timerEvent(QTimerEvent *event)
         //we only get here if there was no double click
         if (d->pressedIndex.isValid() && d->pressedIndex == currentIndex())
             scrollTo(d->pressedIndex);
+    } else if (event->timerId() == d->pressClosedEditorWatcher.timerId()) {
+        d->pressClosedEditorWatcher.stop();
     }
 }
 
@@ -2854,6 +2861,12 @@ void QAbstractItemView::closeEditor(QWidget *editor, QAbstractItemDelegate::EndE
         if (!index.isValid())
             return; // the editor was not registered
 
+        // start a timer that expires immediately when we return to the event loop
+        // to identify whether this close was triggered by a mousepress-initiated
+        // focus event
+        d->pressClosedEditorWatcher.start(0, this);
+        d->lastEditedIndex = index;
+
         if (!isPersistent) {
             setState(NoState);
             QModelIndex index = d->indexForEditor(editor);

src/widgets/itemviews/qabstractitemview_p.h

@@ -364,6 +364,9 @@ class Q_AUTOTEST_EXPORT QAbstractItemViewPrivate : public QAbstractScrollAreaPri
     QIndexEditorHash indexEditorHash;
     QSet<QWidget*> persistent;
     QWidget *currentlyCommittingEditor;
+    QBasicTimer pressClosedEditorWatcher;
+    QPersistentModelIndex lastEditedIndex;
+    bool pressClosedEditor;
 
     QPersistentModelIndex enteredIndex;
     QPersistentModelIndex pressedIndex;

tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp

@@ -102,6 +102,7 @@ private slots:
     void selectAll();
     void ctrlA();
     void persistentEditorFocus();
+    void pressClosesReleaseDoesntOpenEditor();
     void setItemDelegate();
     void setItemDelegate_data();
     // The dragAndDrop() test doesn't work, and is thus disabled on Mac and Windows
@@ -732,6 +733,63 @@ void tst_QAbstractItemView::persistentEditorFocus()
     }
 }
 
+/*!
+    A press into the selection area of an item being edited, but outside the editor,
+    closes the editor by transferring focus to the view. The corresponding release
+    should then not re-open the editor.
+
+    QTBUG-20456.
+*/
+void tst_QAbstractItemView::pressClosesReleaseDoesntOpenEditor()
+{
+    QStandardItemModel model(0, 1);
+    auto *parent = new QStandardItem("parent");
+    for (const auto &childText : {"child1", "child2"}) {
+        auto *child = new QStandardItem(childText);
+        child->setFlags(Qt::ItemFlag::ItemIsEnabled | Qt::ItemFlag::ItemIsEditable | Qt::ItemIsSelectable);
+        parent->appendRow(child);
+    }
+    model.appendRow(parent);
+
+    QTreeView view;
+    view.setModel(&model);
+    view.setExpanded(model.indexFromItem(parent), true);
+    view.setSelectionMode(QAbstractItemView::SingleSelection);
+    view.setEditTriggers(QAbstractItemView::SelectedClicked | QAbstractItemView::DoubleClicked);
+
+    view.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&view));
+
+    const QRect childRect = view.visualRect(model.indexFromItem(parent->child(0)));
+    QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, childRect.center()); // select
+    QVERIFY(view.selectionModel()->selectedIndexes().contains(model.indexFromItem(parent->child(0))));
+    QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, childRect.center()); // edit
+    QTRY_COMPARE(view.state(), QAbstractItemView::EditingState);
+    QPoint inChildOutsideEditor = QPoint(view.indentation() / 2, childRect.center().y());
+    QTest::mousePress(view.viewport(), Qt::LeftButton, Qt::NoModifier, inChildOutsideEditor); // focus itemview, editor closes
+    QCOMPARE(view.state(), QAbstractItemView::NoState);
+    QTest::qWait(10); // process some events, let the internal timer time out
+    QTest::mouseRelease(view.viewport(), Qt::LeftButton, Qt::NoModifier, inChildOutsideEditor); // should not reopen editor
+    QTest::qWait(QApplication::doubleClickInterval() * 2);
+    QCOMPARE(view.state(), QAbstractItemView::NoState);
+
+    // with multiple items selected, clicking from the currently edited item into another
+    // selected item closes the current and reopens a new editor
+    view.setSelectionMode(QAbstractItemView::ExtendedSelection);
+    const QRect child2Rect = view.visualRect(model.indexFromItem(parent->child(1)));
+    QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::ControlModifier, child2Rect.center()); // select
+    QVERIFY(view.selectionModel()->selectedIndexes().contains(model.indexFromItem(parent->child(0))));
+    QVERIFY(view.selectionModel()->selectedIndexes().contains(model.indexFromItem(parent->child(1))));
+    QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, child2Rect.center()); // edit
+    QTRY_COMPARE(view.state(), QAbstractItemView::EditingState);
+    QTest::mousePress(view.viewport(), Qt::LeftButton, Qt::NoModifier, inChildOutsideEditor); // editor closes
+    QCOMPARE(view.state(), QAbstractItemView::NoState);
+    QTest::qWait(10); // process some events, let the internal timer time out
+    QTest::mouseRelease(view.viewport(), Qt::LeftButton, Qt::NoModifier, inChildOutsideEditor); // should open editor
+    QTest::qWait(QApplication::doubleClickInterval() * 2);
+    QCOMPARE(view.state(), QAbstractItemView::EditingState);
+}
+
 
 #if !defined(Q_OS_MAC) && !defined(Q_OS_WIN)