diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-27 13:05:17 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-27 13:05:17 +0200 |
| commit | 5570d0e7495a42a90acf951d396c9185b0319eb9 (patch) | |
| tree | 4052c9060b6ade33577c9bcb285565a1477bd3eb /docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | |
| parent | 12e841b8e2c4c225ea79de87dc7bb50f0404ee69 (diff) | |
| download | qtmaildir-5570d0e7495a42a90acf951d396c9185b0319eb9.tar.gz qtmaildir-5570d0e7495a42a90acf951d396c9185b0319eb9.zip | |
Item 171. A forward carried only the plain-text version of the original,
so formatting was lost; and an original with no plain-text part at all
(30 of 342 sampled inbox messages, ~9%) forwarded as an empty quote with
its content silently gone.
A forward now sends ONE part chosen by the Send-as-HTML toggle: the
original's markup when on, the text quote when off. Not a
multipart/alternative, at the user's decision: a forward's shape is
already decided by that toggle, and sending both hands the choice to the
recipient's client. The toggle is honoured even for an HTML-only
original, which then forwards as a text fallback.
HtmlSanitiser strips remote content from the forwarded markup, checked
by default with a per-forward opt-out. This is the security-critical
part: the markup leaves this process and is rendered by the recipient's
client, where none of MessageView's protections apply, so forwarding a
tracking pixel forwards the tracking. It is an ALLOW-LIST, unlike
HtmlBuilder::namespaceCids(), because a missed rewrite is a broken image
while a missed strip is a beacon reaching the recipient.
An HTML forward does not seed a text quote into the editor. The first
build did, then subtracted it when building the HTML part, so the user
could edit a quote whose edits were discarded; what the composer shows
must be what gets sent. The forwarded message appears in a read-only
pane beside the editor instead, a QSplitter at 60/40 with a toggle in
the Format menu. A plain forward is unchanged.
ComposeContextBuilder::quoteBody() renders htmlBody down to text when
there is no plain part, so the plain path never emits an empty quote.
Design in docs/superpowers/specs/2026-08-27-forward-html-design.md.
Two tests repaired for the splitter: the 60/40 assertion reads stretch
factors rather than pixels, since the offscreen platform gives the
splitter no width and reports 49/49 whatever the code asks; and
theComposerSplitsItsToolbarByScope looked for the body directly in the
composer's column.
Not yet hand-tested in this arrangement.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AtUzfNjMD8fiYfamDd3ywW
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 | 114 |
1 files changed, 54 insertions, 60 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 fddb746..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 @@ -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 | @@ -244,8 +244,9 @@ taking that too literally. | 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 | S | open, 2026-08-27, from the notes. `ComposeContextBuilder::quoteBody()` reads `ParsedMessage::plainBody` only, so the original's `htmlBody` is dropped whatever the composer's own Send-as-HTML state is. Needs a DECISION on what a forward carries before any code, see the entry | +| 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. @@ -1210,61 +1211,54 @@ every tag write passes. Move it, or call it from both send paths. - 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. -## 171. A forwarded HTML message reaches the recipient as plain text - -**Observed (user, from the notes):** "forwarding an html message doesn't -maintain the html formatting of the original message. #bug" - -**Cause (verified in the code, 2026-08-27).** The forward path builds its body -through `ComposeContextBuilder::quoteBody()` (`src/composecontext.cpp`), which -reads `message.plainBody` and nothing else. `MimeParser` parses both halves and -`ParsedMessage` carries `htmlBody` beside `plainBody` (`src/mimeparser.h:150`), -so the HTML is available and simply never asked for. - -Two consequences follow, and they are not the same severity: - -- An original with both parts forwards its text/plain alternative, losing the - sender's formatting. Recoverable-looking, since the words survive. -- An original with an HTML part ONLY has an empty `plainBody`, so the forward - carries the attribution line and an empty quote. The message's content is - gone, and nothing says so. - -`MainWindow::composeReply()` already treats the two kinds differently for the -composer's own HTML state: `context.seedHtml` is the CONFIG's `sendHtml` for a -forward and `original.hasHtml()` for a reply, on the stated reasoning that an -HTML part is a fact about the sender's software. That reasoning is sound for -how the user WRITES and does not decide what the forward CARRIES, which is the -question here. - -**Approach.** The decision comes first; this is not a changed call site. - -A forward is a different act from a reply: the point is to hand somebody else -what arrived, and quoting is the wrong shape for it. Three candidates, in -increasing fidelity: - -- Render `htmlBody` down to text when `plainBody` is empty, so nothing is - silently lost. The smallest fix, and it does not answer the note: formatting - is still gone. -- Carry the original as a `message/rfc822` part, which is what item 130 already - describes and what GMime builds natively. Perfect fidelity, and every - attachment comes with it, but the recipient sees an attached message rather - than a body. -- Build the forward as `multipart/alternative` with the original's HTML nested - in the HTML half, which is what Thunderbird's inline forward does. - -**Constraints.** - -- **The HTML is input from a stranger and the composer is not the message - pane.** The pane's protections (off-the-record profile, JavaScript off, the - interceptor blocking every request) are `MessageView`'s, not - `ComposeWindow`'s. Any route that puts the original's markup into an outgoing - message must decide what it strips, and remote references in particular: - forwarding a tracking pixel forwards the tracking to the new recipient. -- Item 130 overlaps and may subsume this. Decide the two together rather than - building `message/rfc822` twice. -- The markdown body is the composer's source of truth, and markdown has no - syntax for arbitrary HTML the user can then edit. A route that keeps the - original's markup has to keep it OUTSIDE the editable buffer, which is the - same nesting problem item 129 carries. -- `quoteBody()` is shared with Reply. A change there reaches both; the - behaviour asked for is the forward's alone. +## 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. |
