fix(completion): stop the key filter handing itself the key it forwards

Pressing Tab or an arrow key with the completion popup open crashed the
application outright.

QCoreApplication::sendEvent re-runs APPLICATION-level event filters. The
navigation branch forwarded the key to the popup from inside a filter
installed on qApp, so the very same event came back to the filter that
had just sent it. The popup was still visible, the popupVisible() guard
still passed, and it forwarded again: unbounded recursion ending in a
stack overflow rather than in any diagnosable error. Reproduced at 9176
recursive QueryCompleter::eventFilter frames, with a standalone Qt probe
confirming the re-entry independently.

Guard the filter with m_forwarding, checked before the switch so it
covers every branch rather than the navigation keys alone. A key the
filter is itself redelivering now falls through to the popup instead of
being claimed a second time.

The existing tests missed this because they exercised the accept path
without ever forwarding an event. arrowNavigationDoesNotRecurse drives
Down through the grabbing popup and asserts the selection actually
moved, so it fails on a fix that merely swallows the key; against the
old code it takes the process down with it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

3 files changed, 156 insertions, 1 deletions

diff --git a/src/querycompleter.cpp b/src/querycompleter.cpp
index a6a1aeb..7e9dfd9 100644
--- a/src/querycompleter.cpp
+++ b/src/querycompleter.cpp
@@ -422,6 +422,12 @@ bool QueryCompleter::eventFilter(QObject *watched, QEvent *event)
     if (event->type() != QEvent::KeyPress || !popupVisible())
         return QObject::eventFilter(watched, event);
 
+    // A key this filter is itself redelivering. sendEvent re-runs application
+    // event filters, so without this the forwarded key comes straight back and
+    // recurses until the stack is gone.
+    if (m_forwarding)
+        return QObject::eventFilter(watched, event);
+
     auto *keyEvent = static_cast<QKeyEvent *>(event);
     switch (keyEvent->key()) {
     case Qt::Key_Tab:
@@ -452,11 +458,17 @@ bool QueryCompleter::eventFilter(QObject *watched, QEvent *event)
     case Qt::Key_Up:
     case Qt::Key_Down:
     case Qt::Key_PageUp:
-    case Qt::Key_PageDown:
+    case Qt::Key_PageDown: {
         // Navigation belongs to the popup, which is not the focus widget while
         // the user is typing in the bar.
+        //
+        // m_forwarding is what keeps this from recursing; see the guard at the
+        // top of the filter.
+        m_forwarding = true;
         QCoreApplication::sendEvent(m_popup, event);
+        m_forwarding = false;
         return true;
+    }
     default:
         break;
     }
diff --git a/src/querycompleter.h b/src/querycompleter.h
index 6f658f2..27161bc 100644
--- a/src/querycompleter.h
+++ b/src/querycompleter.h
@@ -135,6 +135,11 @@ private:
     const Config &m_config;
     QStringList m_tags;
 
+    /// Set while the filter is redelivering a key to the popup. The filter is
+    /// installed on the application and sendEvent re-runs application filters,
+    /// so without this the forwarded key returns to the filter that sent it.
+    bool m_forwarding = false;
+
     QCompleter *m_completer = nullptr;
     QStandardItemModel *m_model = nullptr;
     CompletionPopup *m_popup = nullptr;
diff --git a/tests/test_querycompleter.cpp b/tests/test_querycompleter.cpp
index c1d7530..09eedc3 100644
--- a/tests/test_querycompleter.cpp
+++ b/tests/test_querycompleter.cpp
@@ -73,6 +73,14 @@ private slots:
     void acceptingAPrefixReopensThePopupForValues();
     void acceptingAValueDoesNotReopenAnEmptyPopup();
     void keysFallThroughWhileThePopupIsHidden();
+
+    // The application filter re-entrancy bug: sendEvent() re-runs application
+    // event filters, so forwarding a key to the popup from inside the filter
+    // hands it straight back and the recursion only ends in a stack overflow.
+    void arrowNavigationDoesNotRecurse();
+    void returnRunsTheQueryOnceCompletionIsDone();
+    void returnRunsTheQueryAfterAMouseAccept();
+    void returnRunsTheQueryWhenThePopupMatchesNothing();
 };
 
 // Copied from tests/test_config.cpp rather than shared, so the two test files
@@ -643,5 +651,135 @@ void TestQueryCompleter::keysFallThroughWhileThePopupIsHidden()
     QVERIFY(ran);
 }
 
