diff options
Diffstat (limited to 'docs/superpowers/plans')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 48 |
1 files changed, 47 insertions, 1 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index f0e4ecd..48d4516 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 @@ -53,7 +53,7 @@ taking that too literally. | 11 | Icon, `.desktop` file, SlackBuild | packaging | M | **partly done**: icon and `.desktop` landed, SlackBuild open | | 13 | No visual feedback that an action stuck | feedback | S | **done** | | 14 | Tag column unreadable, tags need another home | presentation | M | **done** | -| 15 | Attachments are parsed but unreachable from the UI | information | M | open | +| 15 | Attachments are parsed but unreachable from the UI | information | M | **done** | | 16 | Delete on an already-deleted thread should undelete | behavior | S | open | | 17 | No completion for tags in the query bar | workflow | M | open | @@ -557,6 +557,52 @@ This is a gap in the UI only. The backend is complete and already hardened: file. A message with an attachment named `../../etc/passwd` writes inside the chosen directory under a sanitised name and nowhere else. +### Outcome (done) + +Both halves built. The paperclip column needed no new worker query, as the +plan expected: notmuch applies the `attachment` tag itself, so +`ThreadSummary::hasAttachment()` reads what is already there. + +**The bar holds ONE button, not one per file.** The plan's "one button showing +the filename and size" was built first and was wrong: a thread with sixteen +attachments made the bar as wide as the window, pushed the splitter over, and +left the thread list a few pixels wide. It is now `Attachments (N)...` opening +a dialog that lists message number, filename and size with a `Save...` each, +plus `Save all...` when there is more than one. No filename reaches the bar, +so no filename length can resize anything. + +**`Save all` writes into a new subdirectory** named `<date> <subject>`, inside +a parent the user picks. Chosen over zipping: Qt ships no zip API, so a real +`.zip` meant either a new build dependency (quazip, libzip) or shelling to +`/usr/bin/zip` at runtime, and the actual requirement was "do not drop sixteen +files loose among hundreds of others". The picker's title names the subfolder +before the user commits to a location. + +**Two defects found only by using it, both silent:** + +- **`saveTo()` overwrites, which destroyed six of sixteen files.** Several + messages in one thread commonly attach the same filename; each write landed + on the previous one and every one reported success, so the status line said + 16 while the directory held 10. `saveWithoutOverwriting()` now backs the + batch path, appending " (2)" before the extension and keeping a compound + extension whole. `saveTo()` still overwrites, which is right for a single + save the user just chose a location for. +- **`Qt::RFC2822Date` rejects a date carrying a timezone comment.** A header + ending `+0200 (CEST)` is legal per RFC 5322 and common in real mail, and Qt + refuses the whole string rather than the comment, so every such message lost + its date prefix. Comments are stripped before parsing. + +**The subject is untrusted and becomes a directory name.** +`attachmentFolderName()` lives beside the other guards in `mimeparser.cpp`, +strips separators, control characters and leading dots, caps length at 120, +and falls back to a generated name. Its test drives `../../etc`, +`/etc/passwd`, `..`, `.hidden`, a backslash and a null byte, then asserts each +result still resolves inside the parent through `isPathInsideDirectory()`. + +**Deferred, as the plan required:** opening an attachment in its default +application. That hands a stranger's file to `xdg-open` and is a separate +decision from writing it to a directory the user chose. + ## 16. Delete on an already-deleted thread should undelete **Observed:** hitting Delete twice on the same message is a natural way to |
