4 files changed, 206 insertions, 64 deletions
diff --git a/src/quick/items/qquicktableview.cpp b/src/quick/items/qquicktableview.cpp
index 42aaaeb82c..36537a20b0 100644
--- a/src/quick/items/qquicktableview.cpp
+++ b/src/quick/items/qquicktableview.cpp
@@ -1345,10 +1345,9 @@
 
     \snippet qml/tableview/editdelegate.qml 1
 
-    When the edit delegate is instantiated, it will call \l QQuickItem::forceActiveFocus()
-    on the first item inside the delegate that has \l QQuickItem::activeFocusOnTab set to
-    \c true. You can override this by calling \l QQuickItem::forceActiveFocus() explicitly
-    on some other item inside the delegate from \l {Component::completed()}{Component.completed()}.
+    When the edit delegate is instantiated, TableView will call \l QQuickItem::forceActiveFocus()
+    on it. If you want active focus to be set on a child of the edit delegate instead, let
+    the edit delegate be a \l FocusScope.
 
     \sa editTriggers, TableView::commit, edit(), closeEditor(), {Editing cells}
 */
@@ -5046,34 +5045,33 @@ bool QQuickTableViewPrivate::editFromKeyEvent(QKeyEvent *e)
     if (!attached || !attached->editDelegate())
         return false;
 
-    bool anyKeyAccepted = false;
+    bool anyKeyPressed = false;
     bool editKeyPressed = false;
 
