diff options
Diffstat (limited to 'docs/superpowers/specs')
| -rw-r--r-- | docs/superpowers/specs/2026-08-26-card-avatars-design.md | 310 | ||||
| -rw-r--r-- | docs/superpowers/specs/2026-08-27-forward-html-design.md | 181 |
2 files changed, 491 insertions, 0 deletions
diff --git a/docs/superpowers/specs/2026-08-26-card-avatars-design.md b/docs/superpowers/specs/2026-08-26-card-avatars-design.md new file mode 100644 index 0000000..e396a34 --- /dev/null +++ b/docs/superpowers/specs/2026-08-26-card-avatars-design.md @@ -0,0 +1,310 @@ +# Card avatars and the account fade + +Design for backlog item 169. Brainstormed with the user on 2026-08-26. + +**Status:** specified, unbuilt. + +## The problem + +The user's note asks for two things about a card: + +> the left border of a card expresses the account the mail belongs to. the +> background color of the card should fade left to right from the account color +> to the current background color we are using (or to transparent to work both +> in light and dark themes). On the left we should leave room for an account +> avatar (a squircle), for now it could be extracted from the sender name "From: +> john doe" becomes "JD" in the avatar. As soon as we include khard (or some +> other vcard provider/manager) we will switch to images if the corresponding +> vCard has one. + +Half of it already shipped. The account colour is drawn as a solid bar down the +left edge (`CardLayout::accentRect`, filled in `CardDelegate::paint()` through +`CardDelegate::accentLineColour()`), and a reply's spines carry the same colour +muted against the pane's base. What does not exist is any gradient, and any +avatar: `CardLayout` reserves no rect for one, so the geometry has to grow +before the painting can. + +The note calls it an "account avatar", but the brainstorm settled that it is +the SENDER's, not the account's. The account is already expressed twice, by the +accent bar and by the fade; an avatar in the account's colour would conflate +"which mailbox received this" with "who wrote it", which are different facts. + +## What the card looks like + +A card stays exactly three lines and every row keeps the same height, so +`setUniformRowHeights(true)` survives untouched. + +``` ++--+-------+------------------------------------------------+ +| | | sender 26/08 09:14 | +|##| [AV] | * subject @ v 3 replies | +| | | [tag] [tag] | ++--+-------+------------------------------------------------+ + ^ ^ + | +-- avatarRect, full height, its own gutter + +-------- accentRect, unchanged +``` + +The avatar is a full-height squircle in its own gutter, on roots and replies +alike. `contentLeft` shifts right by the gutter, on top of the existing indent. + +Three alternatives were shown to the user and rejected: a two-line squircle +with the tag strip running full width beneath it, and a small squircle inline +on the sender line costing no width. The full-height form won on presence, with +the cost accepted explicitly: at `kMaxDepth` the subject loses the gutter's +width on top of the indent it already loses. + +## The fade + +A horizontal gradient of the account colour, drawn after the chrome and before +the text, so the selection highlight and the deleted-row tint still cover it. +That a selected row reads mostly as selection is expected, not a fault. + +- **On a thread root** it starts at the card's left edge. +- **On a reply** it starts at that reply's INNERMOST spine, which is its own + coloured left border. The wash therefore steps right with the nesting, and a + deeper reply's outer spines stand in plain background. +- **It ends at 60% of the card's width**, in both cases. The end is + proportional, so a reply's wash is shorter as well as further right. +- **A reply's gradient is weaker than a root's**, so an expanded thread reads as + one coloured block with the root leading it. + +The user chose the reply origin against the alternative of starting every fade +at the card's left edge. The spine reading is the correct one because the +spines are ALREADY the account colour: `carddelegate.cpp` muted them at 0.55 +against `QPalette::Base` and resolves a reply's colour by walking to its root, +so the coloured border the fade hangs off is a thing that exists rather than +one this item introduces. + +60% was chosen by the user over 50%. It is a percentage of card WIDTH, which +means the wash grows with the window; a fixed pixel distance and an +anchor-to-the-layout variant were both offered and declined. + +## Whose face + +The **sender's**, except in the flat views (Sent and Drafts) where it is the +**recipient's**, since those views already show recipients in the sender's slot +and `authors` is the user on every row there. + +This costs no extra query. `MainWindow::m_sentView` is really "flat view": it is +assigned from `FlatResult`, and `generatorIsFlat()` in `config.cpp` is the closed +set `{sent, drafts}`, so both already request the recipients fold. The name is +misleading and deserves a comment, but not a rename inside this item. + +For a thread row the sender is the one belonging to `firstMessageId`, the +message the card already stands for. Nothing new is resolved. + +## The initials + +Two letters, always, so every squircle reads the same shape: + +| Input | Rule | Example | +|---|---|---| +| Display name, two or more words | first letter of the first two words | `John Doe` -> `JD` | +| Display name, one word | first two letters of that word | `Cofidis` -> `CO` | +| No display name | first of the local part, first of the domain | `noreply@cofidis.it` -> `NC` | +| Nothing usable at all | first two of the account's label | see the fallback below | + +The user chose the local-plus-domain form over first-two-of-local (`NO`) and +over a single letter (`N`), because it never degrades to one letter and never +reads as a truncated word. + +## The fill + +Two fills, both generated locally from a hash of the sender's address, both +stable per sender. + +- **Identicon** - a 5x5 symmetric grid from the hash bits, with a translucent + dark veil between the pattern and the letters so the initials stay readable + whatever the pattern does. +- **Two-tone** - two related hues and a split angle from the hash, initials on a + large flat field. + +Which one, in order: + +1. The address is listed in `business-senders` -> **two-tone**, whatever display + name it presents. +2. Otherwise a display name is present -> **identicon**. +3. Otherwise -> **two-tone**. + +Rule 2 is the one the user asked for by name: `Ian Farrell +<notifications@github.com>` gets `IF` and the identicon, because it presented +itself as a person, even though the address is corporate. Rule 1 is the override +that lets a listed sender be forced back to two-tone. + +Colours are generated at a FIXED lightness, so `TagColors::textColourOn()`-style +contrast reasoning holds and the initials stay legible in both themes. + +**None of this is Gravatar in the sense of contacting Gravatar.** A real lookup +would send a hash of every correspondent's address to a third party on each +repaint, which is out on the project's privacy stance and on the no-network +rule. Only the generated half of the idea is taken, and it works entirely +offline with `QCryptographicHash`, which Qt Core already provides. + +Item 72's vCard photo would later replace the fill without touching the rect. + +## The fallback + +There is always an avatar. When no sender address is available at all, the +squircle is hashed from the **account's own address**, which the card always +knows through its account, and the initials are the first two letters of the +account's label. This keeps a stable, themed squircle rather than a hole, and +it cannot be confused with a real sender because the two-letter source is the +account. + +## `ThreadSummary::firstMessageSender` + +**This is the one structural change, and it exists because the card currently +has no address to hash.** + +`ThreadSummary::authors` is notmuch's own summarised string and carries display +names ONLY. Measured against the real index on 2026-08-26: `'Standreas'`, +`'Randstad Italia'`, `'Ryanair'`, `'The Hacker News tramite LinkedIn'`. No `@` +anywhere. The initials rule survives that, but two things do not: + +- the identicon has nothing stable to hash, and +- the `business-senders` list has nothing to match. + +Hashing the display name instead was considered and rejected: notmuch builds +those strings, so one sender varies its identity as the string varies +(`The Hacker News tramite LinkedIn` is a constructed label, not a header value). + +So `ThreadSummary` gains `firstMessageSender`, the bare address of the message +the card stands for. It is filled by the SAME worker walk that already fills +`firstMessageId` and `firstMessageTags`, from the same message, and `From` is +served from notmuch's index rather than from the file. This is exactly the +pattern item 111 used for `firstMessageTags` and it carries the same "free, for +the same reason" note. + +The Sent/normal split is already resolved in that walk and must be respected: +`withRecipients` selects the first MATCHED message for a flat view and the +thread's opening message otherwise, so the address follows whichever message +the card is standing for. + +**Measured cost of reading senders from the index**, on the developer's own +database: 1322 distinct senders deduplicated in 12 ms, and 5105 messages +enumerated in 76 ms. There is no cost problem here, which is what makes the +whole feature and its list practical. + +## The `business-senders` list + +`~/.config/qtmaildir/business-senders`. Plain text, one entry per line, `#` +comments, blank lines ignored. + +Deliberately NOT in `qtmaildir.conf` and deliberately not INI. The user's stated +workflow is grep-and-edit ("if a personal email ends in the list by mistake I +can grep it out and remove it"), QSettings would fight a bare list, and the main +config is already large. + +An entry is either: + +- an exact address, `noreply@cofidis.it`, or +- a whole domain, `@cofidis.it`, which is what a company sending from six + addresses actually needs. + +**No globs.** A pattern language nobody asked for is a rule the user cannot grep +for literally, which defeats the file's whole purpose. + +Read once at startup and on an explicit reload. Never per repaint, and never +stat-per-row. + +## How the list fills itself + +At the end of every sync the application runs, the worker collects the senders +of the newly arrived mail and appends CANDIDATES, commented out: + +``` +# noreply@cofidis.it (47 messages) +``` + +**How much mail the scan covers** depends on whether the list has ever been +used. With no file, or a file holding no active entry, it scans the WHOLE +database; afterwards it scans the last week. The first run is exactly when a +full scan earns its cost: a week of mail proposes almost nothing, so a +week-only rule would leave the list taking months to become useful. It is +affordable because it happens once, measured at 76 ms over 5105 messages. + +A file holding only rejected candidates still counts as unused. Rescanning it +re-proposes none of them, since anything already mentioned is skipped. + +A candidate is an address whose local part is in a small built-in word list +(`noreply`, `no-reply`, `donotreply`, `info`, `support`, `billing`, +`newsletter`, `notifications`, `mailer-daemon`), or one that recurs with no +display name. + +Two rules make this safe, and both are borrowed from `mailrules.py`'s existing +discipline: + +- **Never writes an uncommented entry.** Nothing on screen changes until the + user uncomments a line. A step that silently reclassified forty senders would + have to be audited line by line anyway. +- **Never removes, and never re-adds.** An address already present in the file + in ANY form, commented or not, is skipped. An entry the user grepped out + therefore stays out, instead of reappearing within ten minutes with no + explanation. + +### Why the application writes it, and not the `post-new` hook + +Putting the scan in `assets/hooks/` was considered at length and rejected. It +is the better placement on paper: `post-new` runs after EVERY `notmuch new` +whatever started it, including the user's cron, whereas `mailsync.sh` is only +the route qtmaildir drives, and the hook is already scoped to `tag:new` so it +would see only new mail. + +It was rejected on weight. The hook is Python and the reader is C++, so it would +recreate the two-implementations-of-one-format situation that `CLAUDE.md` +documents for `rules.json`, with the same requirement that both sides change +together and the same by-test-only agreement. That is a heavy contract for a +cosmetic feature. + +The application writing it has one implementation, already knows when a sync +finished, and the scan is free. The only case lost is mail indexed by cron while +qtmaildir is not running, and those senders are picked up at the next run. + +## What is testable, and what is not + +Per the standing rule (`tests-only-for-measurable-things`), this feature is +mostly judged by looking. Split accordingly: + +**Assert in tests:** + +- `CardLayout` places `avatarRect` and moves `contentLeft`, at several depths + and for a root against a reply. Geometry only, no painter, per the existing + rule that a card layout must be testable without one. +- The initials function, over all four rules in the table above, including the + one-word and no-display-name cases and an address with no `@` at all. +- The fill CHOICE, over the three ordered rules, including a listed address + that presents a display name. +- Hash stability: the same address yields the same fill twice, and two + different addresses differ. +- `business-senders` parsing: exact entries, `@domain` entries, comments, blank + lines, and an entry with leading or trailing space. +- The append step: never writes an uncommented line, and skips an address + already present commented out. This is the data-loss-adjacent property, so it + is the one worth the most care. +- `ThreadSummary::firstMessageSender` is filled by the query walk, for a normal + view and for a flat one, where the two must resolve DIFFERENT messages. + +**Hand to the user to look at:** the fade's weight and its 60% end, the two +fills against real mail, whether the initials are legible over an identicon at +the desktop's own font size, and whether the gutter costs too much subject at +depth. + +A rendering probe must not be used to judge any of that, for the reasons +`CLAUDE.md` records under "Rendering probes lie". + +## Risks + +- **The gutter compounds with the indent.** A deep reply already loses + `kMaxDepth * kIndentStep`; it now loses the avatar gutter as well. If it reads + badly, the cap is the knob to turn, not the avatar. +- **An identicon behind two letters is the classic legibility failure.** The + veil exists for exactly that and its opacity is a value to tune against the + real font, not to fix on the first guess. +- **`firstMessageSender` is a new field on a struct crossing the thread + boundary.** It is a plain `QString` and crosses like the rest, so there is no + ownership question, but the walk that fills it must finish while the + `NmThread` is alive, exactly as the surrounding code already documents. +- **The candidate word list is a guess.** It will miss senders and propose + wrong ones, which is precisely why nothing it writes takes effect until the + user uncomments it. diff --git a/docs/superpowers/specs/2026-08-27-forward-html-design.md b/docs/superpowers/specs/2026-08-27-forward-html-design.md new file mode 100644 index 0000000..a71cc54 --- /dev/null +++ b/docs/superpowers/specs/2026-08-27-forward-html-design.md @@ -0,0 +1,181 @@ +# Forwarding an HTML message with its formatting + +Item 171. Design, 2026-08-27. **Read this before the backlog row**, which +records only the cause. + +## The defect + +`ComposeContextBuilder::quoteBody()` reads `ParsedMessage::plainBody` and +nothing else, so the forward path drops the original's `htmlBody` entirely. +Two consequences, and they are not the same severity: + +- An original with both parts forwards its text/plain alternative. The words + survive, the sender's formatting does not. This is what the user reported. +- **An original with an HTML part ONLY has an empty `plainBody`**, so the + forward carries an attribution line and an empty quote. The content is gone + and nothing says so. + +Measured on the developer's own inbox 2026-08-27: **30 of 342 sampled +messages** (~9%) declare `text/html` with no `text/plain` part. The silent +half is not an edge case. + +## What was decided, and by whom + +The user chose, 2026-08-27, from three routes put to them: + +1. **Inline, carrying the original's markup** — CHOSEN. The forward carries + the original's own HTML rather than a flattened quote. Highest inline + fidelity, and the most work, because it is the only route that puts a + stranger's markup into an outgoing message. + + **Amended 2026-08-27, after the first build**: a forward sends ONE part, + not a `multipart/alternative`. The Send-as-HTML toggle chooses which — the + original's markup when on, the text quote when off. The user's reasoning is + that a forward's shape is something they have already decided by flipping + that toggle, and sending both halves hands the choice to the recipient's + client instead. An ordinary (non-forward) message still sends the + alternative as before; only the forward path is single-part. +2. Attach the original as `message/rfc822` (item 130's mechanism). Rejected + here, though item 130 may still build it for its own sake: the user wants + the content inline, not as an attachment. +3. Text fallback only. Rejected: it fixes the silent-loss half and does not + answer the note at all, since formatting is still lost on every forward. + +**Remote content is STRIPPED BY DEFAULT, with a per-forward opt-out**, also +the user's choice against "always strip" and "keep everything". A control in +the composer, checked by default, reading roughly "Strip remote content from +the forwarded message". + +## Why this is the security-critical item in the backlog + +Every other HTML path in this application renders a stranger's markup *to the +user*, behind protections that live in `MessageView`: an off-the-record +profile, JavaScript disabled, and `RequestInterceptor` blocking every request +by default and failing closed. + +**None of those protections apply here.** The markup leaves this process and +is rendered by somebody else's mail client, under their policy, on their +machine. The interceptor cannot help: it intercepts requests *we* would make. +So the sanitising has to happen to the bytes, before they are handed to +`MessageSender`, and there is no second line of defence behind it. + +The concrete harm, in the user's own words when the decision was put to them: +forwarding a tracking pixel forwards the tracking. The original sender learns +that the forwarded copy was opened, by whom, and how many times, and the +user's recipient never consented to that. + +## The allow-list rule, which is not negotiable + +`HtmlBuilder::namespaceCids()` is the closest prior art and it is a +**block-list**: it names the attributes that can carry a `cid:` and rewrites +those. It documents scoping `srcset=` out, on the reasoning that its quoting +grammar differs and `cid:` in `srcset` is not seen in the wild. + +**That trade is correct for rewriting and WRONG for stripping**, and the +asymmetry is the whole design: + +| | a missed reference means | +|---|---| +| `namespaceCids` (rewrite) | one broken image | +| this sanitiser (strip) | a tracking beacon reaching the recipient | + +So the sanitiser must **allow-list what may remain**, not block-list what must +go. Anything not recognised is removed. A new HTML attribute, a quoting form +not anticipated, a `srcset`, a CSS `image-set()`, an `@import` — each is +handled by the default, which is removal, rather than by having been +enumerated in advance. + +Stated as the invariant to test against: **after sanitising, no attribute +value and no CSS construct in the output may contain a URL whose scheme is +anything other than `cid:`, and no element that fetches may remain without +one.** + +## What is kept and what goes + +Kept: + +- `cid:` references. They travel inside the message, fetch nothing, and are + what makes an inline logo survive. Item 129 will need the same machinery. +- Structural and presentational markup: tables, lists, headings, spans, + `style=""` attributes with their remote constructs removed. + +Removed: + +- Any `src`, `href`, `background`, `poster`, `srcset`, `data-*` or other + attribute value carrying a non-`cid:` URL scheme. `http:`, `https:`, + `//host/path`, `data:` (which can carry markup), `file:` above all. +- `<link>`, `<script>`, `<iframe>`, `<object>`, `<embed>`, `<base>`, and + `<meta http-equiv="refresh">`. +- CSS `url()`, `@import` and `image-set()` naming anything but a `cid:`, + whether in a `style=""` attribute or a `<style>` block. +- Event-handler attributes (`onload`, `onerror`, …). The recipient's client + most likely disables scripting, but that is their policy and not ours to + assume. + +A stripped `<img>` leaves a gap where the image was. That is the correct +outcome and should not be papered over with a placeholder that itself fetches. + +## Shape of the change + +- **`HtmlSanitiser`, a new namespace of free functions over values** + (`src/htmlsanitiser.h/.cpp`), matching `MarkdownRenderer` and + `MessageBuilder`. No widget, so the security property is testable without a + painter or a web engine. This is where the allow-list lives. +- **`OutgoingMessage` gains the forwarded HTML and the strip flag.** It + currently carries `markdownBody` and `sendHtml` only, so there is nowhere to + put a second HTML source; both are new fields rather than a changed call + site. +- **`MessageBuilder::build()` chooses the part.** On a forward with the toggle + ON, the body is one `text/html` part: the user's rendered markdown, a rule, + then the sanitised original. With the toggle OFF it is one `text/plain` part + carrying the markdown and the text quote. No alternative either way. + + **The toggle is honoured even when the original had no plain-text part**, so + an HTML-only original forwarded with the toggle off goes out as the text + fallback and its formatting is lost. Chosen deliberately 2026-08-27 over + forcing HTML for those messages, so that the toggle means what it says; the + first build forced it and that was reversed. +- **`quoteBody()` is not the fix and must not become it.** It is shared with + Reply, and the behaviour asked for is the forward's alone. It keeps + producing the text quote for the plain half. Its one change is the + silent-loss case: when `plainBody` is empty it should render `htmlBody` down + to text rather than emitting an empty quote, so the plain half is never + blank. +- **The composer control** sits with the existing per-message toggles, checked + by default, and only appears on a Forward carrying HTML. + +## Traps recorded in advance + +- **`quoteBody()` is shared with Reply.** A change there reaches both paths. +- **`QString::arg()` does not collapse `%%`.** CLAUDE.md records the 0.11.0 + placeholder losing its mask, its glow and both gradients this way while + still painting a plausible pane. Any generated CSS here is subject to it. +- **A geometry or rendering probe cannot see this.** The property under test + is "no remote URL survives", which is a property of the STRING. Assert on + the generated bytes. CLAUDE.md's "Rendering probes lie" section is the + general warning; here a probe that renders the result and looks at it would + endorse a tracking pixel it cannot see, because a 1x1 transparent image is + invisible by design. +- **Test with real hostile shapes**, not tidy markup: unquoted attribute + values (`<img src=http://x/p>`), mixed quoting, whitespace and newlines + around `=`, uppercase tags and attributes, a `cid:` whose id contains a + URL-looking substring, and a `style` attribute carrying `url(...)` with no + quotes. `namespaceCids()` documents each of these forms as real. +- **`MimeParser` must parse what we build.** A round trip through + `MessageBuilder` and back is the check that the nesting is right, and it is + cheaper than reading the RFC. + +## Order of work + +1. `HtmlSanitiser` with the allow-list, tested against the hostile shapes + above. Nothing else depends on decisions inside it. +2. `quoteBody()`'s empty-plain fallback, which closes the silent-loss half on + its own and is independently useful. +3. `OutgoingMessage` fields and the `MessageBuilder` nesting, with a + parse-back round trip. +4. The composer control, defaulting to strip. +5. Hand test: forward a real HTML message to yourself with the box checked and + unchecked, and confirm what arrives. + +Steps 1 and 2 are separable and each ship a real improvement, so this does not +have to land as one commit. |