+void TestQueryCompleter::arrowNavigationDoesNotRecurse()
+{
+    // QCoreApplication::sendEvent re-runs application-level event filters, so a
+    // filter that forwards the key it just claimed to another widget is handed
+    // the same key back. With the popup still visible the guard still passes and
+    // it forwards again: unbounded recursion, and the process dies on the stack
+    // rather than on any assertion. Reaching the end of this test is the check.
+    Config config;
+    QLineEdit edit;
+    edit.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&edit));
+    edit.setFocus();
+    QueryCompleter completer(&edit, config);
+
+    QTest::keyClicks(&edit, QStringLiteral("t"));
+    QVERIFY(findPopup() && findPopup()->isVisible());
+
+    QTest::keyClick(keyboardTarget(&edit), Qt::Key_Down);
+    QTest::keyClick(keyboardTarget(&edit), Qt::Key_Down);
+
+    // And navigation actually moved, so the fix is not "swallow the key".
+    QListView *popup = findPopup();
+    QVERIFY(popup);
+    QVERIFY(popup->currentIndex().isValid());
+    QCOMPARE(popup->currentIndex().row(), 1);
+}
+
+void TestQueryCompleter::returnRunsTheQueryOnceCompletionIsDone()
+{
+    // The second half of the user's report: with the query finished, Return has
+    // to reach returnPressed and run it. A popup left visible over a completed
+    // term swallows Return forever, and the query can never be run from the
+    // keyboard at all.
+    Config config;
+    QLineEdit edit;
+    edit.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&edit));
+    edit.setFocus();
+    QueryCompleter completer(&edit, config);
+    completer.setTags({ QStringLiteral("unread"), QStringLiteral("inbox") });
+
+    bool ran = false;
+    connect(&edit, &QLineEdit::returnPressed, &edit, [&ran]() { ran = true; });
+
+    // Drive the whole chain the way the user does: prefix, accept, value, accept.
+    QTest::keyClicks(&edit, QStringLiteral("t"));
+    QVERIFY(findPopup() && findPopup()->isVisible());
+    QTest::keyClick(keyboardTarget(&edit), Qt::Key_Tab);
+    QCOMPARE(edit.text(), QStringLiteral("tag:"));
+
+    QTest::keyClicks(&edit, QStringLiteral("un"));
+    QVERIFY(findPopup() && findPopup()->isVisible());
+    QTest::keyClick(keyboardTarget(&edit), Qt::Key_Tab);
+    QCOMPARE(edit.text(), QStringLiteral("tag:unread"));
+
+    // The query is complete. Return must now run it, not be eaten.
+    QTest::keyClick(keyboardTarget(&edit), Qt::Key_Return);
+    QVERIFY(ran);
+}
+
+void TestQueryCompleter::returnRunsTheQueryAfterAMouseAccept()
+{
+    // The user builds the whole query with the mouse, which never goes through
+    // the key filter, and then Return does not run it. Clicking a row is what
+    // QCompleter reports as activated(), so drive that and then press Return
+    // exactly as the user does.
+    Config config;
+    QLineEdit edit;
+    edit.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&edit));
+    edit.setFocus();
+    QueryCompleter completer(&edit, config);
+    completer.setTags({ QStringLiteral("unread"), QStringLiteral("inbox") });
+
+    bool ran = false;
+    connect(&edit, &QLineEdit::returnPressed, &edit, [&ran]() { ran = true; });
+
+    QTest::keyClicks(&edit, QStringLiteral("t"));
+    QListView *popup = findPopup();
+    QVERIFY(popup && popup->isVisible());
+
+    // Click the "tag:" row.
+    const QModelIndex prefixRow = popup->model()->index(0, 0);
+    QVERIFY(prefixRow.isValid());
+    popup->setCurrentIndex(prefixRow);
+    QTest::mouseClick(popup->viewport(), Qt::LeftButton, Qt::NoModifier,
+                      popup->visualRect(prefixRow).center());
+    QCOMPARE(edit.text(), QStringLiteral("tag:"));
+
+    // Then click a tag value in the popup the accept chained open.
+    popup = findPopup();
+    QVERIFY(popup && popup->isVisible());
+    const QModelIndex valueRow = popup->model()->index(0, 0);
+    QVERIFY(valueRow.isValid());
+    popup->setCurrentIndex(valueRow);
+    QTest::mouseClick(popup->viewport(), Qt::LeftButton, Qt::NoModifier,
+                      popup->visualRect(valueRow).center());
+    QCOMPARE(edit.text(), QStringLiteral("tag:unread"));
+
+    // The query is complete and built entirely with the mouse. Return runs it.
+    QTest::keyClick(keyboardTarget(&edit), Qt::Key_Return);
+    QVERIFY(ran);
+}
+
+void TestQueryCompleter::returnRunsTheQueryWhenThePopupMatchesNothing()
+{
+    // A query the user finished by hand. The bar is mid-token, so the popup is
+    // still up, but nothing in it matches what was typed. Return must run the
+    // query: there is no completion to accept, and accepting the first row of
+    // an unrelated list would rewrite the query the user just wrote.
+    Config config;
+    QLineEdit edit;
+    edit.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&edit));
+    edit.setFocus();
+    QueryCompleter completer(&edit, config);
+    completer.setTags({ QStringLiteral("unread"), QStringLiteral("inbox") });
+
+    bool ran = false;
+    connect(&edit, &QLineEdit::returnPressed, &edit, [&ran]() { ran = true; });
+
+    // "zzz" matches no tag, so the popup has nothing to offer for it.
+    QTest::keyClicks(&edit, QStringLiteral("tag:zzz"));
+
+    QTest::keyClick(keyboardTarget(&edit), Qt::Key_Return);
+
+    QCOMPARE(edit.text(), QStringLiteral("tag:zzz"));
+    QVERIFY(ran);
+}
+
 QTEST_MAIN(TestQueryCompleter)
 #include "test_querycompleter.moc"