-    if (editTriggers & QQuickTableView::AnyKeyPressed) {
-        switch (e->key()) {
-        case Qt::Key_Shift:
-        case Qt::Key_Alt:
-        case Qt::Key_Control:
-        case Qt::Key_Meta:
-            break;
-        default:
-            anyKeyAccepted = true;
-        }
-    }
-
-    if (editTriggers & QQuickTableView::EditKeyPressed) {
-        switch (e->key()) {
-        case Qt::Key_Return:
-        case Qt::Key_Enter:
+    switch (e->key()) {
+    case Qt::Key_Return:
+    case Qt::Key_Enter:
 #ifndef Q_OS_MACOS
-        case Qt::Key_F2:
+    case Qt::Key_F2:
 #endif
-            editKeyPressed = true;
-            break;
-        }
+        anyKeyPressed = true;
+        editKeyPressed = true;
+        break;
+    case Qt::Key_Shift:
+    case Qt::Key_Alt:
+    case Qt::Key_Control:
+    case Qt::Key_Meta:
+    case Qt::Key_Tab:
+    case Qt::Key_Backtab:
+        break;
+    default:
+        anyKeyPressed = true;
     }
 
-    if (!(editKeyPressed || anyKeyAccepted))
+    const bool anyKeyAccepted = anyKeyPressed && (editTriggers & QQuickTableView::AnyKeyPressed);
+    const bool editKeyAccepted = editKeyPressed && (editTriggers & QQuickTableView::EditKeyPressed);
+
+    if (!(editKeyAccepted || anyKeyAccepted))
         return false;
 
     if (!canEdit(index, false)) {
@@ -6114,22 +6112,16 @@ void QQuickTableView::edit(const QModelIndex &index)
     // Inform the delegate, and the edit delegate, that they're being edited
     d->setRequiredProperty(kRequiredProperty_editing, QVariant::fromValue(true), cellIndex, cellItem, false);
 
-    // Find the first child inside the delegate that wants focus. But
-    // we only transfer focus if the edit item didn't already set
-    // it to some other internal item explicitly upon construction.
-    QObject *focusObject = d->editItem->window()->focusObject();
-    QQuickItem *focusItem = qobject_cast<QQuickItem *>(focusObject);
-    if (!d->editItem->isAncestorOf(focusItem)) {
-        QQuickItem *newFocusItem = d->editItem;
-        if (!newFocusItem->activeFocusOnTab())
-            newFocusItem = d->editItem->nextItemInFocusChain(true);
-        if (newFocusItem == d->editItem || d->editItem->isAncestorOf(newFocusItem)) {
-            // Set activeFocusOnTab to false to ensure that QQuickTableView::keyPressEvent()
-            // receives the tab keys, instead of QQuickItemPrivate::focusNextPrev().
-            // TableView knows better which cell is next in the focus chain.
-            newFocusItem->setActiveFocusOnTab(false);
-            newFocusItem->forceActiveFocus(Qt::MouseFocusReason);
-        }
+    // Transfer focus to the edit item
+    d->editItem->forceActiveFocus(Qt::MouseFocusReason);
+
+    // Install an event filter on the focus object to handle Enter and Tab.
+    // Note that the focusObject doesn't need to be the editItem itself, in
+    // case the editItem is a FocusScope.
+    if (QObject *focusObject = d->editItem->window()->focusObject()) {
+        QQuickItem *focusItem = qobject_cast<QQuickItem *>(focusObject);
+        if (focusItem == d->editItem || d->editItem->isAncestorOf(focusItem))
+            focusItem->installEventFilter(this);
     }
 }
 
@@ -6232,28 +6224,6 @@ void QQuickTableView::keyPressEvent(QKeyEvent *e)
     if (d->editIndex.isValid()) {
         // While editing, we limit the keys that we
         // handle to not interfere with editing.
-        if (d->editTriggers != QQuickTableView::NoEditTriggers) {
-            switch (e->key()) {
-            case Qt::Key_Enter:
-            case Qt::Key_Return:
-                if (auto attached = d->getAttachedObject(d->editItem))
-                    emit attached->commit();
-                closeEditor();
-                break;
-            case Qt::Key_Escape:
-                closeEditor();
-                break;
-            case Qt::Key_Tab:
-            case Qt::Key_Backtab:
-                if (activeFocusOnTab()) {
-                    if (auto attached = d->getAttachedObject(d->editItem))
-                        emit attached->commit();
-                    if (d->setCurrentIndexFromKeyEvent(e))
-                        edit(d->selectionModel->currentIndex());
-                }
-                break;
-            }
-        }
         return;
     }
 
@@ -6266,6 +6236,42 @@ void QQuickTableView::keyPressEvent(QKeyEvent *e)
     QQuickFlickable::keyPressEvent(e);
 }
 
+bool QQuickTableView::eventFilter(QObject *obj, QEvent *event)
+{
+    Q_D(QQuickTableView);
+
+    if (event->type() == QEvent::KeyPress) {
+        Q_ASSERT(d->editItem);
+        QKeyEvent *keyEvent = static_cast<QKeyEvent *>(event);
+        switch (keyEvent->key()) {
+        case Qt::Key_Enter:
+        case Qt::Key_Return:
+            if (auto attached = d->getAttachedObject(d->editItem))
+                emit attached->commit();
+            closeEditor();
+            return true;
+        case Qt::Key_Tab:
+        case Qt::Key_Backtab:
+            if (activeFocusOnTab()) {
+                if (auto attached = d->getAttachedObject(d->editItem))
+                    emit attached->commit();
+                closeEditor();
+                if (d->setCurrentIndexFromKeyEvent(keyEvent)) {
+                    const QModelIndex currentIndex = d->selectionModel->currentIndex();
+                    if (d->canEdit(currentIndex, false))
+                        edit(currentIndex);
+                }
+                return true;
+            }
+        case Qt::Key_Escape:
+            closeEditor();
+            return true;
+        }
+    }
+
+    return QQuickFlickable::eventFilter(obj, event);
+}
+
 bool QQuickTableView::alternatingRows() const
 {
     return d_func()->alternatingRows;
diff --git a/src/quick/items/qquicktableview_p.h b/src/quick/items/qquicktableview_p.h
index d7f858e48e..162d576320 100644
--- a/src/quick/items/qquicktableview_p.h
+++ b/src/quick/items/qquicktableview_p.h
@@ -248,6 +248,7 @@ protected:
     void geometryChange(const QRectF &newGeometry, const QRectF &oldGeometry) override;
     void viewportMoved(Qt::Orientations orientation) override;
     void keyPressEvent(QKeyEvent *e) override;
+    bool eventFilter(QObject *obj, QEvent *event) override;
 
 protected:
     QQuickTableView(QQuickTableViewPrivate &dd, QQuickItem *parent);
diff --git a/tests/auto/quick/qquicktableview/data/editdelegate_combobox.qml b/tests/auto/quick/qquicktableview/data/editdelegate_combobox.qml
new file mode 100644
index 0000000000..b7de4cd406
--- /dev/null
+++ b/tests/auto/quick/qquicktableview/data/editdelegate_combobox.qml
@@ -0,0 +1,67 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
+
+import QtQuick
+import QtQuick.Window
+import QtQuick.Controls
+
+Item {
+    width: 640
+    height: 450
+
+    property alias tableView: tableView
+
+    TableView {
+        id: tableView
+        anchors.fill: parent
+        clip: true
+
+        property Item editItem: null
+        property Item focusItem: null
+        property var editIndex
+        property int commitCount: 0
+        property int comboFocusCount: 0
+
+        selectionModel: ItemSelectionModel {}
+
+        delegate: Rectangle {
+            implicitWidth: 100
+            implicitHeight: 50
+
+            required property bool editing
+
+            Text {
+                anchors.centerIn: parent
+                text: display
+                visible: !editing
+            }
+
+            TableView.editDelegate: FocusScope {
+                id: editRoot
+                anchors.fill: parent
+                required property bool current
+                required property bool selected
+                required property bool editing
+
+                TableView.onCommit: tableView.commitCount++;
+
+                Component.onCompleted: {
+                    tableView.editItem = editRoot
+                    tableView.editIndex = tableView.modelIndex(row, column)
+                }
+
+                Component.onDestruction: {
+                    tableView.editItem = null
+                    tableView.editIndex = tableView.modelIndex(-1, -1)
+                }
+
+                ComboBox {
+                    focus: true
+                    model: 4
+                    onActiveFocusChanged: if (activeFocus) tableView.comboFocusCount++;
+                }
+            }
+        }
+    }
+
+}
diff --git a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
index b0134a94d4..ecea4adfd5 100644
--- a/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
+++ b/tests/auto/quick/qquicktableview/tst_qquicktableview.cpp
@@ -256,6 +256,7 @@ private slots:
     void editUsingEditTriggers_data();
     void editUsingEditTriggers();
     void editUsingTab();
+    void editDelegateComboBox();
     void editOnNonEditableCell_data();
     void editOnNonEditableCell();
     void noEditDelegate_data();
@@ -6752,6 +6753,73 @@ void tst_QQuickTableView::editUsingTab()
     QVERIFY(editItem3);
 }
 
+void tst_QQuickTableView::editDelegateComboBox()
+{
+    // Using a ComboBox as an edit delegate should be a quite common
+    // use case. So test that it works.
+    LOAD_TABLEVIEW("editdelegate_combobox.qml");
+
+    auto model = TestModel(4, 4);
+    tableView->setModel(QVariant::fromValue(&model));
+    tableView->forceActiveFocus();
+
+    const char kEditItem[] = "editItem";
+    const char kEditIndex[] = "editIndex";
+    const char kCommitCount[] = "commitCount";
+    const char kComboFocusCount[] = "comboFocusCount";
+
+    WAIT_UNTIL_POLISHED;
+
+    const QPoint cell1(1, 1);
+    const QPoint cell2(2, 1);
+    const QModelIndex index1 = tableView->modelIndex(cell1);
+    const QModelIndex index2 = tableView->modelIndex(cell2);
+
+    QQuickWindow *window = tableView->window();
+
+    // Edit cell 1
+    tableView->edit(index1);
+    QCOMPARE(tableView->property(kEditIndex).value<QModelIndex>(), index1);
+    const QQuickItem *editItem1 = tableView->property(kEditItem).value<QQuickItem *>();
+    QVERIFY(editItem1);
+    QCOMPARE(tableView->property(kComboFocusCount).value<int>(), 1);
+
+    // Press Tab to edit cell 2
+    QTest::keyClick(window, Qt::Key_Tab);
+    QCOMPARE(tableView->property(kCommitCount).value<int>(), 1);
+    QCOMPARE(tableView->property(kEditIndex).value<QModelIndex>(), index2);
+    const QQuickItem *editItem2 = tableView->property(kEditItem).value<QQuickItem *>();
+    QVERIFY(editItem2);
+    QCOMPARE(tableView->property(kComboFocusCount).value<int>(), 2);
+
+    // Press Enter to commit
+    QTest::keyClick(window, Qt::Key_Enter);
+    QCOMPARE(tableView->property(kCommitCount).value<int>(), 2);
+    QCOMPARE(tableView->property(kComboFocusCount).value<int>(), 2);
+    QVERIFY(!tableView->property(kEditIndex).value<QModelIndex>().isValid());
+    QVERIFY(!tableView->property(kEditItem).value<QQuickItem *>());
+
+    // Edit cell 1
+    tableView->edit(index1);
+    // Press escape to close editor
+    QTest::keyClick(window, Qt::Key_Escape);
+    QCOMPARE(tableView->property(kCommitCount).value<int>(), 2);
+    QCOMPARE(tableView->property(kComboFocusCount).value<int>(), 3);
+    QVERIFY(!tableView->property(kEditIndex).value<QModelIndex>().isValid());
+    QVERIFY(!tableView->property(kEditItem).value<QQuickItem *>());
+
+    // Edit cell 2
+    tableView->edit(index2);
+    // Press space to open combo menu
+    QTest::keyClick(window, Qt::Key_Space);
+    // Press Enter to commit and close the editor
+    QTest::keyClick(window, Qt::Key_Enter);
+    QCOMPARE(tableView->property(kCommitCount).value<int>(), 3);
+    QCOMPARE(tableView->property(kComboFocusCount).value<int>(), 4);
+    QVERIFY(!tableView->property(kEditIndex).value<QModelIndex>().isValid());
+    QVERIFY(!tableView->property(kEditItem).value<QQuickItem *>());
+}
+
 void tst_QQuickTableView::editOnNonEditableCell_data()
 {
     QTest::addColumn<QQuickTableView::EditTriggers>("editTriggers");