From dd6f35b8f35f231ebf1511daef3dd707eaa9839d Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 24 Aug 2026 11:02:08 +0200 Subject: fix(compose): default to quoting below, and focus the body on a reply The previous commit fixed the cursor within each quote_position branch and the user still saw the old layout, because the branches were already right: what was wrong was the DEFAULT. quote_position shipped as `above`, and the layout asked for is exactly what `below` already produced, a blank line at the top with the quote underneath. So the default flips, along with the fallback for a malformed value and the warning naming it. Nothing needs an Upgrading note: compose has not been released, so no config in the wild sets this. Focus goes to the body whenever To: is already filled, which a Reply and a Forward always are. The form's first widget took it otherwise, so the user had to click into the editor before typing. A New message keeps the default, since an empty To: is genuinely the first thing to fill in. The focus test asserts on the window's focusWidget() rather than on QWidget::hasFocus(): an unshown window is never active, so hasFocus() reads false whatever the code does and would fail against a correct fix. Both directions are mutation-checked, since focusing unconditionally passes the reply case while breaking the new-message one. --- tests/test_config.cpp | 12 ++++++------ tests/test_mainwindow.cpp | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/test_config.cpp b/tests/test_config.cpp index a902425..c46b153 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -130,7 +130,7 @@ private slots: void garbageSendDelayIsRejectedNotZero(); void garbageAttachmentWarnBytesIsRejectedNotZero(); void zeroOrNegativeAutosaveIntervalIsClamped(); - void unrecognisedQuotePositionWarnsAndFallsBackToAbove(); + void unrecognisedQuotePositionWarnsAndFallsBackToBelow(); }; static QString writeIni(const QTemporaryDir &dir, const QString &body) @@ -2359,8 +2359,8 @@ void TestConfig::composeSettingsDefaultWhenTheSectionIsAbsent() config.load(writeIni(dir, QStringLiteral("[general]\n"))); const ComposeSettings compose = config.compose(); - QVERIFY2(compose.quotePosition == ComposeSettings::QuotePosition::Above, - "default quote position must be Above"); + QVERIFY2(compose.quotePosition == ComposeSettings::QuotePosition::Below, + "default quote position must be Below"); QVERIFY2(compose.sendHtml, "default send_html must be true"); QCOMPARE(compose.autosaveIntervalMs, 30000); QCOMPARE(compose.sendDelayMs, 5000); @@ -2492,7 +2492,7 @@ void TestConfig::zeroOrNegativeAutosaveIntervalIsClamped() .arg(negative.compose().autosaveIntervalMs))); } -void TestConfig::unrecognisedQuotePositionWarnsAndFallsBackToAbove() +void TestConfig::unrecognisedQuotePositionWarnsAndFallsBackToBelow() { // Matches the precedent set by sync_on_exit, language and date_format: // the only silent fallbacks in this file are for ABSENT keys, never for @@ -2503,8 +2503,8 @@ void TestConfig::unrecognisedQuotePositionWarnsAndFallsBackToAbove() "[compose]\n" "quote_position=abov\n"))); - QVERIFY2(config.compose().quotePosition == ComposeSettings::QuotePosition::Above, - "an unrecognised quote_position must still fall back to Above"); + QVERIFY2(config.compose().quotePosition == ComposeSettings::QuotePosition::Below, + "an unrecognised quote_position must still fall back to Below"); QVERIFY2(!config.problems().isEmpty(), "an unrecognised quote_position was accepted silently"); } diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 9e4543c..84a6fd1 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -487,6 +487,7 @@ private slots: void aDisabledAttachmentWarningWarnsAboutNothing(); void theQuotePositionDecidesWhereTheQuoteLands(); void theCursorStartsOnBlankSpaceNotOnTheQuote(); + void aReplyOpensWithTheBodyFocused(); void theSeededQuoteIsNotAnUndoStep(); void aReplySeedsTheHtmlToggleFromTheOriginal(); void aNewMessageSeedsTheHtmlToggleFromConfig(); @@ -12525,6 +12526,48 @@ void TestMainWindow::theCursorStartsOnBlankSpaceNotOnTheQuote() } } +void TestMainWindow::aReplyOpensWithTheBodyFocused() +{ + // A Reply arrives with To: already filled, so the first widget in the form + // taking focus means the user has to click into the editor before typing. + // A New message is the opposite case and keeps the default. + ComposeFixture fixture; + QVERIFY(fixture.build()); + + { + ComposeContext context = newContext(); + context.kind = ComposeContext::Kind::Reply; + context.to = { QStringLiteral("someone@example.org") }; + context.quotedBody = QStringLiteral("> the original"); + + ComposeWindow window(context, fixture.config(), fixture.mailRoot()); + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + // focusWidget(), not hasFocus(): an unshown window is never active, so + // hasFocus() is false whatever the code does and the assertion would + // fail against a correct fix. This is the same class of trap CLAUDE.md + // records for the offscreen platform and window sizing. + QVERIFY2(window.focusWidget() == body, + "a reply did not open with the body focused"); + } + + { + // The guard: without it, focusing the body unconditionally would pass + // the assertion above while taking focus off an empty To: field, which + // is the one thing a new message needs first. + ComposeContext context = newContext(); + context.kind = ComposeContext::Kind::New; + context.to.clear(); + + ComposeWindow window(context, fixture.config(), fixture.mailRoot()); + auto *body = window.findChild(QStringLiteral("body")); + auto *to = window.findChild(QStringLiteral("to")); + QVERIFY(body && to); + QVERIFY2(window.focusWidget() != body, + "a new message stole focus from the empty To: field"); + } +} + void TestMainWindow::theSeededQuoteIsNotAnUndoStep() { ComposeFixture fixture; -- cgit v1.2.3