diff options
| author | Danilo M. <danix@danix.xyz> | 2026-09-29 19:41:24 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-09-29 19:41:24 +0200 |
| commit | fca0a2226e52b4e01ba199950a4eb308040c3e11 (patch) | |
| tree | ac99bcbec9ff215bee55aa8734dd6bd69c908ba7 | |
| parent | e4aa085a74b10cdb74d0ea2aa5105da7de397aa4 (diff) | |
| download | qtmaildir-fca0a2226e52b4e01ba199950a4eb308040c3e11.tar.gz qtmaildir-fca0a2226e52b4e01ba199950a4eb308040c3e11.zip | |
fix: select a launch's row whatever the keyboard last held
A --message launch handed to a window already in use switched the list
to the conversation and made the reply row current, and the message
pane stayed empty. The same launch into a fresh window worked.
selectRowAt() selected the row and then called the view's
setCurrentIndex(). QAbstractItemView::setCurrentIndex() asks
selectionCommand() what to do with the selection, and with no event to
read it answers from QGuiApplication::keyboardModifiers(), which is the
modifier state of the last input event the application received rather
than anything the user is doing now. With Control in that state the
command is a Toggle: the row selected a line earlier was deselected
again, onThreadSelected() refused a current row that is not selected,
and nothing was loaded. The provisional thread row the recovery selects
first went the same way, so the pane never left the placeholder. A
fresh process has received no input at all, which is why only a window
in use was affected.
The current index is now moved through the selection model with
NoUpdate, so the selection stays exactly what select() made it. Every
caller of selectRowAt() shared the defect (the stale-thread recovery,
double-click, the dashboard's message entries and the launch selectors)
and every one of them means "select exactly this row", so all of them
take the fix.
The test puts a window into use, leaves Control as the last modifier
the application saw, launches --message for a reply in another thread,
and asserts the reply is current, selected, and rendered in the pane.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| -rw-r--r-- | src/mainwindow.cpp | 14 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 104 |
2 files changed, 117 insertions, 1 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 3c94c5c..19bf3c2 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -187,7 +187,19 @@ void MainWindow::selectRowAt(const QModelIndex &index) m_threadView->selectionModel()->select( index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); - m_threadView->setCurrentIndex(index); + // NoUpdate, through the selection model, and never the view's own + // setCurrentIndex(). That one asks selectionCommand() what to do with the + // selection, and with no event to read it answers from + // QGuiApplication::keyboardModifiers(): the modifiers of the LAST input + // event this application saw, not what the user is doing now. With + // Control there, the command is a Toggle, so the row selected just above + // was deselected again, onThreadSelected() refused a current row that is + // not selected, and the pane stayed empty. A fresh window has seen no + // input, which is why a launch into a new window worked and one handed + // to a window in use did not. The selection is already exactly what the + // caller asked for; only the current index moves here. + m_threadView->selectionModel()->setCurrentIndex( + index, QItemSelectionModel::NoUpdate); } /// Selects the top-level thread row at `row`. diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 8ffa806..a22c153 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -326,6 +326,7 @@ private slots: void anUnknownAccountSelectorLeavesTheDropdownAlone(); void aThreadSelectorOpensThatThread(); void aMessageSelectorOpensItsThread(); + void aMessageSelectorRendersItsReplyWhateverTheKeyboardLastHeld(); void aThreadSelectorThatIsNotHexIsRefused(); void anEmptySelectorSetChangesNothing(); void aBracketedMessageSelectorOpensThatMessage(); @@ -18010,6 +18011,109 @@ void TestMainWindow::aMessageSelectorOpensItsThread() QCOMPARE(model->rowCount(QModelIndex()), 1); } +void TestMainWindow::aMessageSelectorRendersItsReplyWhateverTheKeyboardLastHeld() +{ + // --message for a reply, handed to a window that is already in use. The + // list switched to the conversation and the reply row became current, and + // the pane stayed empty. + // + // A window in use has received keyboard input, and a fresh one has not. + // QAbstractItemView::setCurrentIndex() takes its selection command from + // QGuiApplication::keyboardModifiers() when there is no event, which is + // the modifier state of the LAST input event the application saw. With + // Control there, the programmatic selection became a Toggle: the row the + // recovery had just selected was deselected again, onThreadSelected() + // refused a current row that is not selected, and nothing loaded. + // + // Simulated with a Control press that is never released, which is what + // the application's state looks like when focus left it with Control + // held. Released on every exit, because the state is process-wide and + // would otherwise leak into every later test. + WorkerBackedWindow backed; + // Dates verified with `date -d <yyyy-mm-dd> +%A`: Qt::RFC2822Date + // validates the weekday against the date. + QVERIFY(backed.fixture().addMessage( + QStringLiteral("inbox"), QStringLiteral("b1@example.org"), + QStringLiteral("Bravo opens"), QStringLiteral("c@example.org"), + QStringLiteral("Sun, 16 Aug 2026 10:00:00 +0200"), + QStringLiteral("Bravo one."))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("inbox"), QStringLiteral("b2@example.org"), + QStringLiteral("Bravo target reply"), QStringLiteral("d@example.org"), + QStringLiteral("Mon, 17 Aug 2026 10:00:00 +0200"), + QStringLiteral("Bravo two."), true, + QStringLiteral("b1@example.org"))); + QVERIFY(backed.fixture().addMessage( + QStringLiteral("inbox"), QStringLiteral("c1@example.org"), + QStringLiteral("Charlie alone"), QStringLiteral("e@example.org"), + QStringLiteral("Tue, 18 Aug 2026 10:00:00 +0200"), + QStringLiteral("Charlie one."))); + QVERIFY2(backed.build(), qPrintable(backed.error())); + + MainWindow window(backed.config()); + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<ThreadListView *>(); + QVERIFY(view); + auto *pane = window.findChild<MessageView *>(); + QVERIFY(pane); + auto *header = pane->findChild<QLabel *>(QStringLiteral("messageHeader")); + QVERIFY(header); + auto *queryEdit = window.findChild<QLineEdit *>(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + + // Put the window into use: another message on display. + queryEdit->setText(QStringLiteral("tag:inbox")); + emit queryEdit->returnPressed(); + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000); + NotmuchWorker probe(backed.config().notmuchConfig()); + const QString other = + probe.threadIdForTesting(QStringLiteral("id:c1@example.org")); + QVERIFY(!other.isEmpty()); + QModelIndex otherRow; + for (int row = 0; row < model->rowCount(QModelIndex()); ++row) { + if (model->threadAt(row).threadId == other) + otherRow = model->index(row, 0, QModelIndex()); + } + QVERIFY(otherRow.isValid()); + view->selectionModel()->select( + otherRow, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + view->setCurrentIndex(otherRow); + QTRY_VERIFY_WITH_TIMEOUT( + !pane->showingPlaceholder() + && header->text().contains(QStringLiteral("Charlie alone")), + 15000); + + QTest::keyPress(view, Qt::Key_Control, Qt::ControlModifier); + const auto release = qScopeGuard( + [view]() { QTest::keyRelease(view, Qt::Key_Control); }); + // A guard proving the state under test is really there: without it the + // assertions below pass for the ordinary reason. + QCOMPARE(QGuiApplication::keyboardModifiers(), Qt::ControlModifier); + + LaunchSelectors selectors; + selectors.messageId = QStringLiteral("b2@example.org"); + window.applySelectors(selectors); + + // The REPLY, not the root the recovery falls back to, current AND + // selected: a current row that is not selected is what the defect left. + QTRY_VERIFY_WITH_TIMEOUT( + view->currentIndex().isValid() + && model->isMessageRow(view->currentIndex()) + && model->messageAt(view->currentIndex()).messageId + == QStringLiteral("b2@example.org"), + 15000); + QTRY_VERIFY_WITH_TIMEOUT( + view->selectionModel()->isSelected(view->currentIndex()), 15000); + + // What the user sees is that message: not the placeholder, not the + // conversation's dashboard. + QTRY_VERIFY_WITH_TIMEOUT( + !pane->showingDashboard() && !pane->showingPlaceholder() + && header->text().contains(QStringLiteral("Bravo target reply")), + 15000); +} + void TestMainWindow::aThreadSelectorThatIsNotHexIsRefused() { // The thread id now comes from another program's command line or the |
