diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-03 16:23:43 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-03 16:23:43 +0200 |
| commit | 212048782be680e2f99341db6c6460e59c708e7c (patch) | |
| tree | 2a963993682c71e17d9dc1d165ac7ae1aa9e572c | |
| parent | e6ea00dce8c74bf0d48e769734958a741ca46ef9 (diff) | |
| download | qtmaildir-212048782be680e2f99341db6c6460e59c708e7c.tar.gz qtmaildir-212048782be680e2f99341db6c6460e59c708e7c.zip | |
feat: own the message-pane zoom and persist it
Zoom was Chromium's, not the application's: the web view handled the
keys natively and never told anyone, so there was no value to save.
qtmaildir now owns it. Zoom in, out and reset are real actions, in the
View menu and rebindable through [keys], and the factor is persisted to
the UI state file. Ctrl+wheel over the body zooms and Ctrl+middle-click
resets, both filtered by ancestry from an application-level filter: the
events are delivered to an internal QQuickWidget the web view creates
lazily, so a filter on the view itself never sees them.
The factor is clamped to 0.5 - 3.0, and NaN, infinity, zero and negative
values fall back to 1.0, since a corrupt state file must not be able to
leave the pane unreadable with no visible way back.
Both risks the plan flagged turned out not to exist, verified by probe
rather than assumed. The application QAction wins over the web view's
native zoom key, so the tracked factor cannot diverge from what is on
screen. And the factor survives setHtml(), so the web view is the single
source of truth and needs no reapply per render.
A third finding is worth recording because it produced a wrong fix
first. A probe using QTest::keyClick() reported Ctrl++ as a dead
binding, and a test was written asserting that. Both were wrong: Ctrl++
is exactly what the '+' key emits on an Italian layout, confirmed
against the real keyboard, and it is the shipped default. Whether a
symbol needs Shift is a property of the layout, not of Qt, and
keyClick() reproduces neither. The test now only checks that every
default parses, and the comment in defaultBindings() says not to
re-derive this from synthetic input.
Ctrl+= is a second binding for reset, skipped when [keys] gives it to
something else.
Also fixes a pre-existing bug the new config key exposed. [general]
entries were read as "general/<key>", which matches nothing: QSettings'
INI backend treats a section literally named [general] as its own
fallback section and strips the prefix. notmuch_config had therefore
never worked. Both keys are now read without it; the file format is
unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| -rw-r--r-- | CLAUDE.md | 17 | ||||
| -rw-r--r-- | README.md | 4 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 34 | ||||
| -rw-r--r-- | src/config.cpp | 26 | ||||
| -rw-r--r-- | src/config.h | 6 | ||||
| -rw-r--r-- | src/keymap.cpp | 12 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 38 | ||||
| -rw-r--r-- | src/messageview.cpp | 93 | ||||
| -rw-r--r-- | src/messageview.h | 24 | ||||
| -rw-r--r-- | tests/test_config.cpp | 56 | ||||
| -rw-r--r-- | tests/test_keymap.cpp | 20 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 3 | ||||
| -rw-r--r-- | tests/test_messageview.cpp | 55 |
13 files changed, 385 insertions, 3 deletions
@@ -77,9 +77,24 @@ Per-account subdirectories *are* configured, since notmuch does not model accoun **Config format gotcha:** QSettings treats `/` in a section name as a group separator, so account sections are `[account.work]`, not `[account/work]`. `childKeys` returns keys -sorted alphabetically, never in file order. Config lives at +sorted alphabetically, never in file order. **`[general]` keys are read WITHOUT the +`general/` prefix** — QSettings' INI backend treats a section literally named `[general]` +as its own fallback section and strips it, so a `general/<key>` lookup silently matches +nothing (this is how `notmuch_config` went unnoticed as broken). Config lives at `~/.config/qtmaildir/qtmaildir.conf`. +Machine-written UI state is a **separate** file, `~/.local/state/qtmaildir/uistate.conf` +via `MainWindow::uiStatePath()`. Never write window blobs into the hand-edited config. +Build the path from `QStandardPaths::GenericStateLocation`, not `StateLocation`: the +latter appends both the organization and the application name, and both are `qtmaildir`. + +**Do not conclude a key binding is dead from `QTest::keyClick()`.** Whether a symbol needs +Shift is a layout property, not a Qt one. `Ctrl++` is the shipped `zoom_in` default and is +exactly what the `+` key emits on an Italian layout, while synthetic input never delivers +it. Verify against a real keyboard before changing a default on reachability grounds. The +separate, real trap `normalizeSequence()` handles is a **bare capital** (`N` parses to +unshifted Key_N, which no keystroke emits). + ## Web view security The most security-sensitive area: a browser engine pointed at input from strangers. Do not @@ -79,6 +79,10 @@ identity. ; Optional. Omit to let notmuch resolve its own config, which is what keeps ; the GUI and the CLI pointed at one database. ; notmuch_config = /home/you/.notmuch-config +; Optional. Starting zoom of the message pane, 0.5 to 3.0. Only the starting +; point: once you zoom with Ctrl+wheel or Ctrl+/Ctrl-, that is remembered +; separately and this value no longer applies. +; message_zoom = 1.0 [sync] ; Optional. Omit and the Sync button disables itself with a tooltip. diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index fc4ddbd..11863a8 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -37,7 +37,7 @@ taking that too literally. | 1 | Splitter/column widths do not survive restart | persistence | S | **done** | | 2 | No way to see full message details (From/To/Cc/Subject) | information | M | open | | 3 | Too few clickable affordances, shortcuts are the only route | discoverability | M | **done** | -| 4 | Message-pane font size does not survive restart | persistence | S | open | +| 4 | Message-pane font size does not survive restart | persistence | S | **done** | | 5 | Thread list is cramped, poor readability | presentation | S | open | | 6 | Opened message stays unread | behavior | S | open | | 7 | HTML view should be default for HTML messages | behavior | XS | **verify first, may already be done** | @@ -261,6 +261,38 @@ already exists. - Route the actions through item 3's `QAction` conversion so they appear in the View menu, which also makes the reset discoverable. +### Outcome (done) + +Built as described, and both of the plan's stated risks turned out not to +exist. Probed rather than assumed: + +- **The application `QAction` wins over Chromium's native zoom key.** The plan + called this "the one real risk in the item". It is not one: the action fires + and the web view's own handling never runs, so the tracked factor cannot + diverge from what is on screen. +- **Zoom survives `setHtml()`.** The plan expected the view might reset it on + navigation and asked for a reapply per render. Not needed; the web view keeps + the factor, so it is the single source of truth and there is no second copy. +- **Do not test key reachability with synthetic input.** A probe using + `QTest::keyClick()` reported `Ctrl++` as a dead binding, and a test was + written asserting it. Both were wrong: `Ctrl++` is exactly what the `+` key + emits on an Italian layout, confirmed against the real keyboard, and it is + the shipped default. Whether a symbol needs Shift is a property of the + layout, not of Qt, and `keyClick()` reproduces neither. The test now only + checks that every default parses. +- `Ctrl+=` is a second binding for reset, skipped when `[keys]` gives `Ctrl+=` + to something else. Ctrl+wheel zooms and Ctrl+middle-click resets, both + filtered by ancestry from an application-level filter: the events land on an + internal `QQuickWidget` the web view creates lazily, so a filter installed on + the view itself never sees them. + +**A pre-existing bug surfaced while adding the config key.** `[general]` +entries were read as `general/<key>`, which matches nothing: QSettings' INI +backend treats a section literally named `[general]` as its own fallback +section and strips the prefix. `notmuch_config` had therefore never worked. +Both keys are now read without the prefix; the file format the user writes is +unchanged. Regression test in `test_config`. + ## 5. Thread list is cramped **Observed:** rows are tightly packed, everything is uniform, the UI reads as diff --git a/src/config.cpp b/src/config.cpp index f21bba9..a2caaee 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -52,8 +52,32 @@ void Config::load(const QString &path) { QSettings settings(path, QSettings::IniFormat); + // Keys of [general] are read WITHOUT the "general/" prefix. QSettings' + // INI backend treats a section literally named [general] as its own + // fallback section and strips the prefix, so "general/notmuch_config" + // never matches anything, in any section arrangement (verified on + // Qt 6.11). The file still reads as [general] to the user; only the + // lookup differs. Same family of trap as the [account.work] dot and the + // childKeys() ordering already documented in CLAUDE.md. m_notmuchConfig = - settings.value(QStringLiteral("general/notmuch_config")).toString(); + settings.value(QStringLiteral("notmuch_config")).toString(); + + // Absent is fine and silent: the default is 1.0. Present but unparseable + // is a problem, since the user asked for something and is not getting it. + // The range check lives in MessageView::clampZoom(), the one place that + // knows what the web view can render. + const QVariant zoom = settings.value(QStringLiteral("message_zoom")); + if (zoom.isValid()) { + bool ok = false; + const double value = zoom.toString().toDouble(&ok); + if (ok) { + m_messageZoom = value; + } else { + addProblem(QStringLiteral("Message zoom '%1' is not a number; " + "using the default.") + .arg(zoom.toString())); + } + } m_syncCommand = settings.value(QStringLiteral("sync/command")).toString(); if (m_syncCommand.isEmpty()) { diff --git a/src/config.h b/src/config.h index 943cbd8..3e94249 100644 --- a/src/config.h +++ b/src/config.h @@ -78,6 +78,11 @@ public: /// Optional alternate notmuch config file. Empty means "let notmuch decide". QString notmuchConfig() const { return m_notmuchConfig; } + /// Starting message-pane zoom for a profile with no saved UI state. Once + /// the user zooms, the state file remembers that instead, so this is only + /// ever the default. Clamped by MessageView::clampZoom() on use. + qreal messageZoom() const { return m_messageZoom; } + /// Every non-fatal problem, both kinds below. Shown in the status bar. QStringList warnings() const { return m_warnings; } @@ -101,6 +106,7 @@ private: QList<SavedQuery> m_savedQueries; QString m_syncCommand; QString m_notmuchConfig; + qreal m_messageZoom = 1.0; QStringList m_warnings; QStringList m_problems; }; diff --git a/src/keymap.cpp b/src/keymap.cpp index 42ccd40..22c8da1 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -35,6 +35,9 @@ QStringList KeyMap::knownActions() QStringLiteral("focus_query"), QStringLiteral("toggle_html"), QStringLiteral("load_remote"), + QStringLiteral("zoom_in"), + QStringLiteral("zoom_out"), + QStringLiteral("zoom_reset"), QStringLiteral("undo"), QStringLiteral("sync"), QStringLiteral("quit"), @@ -63,6 +66,15 @@ QList<QPair<QString, QString>> KeyMap::defaultBindings() { QStringLiteral("Ctrl+L"), QStringLiteral("focus_query") }, { QStringLiteral("Ctrl+H"), QStringLiteral("toggle_html") }, { QStringLiteral("Ctrl+M"), QStringLiteral("load_remote") }, + // Ctrl++ is what the '+' key really delivers on a layout where '+' is + // unshifted, an Italian one among them, confirmed against the actual + // keyboard. QTest::keyClick() cannot reproduce it, so a synthetic-input + // probe wrongly reports this binding as dead; do not "fix" it on that + // evidence. A US layout, where '+' is Shift+'=', wants Ctrl+Shift+= in + // [keys] instead. + { QStringLiteral("Ctrl++"), QStringLiteral("zoom_in") }, + { QStringLiteral("Ctrl+-"), QStringLiteral("zoom_out") }, + { QStringLiteral("Ctrl+0"), QStringLiteral("zoom_reset") }, { QStringLiteral("Ctrl+Z"), QStringLiteral("undo") }, { QStringLiteral("Ctrl+G"), QStringLiteral("sync") }, { QStringLiteral("Ctrl+Q"), QStringLiteral("quit") }, diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 081984d..c4955b6 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -105,6 +105,13 @@ void MainWindow::restoreUiState() if (!header.isEmpty()) { m_threadView->horizontalHeader()->restoreState(header); } + + // The config value is the starting point for a profile that has never + // zoomed; once the user does, the state file is what they last had. + // clampZoom() rejects the garbage a hand-edited file can hold. + m_messageView->setZoomFactor( + state.value(QStringLiteral("message/zoom"), m_config.messageZoom()) + .toDouble()); } void MainWindow::saveUiState() const @@ -116,6 +123,7 @@ void MainWindow::saveUiState() const state.setValue(QStringLiteral("window/splitter"), m_splitter->saveState()); state.setValue(QStringLiteral("threadlist/header"), m_threadView->horizontalHeader()->saveState()); + state.setValue(QStringLiteral("message/zoom"), m_messageView->zoomFactor()); } void MainWindow::closeEvent(QCloseEvent *event) @@ -381,6 +389,32 @@ void MainWindow::registerActions() tr("Load remote images for the current thread"), [this]() { m_messageView->loadRemoteContent(); }); + addAction(QStringLiteral("zoom_in"), tr("Zoom &in"), + tr("Enlarge the message text"), [this]() { + m_messageView->zoomIn(); + }); + addAction(QStringLiteral("zoom_out"), tr("Zoom &out"), + tr("Shrink the message text"), [this]() { + m_messageView->zoomOut(); + }); + auto *zoomReset = + addAction(QStringLiteral("zoom_reset"), tr("&Actual size"), + tr("Return the message text to its default size"), [this]() { + m_messageView->zoomReset(); + }); + + // Ctrl+= alongside the configured binding: '=' reads as "back to normal", + // and on a layout where '+' is Shift+'=' it is the unshifted key next to + // zoom in. Appended rather than assigned, so a [keys] override of + // zoom_reset keeps working and simply gains this as a second way in. + // A user who bound Ctrl+= to something else in [keys] keeps their binding. + const QKeySequence altReset(QStringLiteral("Ctrl+=")); + if (m_keyMap.actionFor(altReset).isEmpty()) { + QList<QKeySequence> shortcuts = zoomReset->shortcuts(); + shortcuts.append(altReset); + zoomReset->setShortcuts(shortcuts); + } + addAction(QStringLiteral("undo"), tr("&Undo"), tr("Undo the last tag change"), [this]() { if (m_undoStack.canUndo()) @@ -428,6 +462,10 @@ void MainWindow::buildMenus() viewMenu->addSeparator(); viewMenu->addAction(m_actions.value(QStringLiteral("toggle_html"))); viewMenu->addAction(m_actions.value(QStringLiteral("load_remote"))); + viewMenu->addSeparator(); + viewMenu->addAction(m_actions.value(QStringLiteral("zoom_in"))); + viewMenu->addAction(m_actions.value(QStringLiteral("zoom_out"))); + viewMenu->addAction(m_actions.value(QStringLiteral("zoom_reset"))); auto *helpMenu = menuBar()->addMenu(tr("&Help")); auto *shortcuts = helpMenu->addAction(tr("&Keyboard shortcuts")); diff --git a/src/messageview.cpp b/src/messageview.cpp index aebb81b..9a1d3e4 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -21,8 +21,12 @@ #include <QDesktopServices> #include <QHBoxLayout> #include <QLabel> +#include <QApplication> +#include <QMouseEvent> #include <QPushButton> +#include <QtNumeric> #include <QTimer> +#include <QWheelEvent> #include <QVBoxLayout> #include <QWebEnginePage> #include <QWebEngineProfile> @@ -101,6 +105,13 @@ MessageView::MessageView(QWidget *parent) settings->setAttribute(QWebEngineSettings::PluginsEnabled, false); settings->setAttribute(QWebEngineSettings::FullScreenSupportEnabled, false); + // Ctrl+wheel zoom. The filter goes on the application rather than on + // m_view: the wheel event is delivered to an internal QQuickWidget the + // view creates lazily, so there is no child to filter at this point and a + // filter on m_view itself would never see it. eventFilter() narrows by + // ancestry, so no event outside this pane is touched. + qApp->installEventFilter(this); + m_headerLabel = new QLabel(this); m_headerLabel->setTextFormat(Qt::RichText); m_headerLabel->setWordWrap(true); @@ -281,6 +292,88 @@ void MessageView::toggleHtml() render(); } +bool MessageView::eventFilter(QObject *watched, QEvent *event) +{ + const QEvent::Type type = event->type(); + if (type != QEvent::Wheel && type != QEvent::MouseButtonPress) + return QWidget::eventFilter(watched, event); + + // Application-wide filter: only events inside this pane are ours. Anything + // else, including a Ctrl+wheel over the thread list, passes untouched. + // isAncestorOf() is false for the widget itself, so test that separately. + auto *widget = qobject_cast<QWidget *>(watched); + if (!widget || (widget != m_view && !m_view->isAncestorOf(widget))) + return QWidget::eventFilter(watched, event); + + if (type == QEvent::Wheel) { + auto *wheel = static_cast<QWheelEvent *>(event); + if (!(wheel->modifiers() & Qt::ControlModifier)) + return QWidget::eventFilter(watched, event); + + // angleDelta is in eighths of a degree; one detent is 120. A high + // resolution wheel sends smaller steps, so scale rather than treating + // every event as one full step. + const int delta = wheel->angleDelta().y(); + if (delta != 0) + setZoomFactor(zoomFactor() + 0.1 * delta / 120.0); + + // Consumed, or Chromium's own Ctrl+wheel zoom would run on top of + // ours and the factor we track would no longer be what is on screen. + return true; + } + + // Ctrl+middle-click resets: the same hand that just zoomed with the wheel + // puts it back, without reaching for the keyboard. + auto *mouse = static_cast<QMouseEvent *>(event); + if (mouse->button() != Qt::MiddleButton + || !(mouse->modifiers() & Qt::ControlModifier)) { + return QWidget::eventFilter(watched, event); + } + + zoomReset(); + + // Consumed: a plain middle click is paste-on-X11 in some contexts, and + // this gesture must do one thing only. + return true; +} + +qreal MessageView::clampZoom(qreal factor) +{ + // qIsFinite rejects the NaN and infinity a corrupt or hand-edited state + // file can produce; qFuzzyIsNull rejects the 0.0 that a missing or + // non-numeric value converts to, which would render nothing at all. + if (!qIsFinite(factor) || factor <= 0.0) + return kDefaultZoom; + return qBound(kMinZoom, factor, kMaxZoom); +} + +qreal MessageView::zoomFactor() const +{ + // The web view is the single source of truth. It keeps the factor across + // setHtml(), verified on Qt 6.11, so there is no second copy to drift. + return m_view->zoomFactor(); +} + +void MessageView::setZoomFactor(qreal factor) +{ + m_view->setZoomFactor(clampZoom(factor)); +} + +void MessageView::zoomIn() +{ + setZoomFactor(zoomFactor() + 0.1); +} + +void MessageView::zoomOut() +{ + setZoomFactor(zoomFactor() - 0.1); +} + +void MessageView::zoomReset() +{ + setZoomFactor(kDefaultZoom); +} + void MessageView::loadRemoteContent() { // Applies to this thread only and is cleared by the next showThread(). diff --git a/src/messageview.h b/src/messageview.h index 9570db5..09e7d71 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -65,13 +65,37 @@ public: /// Tags of the thread on display, shown as chips along the bottom. void setTags(const QStringList &tags); + /// The body zoom factor. Chromium's own range is roughly 0.25 to 5.0; + /// these are tighter, since a pane at either extreme is unusable and the + /// only visible way back is a menu entry the user cannot read. + static constexpr qreal kMinZoom = 0.5; + static constexpr qreal kMaxZoom = 3.0; + static constexpr qreal kDefaultZoom = 1.0; + + /// Clamps to [kMinZoom, kMaxZoom]. A non-finite or non-positive value, + /// which is what a corrupt state file yields, falls back to kDefaultZoom. + static qreal clampZoom(qreal factor); + + qreal zoomFactor() const; + void setZoomFactor(qreal factor); + public slots: void toggleHtml(); void loadRemoteContent(); + void zoomIn(); + void zoomOut(); + void zoomReset(); signals: void statusMessage(const QString &text); +protected: + /// Turns Ctrl+wheel over the body into zoom, and Ctrl+middle-click into a + /// reset. Both events are delivered to the web view's internal QQuickWidget + /// focus proxy, not to the view itself, so this filters the whole subtree + /// rather than one widget. + bool eventFilter(QObject *watched, QEvent *event) override; + private: void render(); void updateHeader(); diff --git a/tests/test_config.cpp b/tests/test_config.cpp index e492480..367643c 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -34,6 +34,8 @@ private slots: void brokenSyncCommandIsAProblem(); void malformedAccountIsAProblem(); void validConfigHasNoProblems(); + void generalSectionKeysAreActuallyRead(); + void messageZoomDefaultsAndValidates(); }; static QString writeIni(const QTemporaryDir &dir, const QString &body) @@ -222,5 +224,59 @@ void TestConfig::validConfigHasNoProblems() QVERIFY(config.warnings().isEmpty()); } +void TestConfig::generalSectionKeysAreActuallyRead() +{ + // QSettings' INI backend treats a section literally named [general] as its + // own fallback section and strips the prefix, so a "general/<key>" lookup + // matches nothing. notmuch_config was read that way and had never worked. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral( + "[general]\n" + "notmuch_config=/somewhere/notmuch-config\n" + "\n" + "[sync]\n" + "command=/bin/true\n"))); + + QCOMPARE(config.notmuchConfig(), + QStringLiteral("/somewhere/notmuch-config")); +} + +void TestConfig::messageZoomDefaultsAndValidates() +{ + // A QTemporaryDir per case, not one shared: writeIni() always uses the + // same file name, and QSettings caches by path, so a second load of the + // same path would return the first case's contents. + + // Absent: 1.0, silently. Nothing the user asked for is being ignored. + { + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n"))); + QCOMPARE(config.messageZoom(), 1.0); + QVERIFY(config.problems().isEmpty()); + } + + { + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n" + "message_zoom=1.25\n"))); + QCOMPARE(config.messageZoom(), 1.25); + QVERIFY(config.problems().isEmpty()); + } + + // Present but unparseable is a problem: the user asked for something and + // is not getting it, which is the line addProblem() draws. + { + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n" + "message_zoom=huge\n"))); + QCOMPARE(config.messageZoom(), 1.0); + QCOMPARE(config.problems().size(), 1); + } +} + QTEST_MAIN(TestConfig) #include "test_config.moc" diff --git a/tests/test_keymap.cpp b/tests/test_keymap.cpp index c81eeb0..0fb4f57 100644 --- a/tests/test_keymap.cpp +++ b/tests/test_keymap.cpp @@ -36,8 +36,28 @@ private slots: void userBindingWinsOverDefaultInMenus(); void defaultsDoNotCollide(); void everyDefaultIsAKnownAction(); + void everyDefaultParses(); }; +void TestKeyMap::everyDefaultParses() +{ + // A default that does not parse is a dead binding, the failure mode + // bareCapitalMatchesShiftedPress() covers for user-written keys. + // + // This deliberately does NOT try to decide which keys a keyboard can + // deliver. Whether a symbol needs Shift is a layout property, not a Qt + // one: Ctrl++ is exactly what the '+' key emits on an Italian layout and + // is unreachable on a US one, and QTest::keyClick() cannot reproduce + // either faithfully. A test asserting reachability from synthetic input + // would encode one layout's habits as a rule for all of them. + for (const auto &binding : KeyMap::defaultBindings()) { + const QKeySequence sequence = KeyMap::normalizeSequence(binding.first); + QVERIFY2(!sequence.isEmpty(), + qPrintable(QStringLiteral("default '%1' for %2 does not parse") + .arg(binding.first, binding.second))); + } +} + void TestKeyMap::defaultsAreLoaded() { KeyMap map; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 628472b..b4d9e4a 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -28,6 +28,7 @@ #include "config.h" #include "keymap.h" #include "mainwindow.h" +#include "messageview.h" /// MainWindow is mostly wiring, and the parts that need a real database are /// still verified manually. What is checked here is the action registry: the @@ -190,6 +191,7 @@ void TestMainWindow::uiStateSurvivesARestart() const Config config; MainWindow window(config); window.resize(resized); + window.findChild<MessageView *>()->setZoomFactor(1.4); window.close(); // closeEvent() is what persists the state } @@ -200,6 +202,7 @@ void TestMainWindow::uiStateSurvivesARestart() const Config config; MainWindow reopened(config); QCOMPARE(reopened.size(), resized); + QCOMPARE(reopened.findChild<MessageView *>()->zoomFactor(), 1.4); QFile::remove(MainWindow::uiStatePath()); QStandardPaths::setTestModeEnabled(false); diff --git a/tests/test_messageview.cpp b/tests/test_messageview.cpp index 0c3353d..4b6bc4c 100644 --- a/tests/test_messageview.cpp +++ b/tests/test_messageview.cpp @@ -36,6 +36,8 @@ private slots: void documentActuallyLoads(); void threadContentReachesThePage(); void dataUrlSubResourceStillBlocked(); + void zoomIsClampedToARenderableRange(); + void zoomSurvivesANewDocument(); private: QWebEngineView *webViewOf(MessageView *view) const @@ -175,5 +177,58 @@ void TestMessageView::dataUrlSubResourceStillBlocked() QVERIFY(text.contains(QStringLiteral("visible-text"))); } +void TestMessageView::zoomIsClampedToARenderableRange() +{ + // A factor outside the range leaves the pane unreadable, and the only way + // back is a menu entry the user can no longer read. A corrupt state file + // reaching setZoomFactor() must not be able to do that. + QCOMPARE(MessageView::clampZoom(100.0), MessageView::kMaxZoom); + QCOMPARE(MessageView::clampZoom(0.01), MessageView::kMinZoom); + + // A missing or non-numeric state value converts to 0.0, and a hand-edited + // one can hold NaN or an infinity. None of those may reach the web view. + QCOMPARE(MessageView::clampZoom(0.0), MessageView::kDefaultZoom); + QCOMPARE(MessageView::clampZoom(-2.0), MessageView::kDefaultZoom); + QCOMPARE(MessageView::clampZoom(qQNaN()), MessageView::kDefaultZoom); + QCOMPARE(MessageView::clampZoom(qInf()), MessageView::kDefaultZoom); + + // In-range values pass through untouched. + QCOMPARE(MessageView::clampZoom(1.4), 1.4); + + MessageView view; + view.setZoomFactor(50.0); + QCOMPARE(view.zoomFactor(), MessageView::kMaxZoom); +} + +void TestMessageView::zoomSurvivesANewDocument() +{ + // MainWindow persists whatever zoomFactor() reports and never reapplies it + // per render, which is only correct if the web view keeps the factor + // across setHtml(). Verified rather than assumed. + MessageView view; + QWebEngineView *web = webViewOf(&view); + QVERIFY(web); + + view.setZoomFactor(1.5); + + QSignalSpy loaded(web, &QWebEngineView::loadFinished); + + ParsedMessage message; + message.ok = true; + message.from = QStringLiteral("Sender <sender@example.org>"); + message.subject = QStringLiteral("Zoom"); + message.plainBody = QStringLiteral("body text"); + + ThreadRenderItem item; + item.message = message; + item.cidPrefix = QStringLiteral("m0"); + item.expanded = true; + + view.showThread({ item }); + QVERIFY(loaded.wait(15000)); + + QCOMPARE(view.zoomFactor(), 1.5); +} + QTEST_MAIN(TestMessageView) #include "test_messageview.moc" |
