From aa86d61c1a716c2e0a2cdc55ae396808eeb494d1 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 3 Aug 2026 21:51:55 +0200 Subject: fix(completion): filter keys on the application, not the line edit Showing the popup takes focus away from the query bar and the popup window grabs the keyboard, so keys pressed while it is up are delivered to the popup. An event filter installed on the line edit therefore never ran at the one moment it had to, leaving Tab to move focus to the next widget and Return to reach the thread list and open a message. Install the key filter on the application instead, which sees events before any widget receives them. It returns immediately unless our own popup is visible, so it cannot affect keyboard handling elsewhere. The FocusIn filter stays on the line edit, where it is correctly scoped: it only fires with the popup down. Accepting a completion now also reopens the popup when the caret lands somewhere more can be offered, so taking "tag:" goes straight on to the tag list instead of needing a second complete_query. The chain stops on a stem that is already a complete candidate, which is what every accept produces. The mouse path chains identically. The previous tests passed against the broken code because they posted events straight to the line edit, bypassing the delivery path a real keypress takes. The new tests route keys through the active popup and run against a real X display; offscreen does not grab the keyboard and cannot reproduce this class of bug. --- tests/test_querycompleter.cpp | 146 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) (limited to 'tests') diff --git a/tests/test_querycompleter.cpp b/tests/test_querycompleter.cpp index 5ae9312..c1d7530 100644 --- a/tests/test_querycompleter.cpp +++ b/tests/test_querycompleter.cpp @@ -63,6 +63,16 @@ private slots: void tabIsIgnoredWhileThePopupIsHidden(); void returnIsIgnoredWhileThePopupIsHidden(); void focusOpensThePopupOnlyWhenConfigured(); + + // Delivered to the widget the window system actually gives the key to, + // rather than straight to the line edit. While the popup is up that is the + // popup, which has grabbed the keyboard, and a filter on the line edit + // never runs. Sending to the edit hides exactly the bug the user reports. + void tabAcceptsWhenTheKeyGoesToTheGrabbingPopup(); + void returnAcceptsWhenTheKeyGoesToTheGrabbingPopup(); + void acceptingAPrefixReopensThePopupForValues(); + void acceptingAValueDoesNotReopenAnEmptyPopup(); + void keysFallThroughWhileThePopupIsHidden(); }; // Copied from tests/test_config.cpp rather than shared, so the two test files @@ -497,5 +507,141 @@ void TestQueryCompleter::focusOpensThePopupOnlyWhenConfigured() } } +// The widget the window system would hand the next key to. While the popup is +// up it has grabbed the keyboard, so that is the popup and NOT the line edit, +// which has by then lost focus entirely. Routing test keys through here is what +// makes these tests reproduce the user's experience instead of a synthetic one. +static QWidget *keyboardTarget(QLineEdit *edit) +{ + if (QWidget *popup = QApplication::activePopupWidget()) + return popup; + return edit; +} + +void TestQueryCompleter::tabAcceptsWhenTheKeyGoesToTheGrabbingPopup() +{ + 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_Tab); + + QCOMPARE(edit.text(), QStringLiteral("tag:")); +} + +void TestQueryCompleter::returnAcceptsWhenTheKeyGoesToTheGrabbingPopup() +{ + // Return must be consumed too, or it reaches the thread list and opens a + // thread, which is what the user sees. + Config config; + QLineEdit edit; + edit.show(); + QVERIFY(QTest::qWaitForWindowExposed(&edit)); + edit.setFocus(); + QueryCompleter completer(&edit, config); + + bool ran = false; + connect(&edit, &QLineEdit::returnPressed, &edit, [&ran]() { ran = true; }); + + QTest::keyClicks(&edit, QStringLiteral("t")); + QVERIFY(findPopup() && findPopup()->isVisible()); + + QTest::keyClick(keyboardTarget(&edit), Qt::Key_Return); + + QCOMPARE(edit.text(), QStringLiteral("tag:")); + QVERIFY(!ran); +} + +void TestQueryCompleter::acceptingAPrefixReopensThePopupForValues() +{ + // The user's third complaint: after taking "tag:" the caret sits where a + // tag value goes, so the values must be offered without a second Ctrl+Space. + Config config; + QLineEdit edit; + edit.show(); + QVERIFY(QTest::qWaitForWindowExposed(&edit)); + edit.setFocus(); + QueryCompleter completer(&edit, config); + completer.setTags({ QStringLiteral("unread"), QStringLiteral("inbox") }); + + QTest::keyClicks(&edit, QStringLiteral("t")); + QVERIFY(findPopup() && findPopup()->isVisible()); + + QTest::keyClick(keyboardTarget(&edit), Qt::Key_Tab); + QCOMPARE(edit.text(), QStringLiteral("tag:")); + + QListView *popup = findPopup(); + QVERIFY(popup); + QVERIFY(popup->isVisible()); + QStringList offered; + for (int row = 0; row < popup->model()->rowCount(); ++row) + offered << popup->model()->index(row, 0).data().toString(); + QCOMPARE(offered, QStringList({ QStringLiteral("unread"), + QStringLiteral("inbox") })); + + // And the chain completes: typing into the reopened popup and accepting + // yields the finished term. + QTest::keyClicks(&edit, QStringLiteral("un")); + QVERIFY(findPopup() && findPopup()->isVisible()); + QTest::keyClick(keyboardTarget(&edit), Qt::Key_Tab); + QCOMPARE(edit.text(), QStringLiteral("tag:unread")); +} + +void TestQueryCompleter::acceptingAValueDoesNotReopenAnEmptyPopup() +{ + // "tag:unread" is complete. Reopening here would put an empty list under + // the caret and swallow the next Return. + Config config; + QLineEdit edit; + edit.show(); + QVERIFY(QTest::qWaitForWindowExposed(&edit)); + edit.setFocus(); + QueryCompleter completer(&edit, config); + completer.setTags({ QStringLiteral("unread") }); + + QTest::keyClicks(&edit, QStringLiteral("tag:un")); + QVERIFY(findPopup() && findPopup()->isVisible()); + + QTest::keyClick(keyboardTarget(&edit), Qt::Key_Tab); + + QCOMPARE(edit.text(), QStringLiteral("tag:unread")); + QVERIFY(!findPopup() || !findPopup()->isVisible()); +} + +void TestQueryCompleter::keysFallThroughWhileThePopupIsHidden() +{ + // The filter is application-wide, so proving it does nothing with the popup + // down is what keeps it from breaking the rest of the application. + Config config; + QLineEdit edit; + edit.show(); + QVERIFY(QTest::qWaitForWindowExposed(&edit)); + edit.setFocus(); + QueryCompleter completer(&edit, config); + + if (QListView *popup = findPopup()) + popup->hide(); + + QLineEdit other; + other.show(); + QVERIFY(QTest::qWaitForWindowExposed(&other)); + other.setFocus(); + + bool ran = false; + connect(&other, &QLineEdit::returnPressed, &other, [&ran]() { ran = true; }); + + QTest::keyClicks(&other, QStringLiteral("hello")); + QTest::keyClick(&other, Qt::Key_Return); + + QCOMPARE(other.text(), QStringLiteral("hello")); + QVERIFY(ran); +} + QTEST_MAIN(TestQueryCompleter) #include "test_querycompleter.moc" -- cgit v1.2.3