diff options
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 552 |
1 files changed, 151 insertions, 401 deletions
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 a422317..9cb8ba9 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 @@ -131,12 +131,12 @@ taking that too literally. | 62 | No config option for the date format on a card | presentation | XS | **done** 2026-08-11 | | 63 | No way to see sent mail, and no filter for it | workflow | M | **done** 2026-08-11; see `specs/2026-08-11-sent-mail-design.md` | | 64 | The Sync button carries a mailbox icon, not a refresh one | presentation | XS | **done** 2026-08-11 | -| 65 | No full code review and optimization pass | correctness | ? | open, unspecified | +| 65 | No full code review and optimization pass | correctness | ? | open, **narrowed 2026-08-26**: the notes now name it as a dead-code and duplication sweep, not a performance or security pass. Produces a LIST for the user to decide on, not a diff. See the entry | | 66 | Selecting a thread root leaves the message pane blank until a reply has been selected | defect | S | **done** 2026-08-14, unreleased. Not the blank pane it was filed as: the root rendered the CONVERSATION until the thread had been expanded once, then one message. Now always one message, and the conversation view is removed at the user's request. **One case unverified by hand:** the notes also report a single-message `id:` query whose card would not open, which is the same empty-`MessageIdRole` failure and should be gone; confirmed 2026-08-15 as a SEPARATE defect with a different cause, see item 96 | | 87 | Auto mark-read marks a whole thread, including replies never displayed | defect | S | **done** 2026-08-16, unreleased. Built on 108, which is why it stayed small: the timer tracks a MESSAGE id now, and arms for a reply too, which it never did before | | 88 | `threadAt(current.row())` answers about the wrong thread for a reply row | defect | M | **done** 2026-08-16, unreleased. The audit found FOUR live sites, not one. `ThreadListModel::threadFor(index)` resolves a reply through its parent; every caller holding a selected index converted, and no `.row()` on a selected index remains in `mainwindow.cpp`. Unblocks 87 | | 67 | The placeholder pane counts unread, flagged and inbox, but not sent or drafts | information | XS | **done** 2026-08-11, shipped in 0.15.0 | -| 68 | A forwarded subject gets no `passed` tag | workflow | S | open; no subject rule exists, measured 2026-08-11. Decision needed: display mark (XS) or write the flag (S, syncs out) | +| 68 | A forwarded subject gets no `passed` tag | workflow | S | **done 2026-08-26**, unreleased, as THREE things once the premise was measured away. The note asked to expand a subject rule to `Fw:`; there was no subject rule, and the correlation it rested on did not exist. What did exist was a gap nobody had reported: qtmaildir has never written `R` or `P`, so a reply and a forward now flag their source (off the undo stack, per the auto-mark-read precedent), and `subjectIsForwarded()` drives a SEPARATE received-forward mark, display only, extendable through `[general] forward_prefixes`. The user chose all three | | 69 | `passed` and `replied` read as words where every other state is a glyph | presentation | S | **done** 2026-08-11, inside item 70 | | 70 | Pane icons are a private set where the main window uses the system theme | presentation | M | **done** 2026-08-11; six shipped SVGs | | 71 | A toolbar action does not sync, so the edit sits until the next cron run | workflow | S | **done** 2026-08-11; 2s default, `auto_sync_delay_ms` | @@ -165,12 +165,12 @@ taking that too literally. | 96 | A query returning the thread already on display opens onto the placeholder | defect | S | **done** 2026-08-15, unreleased. Split from 66's unverified half, which had a different cause. Reproduced from two screenshots after four measured eliminations | | 97 | An edit made during a sync is reverted in the list when the sync ends | defect | S | **done** 2026-08-15, unreleased. Found by hand-testing item 89's fix. The sync-end refresh ran BEFORE the held-edit flush, so it read a database that still carried the old tag | | 98 | "Important" adds the tag but cannot remove it, unlike every other toggle | defect | XS | **done** 2026-08-17, unreleased. Calls `everySelectedRowHasTag()`, as the entry required. Its reply test needed THREE different states (list-first thread, the reply's own thread, the reply) before it could tell the two wrong answers apart; with the reply defaulted to its thread's state the item 105 mutation stayed green, measured | -| 99 | The unread action is labelled "Toggle unread" whichever way it will go | presentation | S | open; depends on 98's toggle shape, and the label is harder than it looks | +| 99 | The unread action is labelled "Toggle unread" whichever way it will go | presentation | S | **done 2026-08-25**, unreleased, with 112: the user's note is ONE design across both. The label names the direction it will go, and the entry is hidden on a selection with no single state. `refreshUnreadAction()` reads the new three-valued `selectionTagPresence()` | | 100 | The message pane offers Back, Forward, Reload and Save page, none of which mean anything | defect | XS | **done** 2026-08-17, unreleased. `MessageView::removeBrowserActions()` filters the standard menu by `pageAction()` POINTER, never by text; `ViewSource` went with them, and stranded separators are swept | | 101 | Sync is account-aware for edits but not for the account the user is looking at | workflow | S | open; item 49 built the edit half deliberately. Needs a decision, see the entry | | 102 | The rules table shows no note, so the field explaining a rule is invisible until it is opened | workflow | XS | **done** 2026-08-17, unreleased. A Note column before `ColumnCount`, so the appended Matches column stays last. Found a second defect on the way: `restoreState` REFUSES a header state with a different column count, and the sized flags were being set regardless | | 103 | What Delete does to mail on the server is undocumented and unverified | clarification | S+M | done; Delete moves to the account trash, with Restore and a stranded-mail cleanup. Section in the closed file | -| 104 | Mail visible in Thunderbird never reaches qtmaildir | defect | ? | open, reported 2026-08-16, cause NOT established. Most likely outside this repo; see the entry before writing code | +| 104 | Mail visible in Thunderbird never reaches qtmaildir | defect | XS | **done 2026-08-25**, hand-tested. The worker never reopened its read-only notmuch handle, so no query saw mail indexed after startup. Confirmed on a sync run from the application that added 20 messages: they appeared without a restart | | 109 | A root card's own message is invisible to a message-scoped write | defect | S | **done** 2026-08-16, unreleased. Found by hand-testing 108. `applyMessageTagChange` and `messageById` searched only the loaded replies, and a root's message is never among them, so the ORDINARY gesture repainted nothing and wiped the pane's chip row | | 110 | A card and the message pane show tags belonging to a message's siblings | defect | S | **done** 2026-08-16, unreleased. Found by hand-testing 109 against a real 4-message thread. `ThreadSummary::tags` is notmuch's UNION; a card standing for one message drew it. Also the reason a root card could not repaint at all | | 111 | A card should show its siblings' tags smaller, not drop them | presentation | S | **done** 2026-08-16, unreleased. The user's own design, from looking at 110's result: own tags full size, the thread's others smaller and muted, so nothing appears to vanish on selection | @@ -178,14 +178,14 @@ taking that too literally. | 106 | A tag change made on one message during a sync is silently lost | defect | XS | **done** 2026-08-16, unreleased. Found by READING while fixing 105, never reported. `flushHeldEdits` re-sent only thread-scoped edits, so a message-scoped one was shown, counted as pending, and never written | | 107 | A thread-scoped write leaves the loaded replies showing their old tags | defect | XS | **done** 2026-08-16, unreleased. `applyTagChange` updated the summary only, so marking a thread read left its expanded replies bold | | 108 | Acting on a thread root means the whole thread, though it displays one message | workflow | M | **done** 2026-08-16, unreleased. `messageScopeFor()` beside `scopeFor()`; five `*_thread` actions in a "Whole thread" submenu on `Ctrl+Alt+<key>`. User-visible: minor bump, `### Upgrading` written | -| 112 | Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread | defect | S | open, found 2026-08-17. A toggle over a UNION has no direction on a mixed thread | +| 112 | Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread | defect | S | **done 2026-08-25**, unreleased. Built to the user's own note rather than to this entry's approach, which had it only half right. The thread toggle splits into two absolute actions AND the message-scoped one keeps its toggle with a dynamic label, hidden when the selection disagrees. Closes 99 and 147 with it | | 113 | No way to see a message's HTML source | information | S | open, 2026-08-17. Chromium's own View source cannot work here; needs our own plain-text dialog. Item 100 removed the dead entry, which was an overreach: the user had not asked for it | | 114 | Save image is offered on every image and does nothing | defect | S | open, found 2026-08-17, re-confirmed by hand 2026-08-20. No `downloadRequested` handler exists, so the request is emitted and never answered. The handler is per-profile, so it must decide per request or it revives the Save link item 127 removed | | 115 | A copy from the message pane gives no confirmation | presentation | XS | **done** 2026-08-19, unreleased. Four entries report, each naming what it copied; connected to the page's own QActions, so the entry is covered wherever it is triggered from | | 116 | Copy image copies markup instead of the image | defect | XS | **dropped** 2026-08-17, same day. NOT A DEFECT: `wl-paste --list-types` run immediately after a copy reports `image/png`, `application/x-qt-image` and 30 more image flavours. The clipboard is correct and Chromium is behaving. The earlier "text only" reading was taken minutes late off a clipboard that had been overwritten, and a whole cause was theorised on it | | 117 | The message pane offers no Select all | workflow | XS | **done** 2026-08-19, unreleased. `addPaneActions()` supplies it. The call site is NOT covered by a test and cannot be: the production menu needs a real context-menu event. Stated in the test rather than faked | -| 118 | No way to empty the trash from inside the app | workflow | S | open, 2026-08-17. **Blocked on 103**, which creates the trash in the first place. Deliberately left out of 103's spec at the user's request rather than squeezed in | -| 119 | The unsynced-changes count cannot be opened to see what it counts | information | S | open, 2026-08-19, from the notes. One of the four things it sums carries no message ids at all, so a list cannot be complete without a change to how the count is kept | +| 118 | No way to empty the trash from inside the app | workflow | S | **done 2026-08-25**, unreleased. Unblocked by 103. `Message > Empty trash...`, scoped to the account selector, no shortcut. The one confirmation in this application, and CLAUDE.md now records it as the single exception rather than leaving it to be discovered. Found a defect while testing: the count claimed messages whose files were already gone | +| 119 | The unsynced-changes count cannot be opened to see what it counts | information | S | **done** 2026-08-26, unreleased. **The stated blocker was not real**: the fourth term counted confirmed changes with no message ids, and `applyTags()` returns early on exactly that condition, so it could never fire. Measured before removing it, not read. The label opens a read-only list, grouped as the user asked: subject once, actions beneath. Scope follows the ACTION, so a held thread edit stays one thread row and reports its message count. A snapshot, frozen once open | | 121 | The thread list shows nothing while a query is running | feedback | S | open, 2026-08-20, from the notes. Follows item 74, which fixed the status-bar half and left the list itself blank | | 122 | The README documents a version of the app that no longer exists | documentation | M | **done** 2026-08-23, unreleased, inside item 123 task 13. `trash`, `send_command` and the whole `[compose]` section were undocumented; a Composing section is added and "sending is not implemented" removed. Every default was read from `config.h` rather than from the prose, which caught `send_html` documented as false when it defaults to true | @@ -202,7 +202,7 @@ taking that too literally. | 130 | A message cannot be attached to another message directly | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** A `message/rfc822` part, which GMime builds natively. The manual route exists from 123's first commit: `save_message` writes the `.eml` and it is attached as a file | | 131 | The markdown dialect and extensions are fixed | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** Configurable in the shape Hugo's config uses. Deliberately fixed initially: CommonMark plus autolink, strikethrough and tasklist | | 132 | Every action must have a shortcut, and that no longer serves | policy | S | done, 2026-08-20. `everyActionHasAShortcut` is deleted and nothing replaces it: `everyActionIsReachableFromAMenu()` is the required rule and a shortcut is now a chosen subset. Nothing else needed changing, since `showShortcutReference()` already printed `(unbound)` for an empty sequence. Verified by unbinding `tag_rules` and running the suite green, which would have failed before | -| 133 | The composer shows no markdown syntax highlighting | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** A `QSyntaxHighlighter` over the composer's editor, so `**bold**` reads as bold while the buffer stays plain markdown. Standard Qt, no dependency. Deliberately after 123's formatting toolbar: agreeing with the grammar about nesting and about code spans suppressing what is inside them is the expensive part, and the toolbar is what makes the feature usable | +| 133 | The composer shows no markdown syntax highlighting | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** A `QSyntaxHighlighter` over the composer's editor, so `**bold**` reads as bold while the buffer stays plain markdown. Standard Qt, no dependency. Deliberately after 123's formatting toolbar: agreeing with the grammar about nesting and about code spans suppressing what is inside them is the expensive part, and the toolbar is what makes the feature usable . **Subsumed by item 173** 2026-08-27: that is a rich-text editor, this is the cheap answer to the same want. Build one or the other, never both | | 134 | The busy indicator is built inline and is about to be built twice | maintenance | S | done, 2026-08-20, af902e0. `BusyIndicator` (`src/busyindicator.h`) carries both modes: `MainWindow` uses the indeterminate one, and item 123's send popup takes the determinate half for its undo countdown, switching the same widget over when the command starts. Only the BAR was extracted, not the status label this row paired with it. `m_statusLabel` has 34 uses across `MainWindow` for transient messages, selection counts and sync phases, so it belongs to the window rather than to the indicator, and the send popup owns its own phase text | | 135 | The formatting toolbar's buttons stack rather than toggle | v2 | S | open, 2026-08-21, asked for by the user during item 123 task 8 and reverted the same session. **A spec change, not a defect**: it conflicts with spec:236 ("deliberately no live toggle") and spec:187-190. Both sites need amending FIRST, and the amendment must resolve what replaces bold-then-italic, which is the gesture spec:187's preserved selection exists to serve and which a toggle makes unreachable. That question is the work; the state machine is understood and written up in the section | | 136 | `undoMovesTheMessageBack` fails when run ALONE, passes in the full suite | defect | ? | open, 2026-08-21, re-measured 2026-08-24 and it is not what the row said. Filed as an intermittent race (1 in 6); it is in fact **deterministic on the selection**: 6 failures in 6 when named on the command line, and, as of 2026-08-24, it fails in the FULL run too: measured at 58f13ad with the day's work stashed out, 274 passed and this one failed. The "passes in the suite" half of this row is therefore no longer true, and the selection-dependence it was named for may not be either. Re-measure before theorising. All three of its 15s `QTRY` timeouts expire, giving 45s against a 25s whole-suite run, so undo never moves the file rather than losing a race. A test that needs its predecessors is the likely shape (the `init()` lock-table fixture of item 61 is one candidate), which makes it a TEST defect until shown otherwise. Not caused by item 149, and re-confirmed 2026-08-24 as not caused by item 152 either, by running the test at the preceding commit in a throwaway worktree. The assertion that fails names the real question: the restored file is in NEITHER `cur` nor `new` of the account inbox, so establish where it went before theorising about a race | @@ -215,8 +215,8 @@ taking that too literally. | 143 | The formatting buttons are text, where every editor uses icons | presentation | XS | **done** 2026-08-24, unreleased, inside 142. `QIcon::fromTheme` per CLAUDE.md's chrome rule, the words kept as the tooltip, and an action whose theme lacks the name keeps its text rather than rendering an empty button | | 144 | "Also send a formatted copy" is prominent and does not say what it does | presentation | XS | **done** 2026-08-24, unreleased, inside 142. "Send as HTML", icon and text, alone at the right end of the editor bar where it reads as a control of the editor rather than as a formatting button. The Italian entry was refreshed with it, and `lrelease` reports 477 finished, 0 unfinished | | 145 | Cc and Bcc are permanent rows on every composer | presentation | S | **done** 2026-08-24, unreleased, inside 142. A `QToolButton` disclosure beside To:. `revealCcBccIfUsed()` is the load-bearing half the entry called for: it only ever SHOWS, never hides, so nothing but the user's own click can make a field holding an address invisible. `ComposeContext` carries no `bcc` at all, so the seeded-Bcc case can only arrive from a reopened draft, which is what its test drives. The LABEL is hidden with each field: a `QFormLayout` holds the two as separate items, so hiding the line edit alone strands a `Cc:` over empty space | -| 146 | The unsynced-changes count cannot be opened to see what it counts | information | S | **duplicate of 119**, recorded 2026-08-23 from the notes. Same request, and 119 already carries the blocker: one of the four things the count sums holds no message ids, so a list cannot be complete without changing how the count is kept | -| 147 | Toggle unread reads the same whichever way it will go | presentation | S | **duplicate of 99**, recorded 2026-08-23 from the notes. The notes ask for exactly what 99 describes: "Mark as read" on an unread message and the reverse. 99 already records that the label is harder than it looks, since a multi-row selection has no single direction | +| 146 | The unsynced-changes count cannot be opened to see what it counts | information | S | **done as 119** 2026-08-26. Duplicate, recorded 2026-08-23 from the notes; closed by the same work | +| 147 | Toggle unread reads the same whichever way it will go | presentation | S | **duplicate of 99**, recorded 2026-08-23 from the notes, and closed with it on 2026-08-25 | | 148 | Ctrl+W does not close the composer | discoverability | XS | **done** 2026-08-24, unreleased. A `QAction` parented to the composer, so it is a WindowShortcut dispatched to the active composer only and the main window's namespace is untouched, exactly like the formatting shortcuts. It calls `close()` rather than doing anything of its own: `closeEvent()` already decides whether the draft is saved, and a second route out that skipped it would lose the message. Not registered in `KeyMap`, so item 132's rules do not apply | | 149 | A reply's cursor lands on the attribution line, not on blank space | defect | XS | **done** 2026-08-24, unreleased, in TWO passes. The first fixed the cursor within each branch (`End` under Above, `Start` under Below) and the user still saw the old layout, because the branches were already right and the DEFAULT was wrong: `above` shipped, and the layout asked for is what `below` produces. Default flipped, and the composer now focuses the body whenever To: is already filled, which a Reply and a Forward always are. Both halves were invisible to the existing `theQuotePositionDecidesWhereTheQuoteLands`, which asserts the quote's position and never the cursor's | | 150 | The receive-only ribbon stays up after the message that raised it is gone | defect | S | **done** 2026-08-24, unreleased. One line in `MessageView::clear()`, beside the blocked-content bar, the stale notice and the attachment bar it already reset by hand. Only `setReceiveOnlyAccount()` hid the ribbon, which every SELECTION change reaches, so a row-to-row move was never the reproducer: it survived the FOUR routes that blank the pane without one (`clear_pane`, `clear_selection`, a new query, a multi-row selection). The first test written for it passed against the defect for exactly that reason | @@ -237,8 +237,16 @@ taking that too literally. | 162 | Delete fails while a sync is renaming the file underneath it | defect | S | **done, 2026-08-25.** mbsync renames an uploaded file to add its `,U=<uid>` infix and notmuch keeps the pre-`U=` name until that sync's `notmuch new` runs, so `moveMessages` renamed a path that no longer existed and Delete silently did nothing while blaming the destination folder. `moveMessages` now re-resolves by MESSAGE ID when the recorded path is gone: one reindex of that directory, then the filename that exists on disk. Bounded to one retry, so a file genuinely gone still reports. Holding the move during a sync was the other candidate and is NOT the fix: `sendMove` already refuses on notmuch's write lock, but this window sits between mbsync's rename and that sync's `notmuch new`, which touches no lock | | 163 | The message pane shows a stale path, and the composer forks the draft | defect | S | **done, 2026-08-25.** mbsync renames an uploaded file to add its `,U=<uid>` infix while the model still holds the name the query returned. `MaildirName::resolveRenamed()` returns the path unchanged when it exists, else finds the file in that one directory whose unique stem matches; it refuses an ambiguous match and yields nothing for a genuinely missing file. Wired into all THREE read sites: the pane, Reply/Forward, and the draft reopen. The reopen was the one that cost data, forking a draft into two files with two Message-IDs, both reaching the server | -| 164 | A draft this application saved keeps `inbox` | defect | S | open, 2026-08-25, **cause corrected 2026-08-25**. The first diagnosis blamed a missing drafts helper and was WRONG: `NOT_ARRIVALS` in `qtmaildirconf.py` is `("sent", "drafts")`, the folder list includes every account's drafts folder, and `notmuch count` confirms the carve-out query MATCHES the affected draft. The carve-out is scoped to `tag:new`, and the draft carries `inbox` while `tag:new` is 0, so it was never in scope when the hook ran. Measured separately: an mbsync-style rename does NOT re-add `new.tags`, so the retag theory is out too. What remains unestablished is WHICH pass tagged it; establish that before writing code | +| 164 | A draft this application saved keeps `inbox` | defect | S | **dropped** 2026-08-27, NOT A DEFECT. The premise was a measurement artifact: its evidence was `notmuch search --output=tags`, which DISPLAYS the union over a thread, and a reply-draft under an arrived message reads `draft inbox unread` while no message carries both. Re-measured at message level: 0 of 12 drafts carry `inbox`, including nine written on or before 2026-08-25. The `unread` half was real and is item 172 | | 165 | A draft gets a new Message-ID on every autosave | enhancement | ? | open, 2026-08-25, found while hand-testing 163 and 164. `MessageBuilder::build()` generates an id unconditionally and every autosave calls it, so each revision is a distinct MESSAGE to notmuch and to the server rather than a new version of one. Invisible while the file is replaced correctly, which item 163's fix restores; it is what turned that fork into two messages rather than one duplicated file. Needs a DECISION on what a draft's identity is before any code: a stable id reused at send, a stable id discarded at send, or the status quo. Neither `ComposeContext` nor `OutgoingMessage` has a field to carry an id, so it is not a changed call site | +| 166 | Mail you send to your own other account loses `inbox` | defect | S | **done 2026-08-25**, unreleased. `sent_only()` keeps a message only when EVERY file is inside a sent folder, which is what the carve-out's docstring already claimed. No query can express it, measured; the root comes from `database.mail_root`, with a split-index fixture the ordinary layout cannot provide. Verified read-only against the live index: 780 of 807 still stripped, 27 spared, no arrival affected | +| 167 | No way to tell one build of an unreleased version from another | enhancement | XS | **done 2026-08-25**, unreleased. The user chose a counter over a git description: `QTMAILDIR_BUILD_NUMBER`, a cmake option ON by default, increments a counter in the BUILD directory on every build and writes `buildnumber.h`. `QTMAILDIR_VERSION_DISPLAY` carries it; `QTMAILDIR_VERSION` stays clean and is what the window title, `applicationVersion` and the release procedure use | +| 168 | Delete is offered on mail already in the trash, and does nothing | defect | S | **done 2026-08-25**, unreleased. Delete is hidden when every selected row is already in its account's trash, Restore when none is, both keyed on the PATH rather than the `deleted` tag. Delete also drops `unread` now, in the same TagChange so one undo returns the folder and the tag together | +| 169 | A card shows the account only as a bar, with no fade and no avatar | presentation | M | **done** 2026-08-26, unreleased, on `card-avatars`, merged fast-forward. Both halves: a `QLinearGradient` from the account colour to the pane's base across the card, and a squircle avatar with initials, given a rect in `CardLayout` so the geometry is asserted without a painter. **Hand-testing found four defects**, all fixed in 9ae43f9: `Avatar::initialsFor()` normalises the display name first (drops the angle-addr, takes the first comma-separated author, unwraps quotes, treats a bare address as no name, requires a word to carry a letter or digit); the two-tone gradient axis spans the DIAMETER rather than a radius, which was letting one hue fill the whole face; the account fade runs right to left, anchored opaque at the card's right edge; and a flat view hashes `ThreadSummary::firstMessageRecipient` rather than the user's own address. The vCard half stays blocked on item 72 | +| 170 | A row that stops matching the view only leaves it on the Delete path | defect | S | open, 2026-08-26, from the notes, **cause found the same day and the premise is NOT stale**. The optimistic REPAINT is universal; the optimistic MEMBERSHIP is not. `removeThreadsWithoutTag()` has exactly one caller, on the move path, so marking a message read in the Unread view repaints the row and leaves it in a list it no longer belongs to | +| 171 | A forwarded HTML message reaches the recipient as plain text | defect | M | **done** 2026-08-27, unreleased. Design in `specs/2026-08-27-forward-html-design.md`. The user chose inline `multipart/alternative` over attaching the original, and remote content stripped by default with a per-forward opt-out. Four parts: `HtmlSanitiser` (an ALLOW-LIST, unlike `namespaceCids()`, because a missed strip is a beacon where a missed rewrite is a broken image), a text fallback for the ~9% of mail with no plain part, the MIME nesting, and the composer control | +| 172 | A draft this application writes is tagged `unread` | defect | XS | **done** 2026-08-27, unreleased. `DraftStore::write()` was called with `"D"`, and `maildir.synchronize_flags` makes notmuch tag anything without `S` as `unread`. Self-healing on the next sync of that folder, which is what made it look intermittent | +| 173 | The composer is a plain-text editor, not WYSIWYG | v2 | L | open, 2026-08-27, **asked for by the user** while hand-testing 171. This is a GUI mail client and should edit rich text the way one does: the forwarded original, and the user's own formatting, visible and editable in place. Supersedes the preview 171 shipped as a middle ground, and **subsumes item 133** (markdown syntax highlighting), which is the same want answered cheaply. See the entry: the draft format and the markdown-as-source-of-truth model both change | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -362,71 +370,36 @@ batches of 200 so a 10k-thread query paints immediately) already holds. An optimization pass with no measurement behind it is the kind of work that produces a large diff and no change a user can notice. -**What it needs before it can be sized.** The user saying which of these they -meant: a correctness/security review of a named area, a specific operation that -feels slow with the query that makes it slow, a dead-code and duplication sweep, -or the translatability audit that is already item 22. The first three are -different pieces of work with different sizes, and the fourth is already -recorded. - -**Size: `?`, unspecified.** Do not propose a design for this; ask. - -## 68. A forwarded subject gets no `passed` tag - -**Observed (user, from the notes):** "passed tag should appear when subject is -`Fwd:` and `Fw:`." Refined in session on 2026-08-11: the user had noticed -`passed` appearing on messages whose subject carried `Fwd:` and not on `Fw:`, -and asked to expand the rule to both. - -**Cause:** there is no rule to expand. `passed` is the Maildir `P` flag in the -message filename, translated into a tag by notmuch because -`maildir.synchronize_flags=true`. The flag is written by whichever client -forwarded the message, or by the server over IMAP; nothing reads a subject line -anywhere in the chain. qtmaildir only ever colours the tag -(`src/tagcolors.cpp:36-37`) and the database's `post-new` hook does not mention -it either. - -**Measured against the real database (2026-08-11):** - -| Query | Count | -|---|---| -| `tag:passed` | 6 | -| `tag:passed and subject:"Fwd:"` | 1 | -| `tag:passed and subject:"Fw:"` | 0 | -| `subject:"Fwd:" and not tag:passed` | 194 | -| `subject:"Fw:" and not tag:passed` | 28 | - -Six tagged messages in the whole database, and every one of them carries `P` in -its filename flags. The single overlap with `Fwd:` is a message that was -forwarded and whose subject was already a forward, not evidence of a rule: 194 -`Fwd:` subjects carry no tag at all. The correlation the observation rests on -does not exist. - -**Approach and the decision it needs first.** Two different features, and the -measurements above decide how far apart they are. - -*Display only.* The card shows a forwarded mark when the subject matches. Touches -no mail, changes no flag, reversible by deleting the rule. XS. - -*Write the tag.* qtmaildir sets `P` from a subject heuristic. With -`maildir.synchronize_flags=true` that flag is a filename change that mbsync -carries out to the server, on 222 existing messages, on a guess about a string. -Not cleanly undoable, and it asserts a meaning for a flag this application did -not define. Recommended against; recorded so the choice is deliberate rather than -forgotten. - -**Constraints:** localised clients use their own prefixes, and `Fwd:` can appear -inside a subject rather than at its head, so whatever matches must be anchored. -If the tag is ever written, it must not be re-applied on every sync in a way that -produces pending edits the user never made, item 28 is the record of a count -going wrong. The display-only route avoids that entirely, since it derives the -mark at paint time and stores nothing. - -**Size: S** as written, XS if it is display only. Most of it is the decision, not -the code. - -**Status:** left open deliberately on 2026-08-11. The cause is settled and the -options are costed; the user has not chosen, and no code was written. +**Narrowed by the user, 2026-08-26.** The notes now name two sub-bullets, and +they are the same piece of work rather than two: "deduplication of +functionalities" and "check for dead code (functionalities superseded by other +additions, rendering them useless now)". So this is a dead-code and duplication +sweep, NOT a performance pass and not a security review. Nothing slow has been +reported, and the translatability audit it might have meant is item 22, already +done. + +**What that makes it.** A read of the whole tree looking for a function with a +newer twin and for a path nothing reaches any more. The codebase has precedent +for both: `threadAt(int)` survives beside `threadFor(index)` for one legitimate +caller, `SubjectDelegate` was deleted outright at item 53, and item 132 deleted +a whole test rule that had stopped serving. The output is a LIST first, one +entry per candidate with the evidence that it is dead or duplicated, not a +diff; the user decides what goes. + +**Constraints.** + +- "Unreachable from the UI" is not the same as dead. Item 16's + double-press-to-undelete branch reads as dead and is not, because stranded + mail reaches it. Every candidate needs the reachability argument written out + before it is cut. +- A test is a caller. Deleting production code with only test callers is + usually right; deleting the test with it needs saying so explicitly. +- The sweep is worth nothing if it is not run against a green suite before and + after, since the whole value is that nothing observable changed. + +**Size: still `?` until the list exists.** The sweep that produces the list is +S to M; what it finds is the work. + ## 72. No khard/khal integration @@ -535,140 +508,6 @@ reaches it (item 42), so most of this exists. **Size: S** for the on-demand button, XS for the visibility half. Ask which. -## 104. Mail visible in Thunderbird never reaches qtmaildir - -**Observed (user, from the notes):** "sync doesn't work compared to thunderbird. -New mail received on thunderbird did not appear in qtmaildir. Need to investigate -further." - -**Cause: NOT established.** Recorded because it is a defect report about mail -going missing, which is the most serious kind this backlog carries, and it has -been sitting in the notes unrecorded. What follows is one measured mechanism that -would produce exactly this symptom, not a diagnosis. - -**qtmaildir cannot show what mbsync did not fetch, and mbsync fetches folders by -pattern.** Three of the five channels in the user's `~/.mbsyncrc` name their -folders explicitly: - -``` -Patterns "INBOX" "[Gmail]/Posta inviata" "[Gmail]/Bozze" "[Gmail]/Speciali" -``` - -and one names only `"INBOX"`. The two non-Gmail channels use `Patterns *`. -Gmail applies labels, and a message whose label is not one of those four is in a -folder mbsync never asks for. Thunderbird speaks IMAP directly and sees every -folder, so the same message is visible there and absent locally. This is a -configuration property of the user's mbsyncrc, outside this repository entirely. - -**One inconsistency worth reporting regardless**, found while checking the -above: one of the Gmail accounts is configured in `qtmaildir.conf` with -`sent = [Gmail]/Posta inviata` and `drafts = [Gmail]/Bozze`, while its mbsync -channel has `Patterns "INBOX"` and fetches neither. The Sent and Drafts filters -for that account can therefore only ever be empty. That is real, and it is -independent of whatever this item turns out to be. - -**Approach.** Reproduce before anything else, and the reproduction has to -distinguish three layers, because the fix lives in a different place for each: - -1. Is the message on disk? `find` in the Maildir, or `notmuch count` on a term - from it. If not, this is mbsync or `.mbsyncrc`, and there is nothing to - change here. -2. If it is on disk, is it indexed? `notmuch new` and count again. If not, this - is notmuch config, `new.ignore` or the hook. -3. Only if it is indexed and still not shown is this qtmaildir's defect, and - then the question is which query hid it: the account scope, the built-in - filter, or a rule that tagged it out of the inbox. - -**Constraints.** - -- Ask the user for one concrete example before investigating: which account, - roughly when, and what Thunderbird shows for it. A general "sync doesn't work" - cannot be reproduced, and the last four defects in this backlog were all found - from a specific message. -- The `post-new` hook from mailctl tags mail unattended. A rule that removes - `inbox` would make a correctly fetched, correctly indexed message vanish from - the default view, which looks identical to a sync failure from the outside. - `notmuch search` without a filter is what tells them apart. -- Do not change `.mbsyncrc` as part of this. It is the user's, it is outside the - repo, and a Patterns change refetches folders. - -**Size: `?`** until reproduced. Most likely not a code change here at all. - - -## 112. Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread - -**Observed (user, 2026-08-17):** clicking a thread root and asking to mark the -whole thread unread does not do it. On a seven-message thread with two unread -replies, the result is that every message is toggled unread **except those -two**, which are left as they were. The user asks for an explicit "mark whole -thread read/unread" rather than a toggle. - -**Cause (verified in code):** the action exists, and its direction is the -defect. `toggle_unread_thread` (`src/mainwindow.cpp:931`, `Ctrl+Alt+U`) chooses -between adding and removing by asking -`everySelectedRowHasTag("unread", TagScope::Thread)`, which reads -`ThreadListModel::threadFor(index).tags`. That is notmuch's **union over the -thread** (`CLAUDE.md`, item 110), so a thread containing even one unread message -answers "unread" and the action picks *Mark thread read*. There is no input a -user can give that reaches *Mark thread unread* on a mixed thread: the only -threads that take that branch are the ones already entirely read, and the only -threads reporting "not unread" are the ones the user does not need the action -for. - -The write itself is absolute and correct. `tagSelected` with `TagScope::Thread` -adds or removes `unread` across every message, so the two unread replies in the -report are not skipped by the write. They are the reason the write ran in the -opposite direction from the one the user wanted. - -**A union is not a state, and a toggle needs a state.** This is the same class -as item 110 and the third time the union has produced a defect. Items 105 and 88 -fixed *which object* a toggle resolved; this one is about a thread having no -single answer to give. `everySelectedRowHasTag` is a two-valued predicate over a -three-valued reality: all read, all unread, or mixed. The mixed case is the one -that has no correct toggle direction, and picking either one silently is what -ships as "the action does the wrong thing". - -**Approach.** The user has already named it: stop toggling at thread scope. - -- Split `toggle_unread_thread` into two explicit actions, **Mark thread read** - and **Mark thread unread**, each with a fixed direction. Both appear in the - "Whole thread" submenu, where an entry always carries text, so a fixed label - is honest in a way a toggle's cannot be. -- The message-scoped `toggle_unread` stays a toggle. One message has a real - two-valued state, so the trap does not exist there. Do not "unify" the two: - the asymmetry is the point. - -**Constraints.** - -- **Adding an action is four places**, all enforced by tests that fail - confusingly: `KeyMap::knownActions()`, `defaultBindings()`, the icon table, - and the no-duplicate-icons exception list. See `CLAUDE.md`. Splitting one - action into two means one new entry in each, and the pair shares the twin's - icon under the existing named exemption for thread actions. -- **`Ctrl+Alt+U` is taken by the action being split**, and the whole-thread - bindings are already one modifier out from their twins because `Ctrl+Shift+U` - was claimed. Two directions need two sequences; if a second chord cannot be - found that is not worse than the menu, bind one and leave the other to the - submenu rather than inventing a three-modifier chord nobody will press. -- **This interacts with items 98 and 99**, which is the reason to decide all - three together. 99 asks for a dynamic label on the message-scoped toggle, - which is the opposite move: keep the toggle, make the label tell the truth. - A thread cannot do that, because on a mixed thread there is no true label to - show. Deciding 99 first will produce the wrong answer here by analogy. -- The undo entry must name the direction that ran (`Mark thread unread`), not - the action. `tagSelected` already takes the text, so this comes free from - splitting. -- **The test needs a MIXED thread**, which is the whole defect: a thread whose - messages are all in one state answers identically whichever way the direction - is computed, so a fixture built from a uniformly-unread thread passes against - the bug. Same trap as item 88's opposite-states requirement, recorded in - `CLAUDE.md`. - -**Size: S.** The write path is already correct and thread-scoped; the work is -the action split, the four registration sites, the binding decision, and a test -over a mixed thread. - - ## 113. No way to see a message's HTML source **Observed (user, 2026-08-17):** reviewing item 100's removals, "view source @@ -804,94 +643,6 @@ make Save image work must not make Save link reachable again. The test fails if it does, which is the point: the handler is per-profile, so the natural implementation would light up both entries at once. -## 118. No way to empty the trash from inside the app - -**Observed (user, 2026-08-17):** raised while reviewing item 103's spec, as -something that had been forgotten rather than newly noticed: "we could add -'Empty Trash' to the backlog as a future item. I forgot it existed, but I don't -want to squeeze it in this spec." - -**Blocked on 103**, which creates the trash folder this would empty. Until that -ships there is nothing to empty: Delete writes a tag and moves no file, so no -account has a populated trash folder except through another client. - -**Deliberately excluded from 103's spec**, at the user's request and recorded in -its "Out of scope" section. Worth keeping separate for a reason beyond scope -control: emptying the trash is the first action in this application that would -destroy mail with no undo. Every mutation so far is a tag or, after 103, a move, -and both are reversible. A purge is not. - -**Approach, unspecified.** The shape depends on decisions not yet made, and the -spec for 103 answers none of them: - -- **Local or remote.** Deleting the files locally and letting `Expunge Both` - carry it to the server is one thing; asking the provider to empty its own - trash is another, and mbsync offers no verb for the latter. The first is - probably what "Empty Trash" should mean here. -- **Whether the no-confirmation rule survives it.** It does not, on the face of - it. `CLAUDE.md` grants undo in place of confirmation dialogs, and this is the - action where undo cannot exist. That makes it the second item, after 103, that - re-examines the rule rather than assuming it, and unlike 103 it will probably - have to break it. -- **Per-account or all-accounts**, which should follow whatever the Trash filter - does once 103 ships rather than being decided independently. - -**Size: S**, provisionally, and not worth sizing properly until 103 exists. - -## 119. The unsynced-changes count cannot be opened to see what it counts - -**Observed (user, from the notes):** "the bottom left statusbar message needs to -be clickable and show what 'N unsynced changes' are in a modal window". - -**Cause (verified in the code).** `m_pendingLabel` is a plain `QLabel` added to -the status bar with `addPermanentWidget` (`src/mainwindow.cpp:502-505`). A -`QLabel` has no clicked signal and none is installed, so there is nothing to -click and no route to a list. It carries a tooltip and nothing else. - -**The count is a SUM OVER FOUR SOURCES, and that is what makes this bigger than -it looks.** `pendingEditCount()` returns -`m_pendingTagEdits.size() + m_unnettablePendingEdits + held + heldMoves`. -Three of those can name what they hold: `m_pendingTagEdits` is a -`QHash<QString, bool>` keyed by message id, `m_heldEdits` and `m_heldMoves` are -queues of edits waiting for a sync to end. **`m_unnettablePendingEdits` is a -bare `int`** (`src/mainwindow.h:1248`), deliberately so: it counts confirmed -changes that carry no message ids and therefore cannot be netted against -anything. - -So a dialog built from what is currently kept would list three of the four -groups and then have to account for a remainder it cannot describe. Showing "and -3 more" is worse than the tooltip, because the user opened the window -specifically to find out what those were. - -**Approach.** Two halves, and the second is the real work. - -- The clickable half is small: a label that emits on click (an event filter, or - a flat `QToolButton` styled as a label), plus a dialog listing what the three - describable groups hold. The message pane already resolves an id to a subject. -- The complete half needs `m_unnettablePendingEdits` to become something that - can name its entries. Its comment says why it is an int: understating the - indicator is the direction that costs the user work, so it counts what it - cannot identify rather than dropping it. Making it describable means finding - out what those changes actually are and whether they can carry an id. - -**Constraints.** - -- **The count is deliberately conservative and must stay so.** Item 28 and item - 54 both landed on this indicator being wrong in the direction that made the - user think their work was safe. A dialog that lists fewer changes than the - count claims is the same failure in a new place: reconcile the two, or state - the remainder honestly rather than hiding it. -- **An external `notmuch` run can clear pending changes without this count - noticing**, which the tooltip already admits. A dialog makes that staleness - much more visible, since a listed change may no longer exist. Worth deciding - whether the dialog re-verifies against the database before showing. -- Read-only. This is an information window, not a place to retry or discard a - change; either would be a new mutation path with its own undo question. - -**Size: S** for the clickable half over the three describable groups. **Unknown** -for the fourth, and the item is not complete without it. - - ## 121. The thread list shows nothing while a query is running **Observed (user, from the notes):** "can we show a spinner in the left panel @@ -1342,109 +1093,6 @@ The 70-second duration recorded above fits a `QTRY_*` waiting for a file that is never going to appear, which is consistent with a wrong destination rather than a slow one. -## 164. A draft this application saved keeps `inbox` - -**Observed (developer, 2026-08-25):** `notmuch search --output=tags` on a -draft this application had just written reported `draft inbox unread`. - -**The first cause recorded here was WRONG, and the correction is the useful -part.** It said `strip_inbox_from_sent()` reads a sent-only folder list and -that `qtmaildirconf.py` has no drafts equivalent. Neither is true: - -- `NOT_ARRIVALS` is `("sent", "drafts")`, so `sent_folders()` already returns - both. The name says "sent" and the contents do not, which is what made the - wrong reading plausible. -- Run against the real config it returns every account's drafts folder. -- `notmuch count "(<carve-out query>) and id:<the draft>"` returns **1**. The - query the hook builds MATCHES the affected message. - -So the folder list and the query are correct, and the fix is not there. - -**What is actually established.** - -- The carve-out is scoped to `SCOPE = "tag:new"` (`post-new:106`). -- The affected draft carries `inbox`, and `notmuch count tag:new` is **0**. -- The installed hooks are SYMLINKS into this repository, so the code read is - the code that runs. Verified rather than assumed. -- An mbsync-style rename does **not** re-apply `new.tags`: measured in a - throwaway database, a file renamed to add `,U=4` and reindexed kept the tags - it had. The "the rename retags it" theory is therefore also out. - -**What is NOT established, and must be before any code is written:** which -pass put `inbox` on this file, and why it was not carrying `tag:new` when the -hook's carve-out ran. The likely shape is an ordering one, since item 158 -indexes a draft from the application itself, outside `notmuch new`, and a file -already known to the database is not a new file on the next pass. But that is -a hypothesis and the last two hypotheses here were both wrong. - -**The reproducer was built (2026-08-25) and it settles the mechanism.** Seven -variants were driven in throwaway databases, modelling `indexDraftFile()` with -a real `notmuch_database_index_file` call rather than the CLI, because no CLI -command indexes an untracked path without applying `new.tags`. - -What the sweep established, each measured rather than reasoned: - -- `index_file` applies **no tags at all**. A draft the application indexes is - therefore never in `tag:new` scope, and the hook has nothing to carve out. -- Whenever the file IS in `tag:new` scope, the carve-out strips `inbox` - correctly, in every filename shape tried: `:2,DS`, `:2,D`, no info suffix, - in `cur/` and in `new/`, with and without the `,U=4` infix. The real file's - shape (`,U=4:2,D`) is among them. -- It survives the orderings too: `notmuch new` first then the app's index, - the app's index first then the rename, an autosave landing between - `notmuch new` and the hook, and the stale-path `remove_message` that makes - the renamed file arrive as new mail. All six left the draft clean. -- The `D` flag is what puts `draft` on the message (`synchronize_flags`), and - the `S` flag is what removes `unread`. The affected file is `:2,D`, which is - why it carries `unread`, and that matches the reported tag set exactly. - -**The one variant that reproduces it** is the general shape rather than a -filename detail: a pass where `inbox` is applied while `tag:new` has ALREADY -been consumed. Modelled as a file indexed at a path the carve-out does not -cover and moved into the drafts folder afterwards, it ends in precisely the -live end state, `draft inbox unread` in Drafts with `,U=4` and `tag:new` at 0. -Nothing revisits a message once the marker is gone, so the tag is permanent. - -**What is still NOT established, and the next step.** The affected account -writes drafts straight to `<account>/Drafts`, which the carve-out -covers (verified against the live config and the live query, which matches the -message by id today), so the reproducing variant's premise does not hold for -it as written. The live log for the pass that added it reads - - 10:10:52 Added 1 new message to the database. Detected 9 file renames. - 10:10:52 post-new: sent-folder carve-out applied over 9 folder(s) - -so the hook DID run on that pass, over a path the query covers, and logged -success. The remaining candidates are all about what the path or the marker -looked like at that instant, not about the query text: the carve-out logs -"applied" on a `notmuch tag` that matched zero messages, so a successful log -line is not evidence the message was in scope. Instrumenting the hook to log -the carve-out's MATCH COUNT, and leaving it to run until the next draft, is -the cheapest way to close it, and is a log-only change to code that tags real -mail unattended. - -The filename also rules one thing in: `1787645266.M802P16149Q3.<host>` is -exactly `MaildirName::fresh()` output, so the application wrote this file. It -is not a draft another client left behind. - -The reproducer scripts are throwaway and were not kept; `indexfile.c` is -fifteen lines around one `notmuch_database_index_file` call and is trivial to -rebuild from this entry if the instrumentation points back at the hook. - -**Constraints.** - -- **The hook tags real mail unattended every ten minutes.** Nothing here is - worth a speculative change. -- The 0.27.0 changelog claims sent mail and drafts both stay out of the inbox. - Whatever the cause, that claim is currently false for drafts and the entry - needs correcting with the fix. -- Only `inbox` may be touched. A draft legitimately carries `draft` and - `unread`, and `maildir.synchronize_flags` means removing `unread` rewrites - the filename and reaches the server. -- The hook must keep refusing to consume `tag:new` when a carve-out fails. -- `test_post_new.py` and `test_qtmaildirconf.py` both live beside the hook and - have sent-carve-out tests to copy. - ## 165. A draft gets a new Message-ID on every autosave **Observed (developer, 2026-08-25), while hand-testing items 163 and 164.** @@ -1512,3 +1160,105 @@ id and a draft of a reply carries both. - Item 163's fix stands on its own and this does not block it: the file is replaced correctly now, so the fork this would have mitigated no longer happens by that route. + +## 170. A row that stops matching the view only leaves it on the Delete path + +**Observed (user, from the notes):** "should we refactor the list UI to be +responsive so changes are applied immediately instead of waiting for a view +change to repaint?" + +**Cause (verified in the code, 2026-08-26).** Two different properties were +being called "responsive", and only one of them was built. + +The optimistic **repaint** is universal. `ThreadListModel::applyTagChange()` +covers a thread-scoped write, `applyMessageTagChange()` a message-scoped one +(items 105 to 111), and `revertPendingTagChange()` undoes either if the write +is rejected. A chip, a bold row and a dimmed row all move the moment the user +acts. + +The optimistic **membership** is not. `ThreadListModel::removeThreadsWithoutTag()` +has exactly ONE caller, in `trashMessages()`, added last session because Delete +strips `inbox` and a deleted message sat in the Inbox view across restarts. The +ordinary tag path never calls it: neither `sendMessageTagChange()` nor +`sendThreadTagChange()` asks whether the row still belongs in the view. + +So in the Unread view, marking a message read repaints the row and leaves it in +a list defined by `tag:unread`, which it no longer matches. Un-flagging in the +Flagged view is the same, and so is removing `inbox` by hand from the Inbox +view. It corrects itself at the next query or sync, which is exactly the "waits +for a view change" the note describes. + +**Approach.** Not a refactor. `viewFilterTag()` already resolves the view's own +tag from the query, and `removeThreadsWithoutTag()` already does the removal. +The gap is that the guard sits in `trashMessages()` rather than at the funnel +every tag write passes. Move it, or call it from both send paths. + +**Constraints.** + +- The guard's existing reasoning is what makes this safe and must be kept: only + a plain `tag:<x>` view has a membership one tag decides. A path query (Trash, + Sent, Drafts) is unaffected by a tag going away, and a hand-typed query cannot + be reasoned about. Both are left alone. Without that, marking read in an `id:` + view would empty the list. +- A row leaving is not revertible by `revertPendingTagChange()`, which repaints + rather than reinserts. A REJECTED write would leave the row gone until the + next query. The move path already carries that exposure; check whether it is + acceptable at the tag path's much higher frequency, or make the removal wait + for confirmation there. +- The inverse case is deliberately out of scope: a row that starts matching + cannot be inserted optimistically, since the model has no summary for a + thread the query never returned. +- Undo goes back through the same funnel, so a removal must not make an undone + mark-read invisible in the view it was undone in. + +## 173. The composer is a plain-text editor, not WYSIWYG + +**Observed (user, 2026-08-27):** asked for directly while hand-testing item +171. "This is a GUI mail client and should have a wysiwyg editor." + +**Where it came from.** Item 171 forwards an HTML message by carrying the +original's markup, and the markup cannot be shown in a `QPlainTextEdit`. The +first build seeded a text quote into the editable buffer and then dropped it +when building the HTML part, so the user could edit a quote whose edits were +silently discarded. The user's objection is the right one and is more general +than that bug: **what the composer shows should be what gets sent.** + +171 shipped the middle ground, a read-only preview beside the editor. This +item is the real answer. + +**Cause (verified in the code).** The composer is a `QPlainTextEdit` over +markdown, deliberately: `OutgoingMessage::markdownBody` is "the source text, +exactly as typed", `MarkdownRenderer` turns it into HTML at build time, and +`DraftStore` autosaves that same markdown. There is nowhere in that model for +a stranger's markup, or for the user's own rich text, to live and be edited. + +**Approach.** A rich-text editor for the body, which is what every graphical +mail client does. Thunderbird is the reference: forwarding HTML opens a +rich-text composer with the original inside it, editable. + +**This is not a widget swap, and the constraints are why it is L.** + +- **The draft format changes.** A draft currently round-trips as markdown; a + rich-text composer means storing HTML, and a draft written by one and read + by the other loses formatting silently. Items 163 and 165 are already about + draft identity and are worth settling first. +- **`QTextEdit`'s HTML subset is narrow.** It is not a browser: real + newsletter markup (tables, modern CSS) degrades in it. So a naive swap makes + the FIDELITY of a forward worse than what 171 currently sends, while making + the editing better. Measure before committing to it. +- **The formatting toolbar has two masters.** `MarkdownFormat` answers "what + does this button do to a selection" over markdown text. A rich-text editor + has its own notion, and the toolbar must drive whichever is active without + the two disagreeing. +- **Plain text must stay reachable.** Not every message should be HTML, and + the `send_html` config default plus the per-message toggle both already say + so. A rich-text composer that cannot produce clean plain text regresses the + common case. +- **The security work does not go away.** A forwarded original is still input + from a stranger. Whatever renders it for editing must not fetch remote + content, and `HtmlSanitiser` (item 171) is what already answers that. + +**It subsumes item 133**, markdown syntax highlighting: that is a +`QSyntaxHighlighter` over the plain editor, which is the cheap answer to the +same want ("show me what I am writing"). If this is built, 133 is moot; if +this is deferred, 133 is the thing to do instead. Do not build both. |