diff options
Diffstat (limited to 'docs/superpowers/plans')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 294 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 450 |
2 files changed, 563 insertions, 181 deletions
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 3620684..f46fec5 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 @@ -6318,3 +6318,297 @@ rather than stepping down mid-row. does not carry, and has no second tier to show. **Size: S.** + + +## 78. No way to build a rule from something visible in a message + +**Observed.** The user would like to select an address or another piece of a +message in the main window, right-click, and be offered a rule built from it. + +**Cause.** Not a defect, unbuilt. + +**Narrowed 2026-08-14, and most of the work is already done.** The search half +shipped as item 85, which is the road a rule is made from: search for a value, +save the query, create a rule from the saved query. What remains here is a +SHORTCUT across that road, and both of its halves now exist. + +- The menus are built and every surface already extracts its value as a + finished query (`SearchOffer`, `src/searchterm.h`). A rule entry is another + action beside the two search ones, not new plumbing. +- The seeded-dialog path exists from item 81: + `MainWindow::showTagRulesDialog(const TagRule &seed)`. A rule from a message + becomes a second caller of it, with a different seed, which is what item 81's + spec anticipated when it made the seed a whole `TagRule` rather than a query + string. + +**Approach.** Decide it after using item 85 for a while. Which values are worth +promoting straight to a rule is a usage question, and the earlier answer to it +was wrong (see below), so it is worth having the evidence first. + +**Constraints.** The original approach here said to start from the thread +list's context menu "where the sender is already a value the model holds". +**That is false and item 85 verified it.** `ThreadSummary::authors` comes from +`notmuch_thread_get_authors` and is a DISPLAY SUMMARY, reading `Alice, Bob` or +`Alice| Bob`, so a `from:` built from it matches nothing. A real address comes +from `MessageNode::from` or `ParsedMessage::from`, neither of which the thread +list carries. Any thread-list entry needs an address resolved from a message +first. + +JavaScript is disabled in the profile and must stay disabled. Item 85 reads a +body selection with `QWebEnginePage::selectedText()`, which injects no script; +reuse that rather than adding anything. + +The rules file is shared with mailctl, so a rule created here must go through +`TagRules` and preserve unknown fields; see "Changing the shared rule format" +in CLAUDE.md. + +**Size: S**, down from M now that item 85 has built the menus and item 81 the +seeded dialog. + +**Dropped 2026-08-17** at the user's request, and it never was a defect. The +whole journey already exists: right-click any value to search it (item 85), save +that query (item 23), turn the saved query into a rule (item 81). This item was +only a shortcut across those three steps, and its own Approach had already said +to use 85 for a while first and see which values were worth promoting. That +evidence never appeared, which is itself the answer. Reopen if a value turns up +that is worth one click. + + +## 98. "Important" adds the tag but cannot remove it, unlike every other toggle + +**Observed (user, from the notes):** "the add 'Important' action should be a +toggle (like unread)." + +**Cause (verified in the code).** `src/mainwindow.cpp:863` registers `flag` as a +one-way add: + +```cpp +addAction(QStringLiteral("flag"), tr("&Important"), + tr("Mark the selected threads as important"), [this]() { + tagSelected({ QStringLiteral("flagged") }, {}, tr("Mark important")); +}); +``` + +Adding a tag that is already there is a no-op the user cannot see, so pressing +the key or the button on an already-important thread appears to do nothing at +all. Nothing in the UI removes `flagged` except the general tag dialog. + +**The two neighbouring actions are already toggles**, so this is an +inconsistency rather than a missing feature. `delete` +(`src/mainwindow.cpp:825`) and `toggle_unread` (`:867`) both read the current +state and choose a direction, and `delete`'s comment states the rule this should +follow: one direction for the WHOLE selection, flipping only when every selected +thread is already in the target state, because a single keystroke that leaves a +selection in two states is worse than either outcome. + +**Everything needed is already loaded.** `ThreadSummary::isFlagged()` +(`src/types.h:64`) reads the tag off the summary, so the direction can be +decided without a worker round trip, exactly as `isDeleted()` is. + +**Approach, now a two-line change.** Item 105 extracted +`MainWindow::everySelectedRowHasTag()`, which is the whole of the direction +logic: +`everySelectedRowHasTag("flagged") ? tagSelected({}, {"flagged"}, tr("Unmark important")) : the current add`. +The undo stack needs nothing new, since `TagChange::inverted()` already covers +both directions. + +**Constraints.** + +- Call `everySelectedRowHasTag()`, never a hand-rolled loop. Two separate fixes + went into that logic on 2026-08-16 (items 88 and 105) and both were bugs a + copy of the then-current `delete` loop would have inherited: resolving a + reply's row number to the wrong thread, and asking a reply's thread instead + of the reply. +- The action's tooltip says "Mark the selected threads as important" and would + become wrong. Item 99 is the same problem for `toggle_unread` and the two + should be decided together. +- The label question belongs to item 99, not here. This item is the behaviour + only: the key stops being a no-op. + +**Size: XS.** + +**Done 2026-08-17, unreleased.** Two lines, as the entry predicted, calling +`everySelectedRowHasTag()`. The tooltip became "Add or remove the important +tag", matching Delete's wording; the label itself is still item 99's. + +**The test was the work, and its first version was too weak to see the bug it +was written for.** The reply case needs THREE different states, not two: the +first thread in the list unflagged, the reply's own thread flagged, and the +reply itself unflagged. Only then do the two wrong answers (a row number +resolved against the top-level list, and the reply's thread instead of the +reply) differ from the right one. Built the obvious way, with the reply +defaulted to its thread's tags, the mutation putting item 105's bug back stayed +GREEN, measured, for exactly the reason `CLAUDE.md` records about item 88's +opposite-states requirement. The fixture helper defaults `replyTags` to the +thread's, so a test that does not pass them explicitly asserts nothing about +scope. + + +## 100. The message pane offers Back, Forward, Reload and Save page, none of which mean anything + +**Observed (user, from the notes):** "back/forward/save page in the right pane +don't make sense, shouldn't be visible." + +**Cause (verified in the code).** `MessageView::showBodyContextMenu` +(`src/messageview.cpp:619`) starts from Chromium's own menu: + +```cpp +QMenu *menu = m_view->createStandardContextMenu(); +``` + +That menu is built for a browser and carries the navigation and page actions +whole. The pane is not a browser: every document arrives through `setHtml()` +with a fixed base URL, so there is no history to go back to, nothing to reload +from, and the request interceptor blocks everything by default anyway. The +entries are inert as well as meaningless. + +**Deliberate as far as it goes.** The comment above the call says the page's own +menu comes first so "copy, select all and the rest stay exactly as they were", +which is right for the editing actions and wrong for the navigation ones. The +item is that the filter was never applied, not that the base menu was a mistake. + +**Approach.** Keep the menu, drop the actions that cannot apply. Qt names them +as `QWebEnginePage::WebAction` values (`Back`, `Forward`, `Reload`, +`SavePage`, and `ViewSource` is worth the same look), and each has a +`pageAction()` whose pointer can be matched against the standard menu's entries +and removed. Removing by matching the action pointer is safer than matching by +text, which is translated. + +**Constraints.** + +- Do not rebuild the menu from scratch. Copy, Copy link address and Select all + are the reason the standard menu is used, and item 85's search entries are + appended to it. +- `Save page` is not the attachment save. Attachments have their own bar and + their own path-traversal checks (see the web view security notes in + `CLAUDE.md`); nothing here should grow a second way to write a file. +- Verify against a real right-click on a real message. The offscreen platform + builds the menu but a screenshot of it proves nothing, and the entry list + depends on what the page reports as available at that moment. + +**Size: XS.** + +**Done 2026-08-17, unreleased.** `MessageView::removeBrowserActions(QMenu *, +QWebEnginePage *)`, static and taking both, so a test builds a menu and checks +it without a rendered document or a shown popup. `ViewSource` went with the +four the user named, for the same reason. Copy and Select all are asserted to +SURVIVE, since they are why the standard menu is used rather than rebuilt. + +Two things worth keeping. The match is on the `page->action()` POINTER, and the +test asserts through pointers too: matching on text would pass in English and +fail in every other locale, which is a defect no test written in English would +catch. And removing entries strands separators at the edges or doubles them up, +which reads as a menu that lost something, so the filter sweeps them; Qt offers +nothing for this. + +**Still worth a real right-click.** The offscreen platform builds the menu, and +what Chromium offers depends on what is under the cursor at that moment. + + +## 102. The rules table shows no note, so the field explaining a rule is invisible until it is opened + +**Observed (user, from the notes):** "add 'notes' column to the filters table." + +**Cause (verified in the code).** `TagRule` carries a `note` field +(`src/tagrules.h:37`, "Why the rule is shaped this way. Shown in the dialog"), +and the editor below the table edits it, but the table itself lists five columns +and none of them is the note (`src/tagrulesdialog.cpp:116`): + +```cpp +m_list->setHeaderLabels({ tr("On"), tr("Stage"), tr("Rule"), tr("Tags"), + tr("Matches") }); +``` + +So the one field written specifically to explain a rule can only be read one rule +at a time, by selecting it. With several rules the note is exactly the thing that +would let the user pick the right one without opening each. + +**Approach.** A sixth column. The column widths already persist (item 75), so a +new column needs a sensible default width and nothing else in the way of state. + +**Constraints.** + +- The note is free text of any length and would stretch the column. Elide it and + put the full text in the tooltip; the `Rule` column already faces the same + problem with a long query and is the pattern to match. +- `ColumnCount + 1` in `setColumnCount` is load-bearing: the enum drives the + column indices and there is a spare. Add the enum value rather than hardcoding + 5, and check every place that indexes a column by number. +- Notes are the user's own words and can be empty. An empty cell is correct + here; do not substitute a placeholder. + +**Size: XS.** + +**Done 2026-08-17, unreleased.** `ColumnNote` inserted BEFORE `ColumnCount`, +which matters: "Matches" is appended past the end of the enum, so a column added +after it would put the counts under Note. The cell is `simplified()`, because a +note is free text and a newline truncates a tree row at it; the full text is the +cell's tooltip and is untouched in the editor. + +**A second defect fell out of it, and it was not in the plan.** +`QHeaderView::restoreState` REFUSES a state saved with a different column count, +returning false and leaving the header alone, which is what every existing +`uistate.conf` now does. The restore path set `m_columnsSized` and +`m_countColumnSized` regardless, spending the one auto-size each column gets on +a restore that did nothing: the new Note column would have opened at its default +width, once, permanently. Now guarded on the return value. Upgrading costs one +reset of the rule dialog's column widths, which is unavoidable, since the saved +state genuinely describes a table that no longer exists. + + +## 116. Copy image copies markup instead of the image + +**Dropped 2026-08-17, the same day it was raised. There is no defect here**, and +the way this item was reasoned about is the part worth keeping. + +**Observed (user):** Copy image on a displayed remote image appeared to put +`<img src="https://..."/>` on the clipboard, and GIMP reported "no image data in +the clipboard to paste" while pasting fine from other sources. + +**Measured, correctly, on the second attempt.** `wl-paste --list-types` run +IMMEDIATELY after a copy reports: + +``` +application/x-qt-image +text/html +text/x-moz-url +image/png +image/bmp +... 30 more image flavours +``` + +Chromium puts the pixels on the clipboard, in `image/png` and +`application/x-qt-image` among many others. The clipboard is correct and nothing +in this application is involved: `grep -rn "clipboard\|Clipboard" src/` is +empty. + +**Confirmed from the other end too** (user, same day): copying and then pasting +into GIMP IMMEDIATELY works. The original failing paste was against a clipboard +that had already moved on, which is the same staleness that produced the wrong +`wl-paste` reading below. Both halves of the false report had one cause. + +**The process failure is the reason this entry survives its own drop.** An +earlier `wl-paste --list-types` reported text flavours only, and an entire cause +was built on it: "Chromium is not putting pixels on the clipboard, and the paste +target is blameless", with a three-step plan, a Wayland suspect, and a size of +`?`. That reading was taken minutes after the copy, off a clipboard that had +been overwritten in between. The staleness was even NOTED in the entry as a +caveat, and the theory was written as though it had not been. + +Three lessons, in the order they were paid for: + +- **A clipboard reading is only valid immediately after the copy.** It is + shared, global, mutable state that any application can overwrite at any + moment. Treat a late reading as no reading. +- **Do not theorise past your own caveat.** The measurement was correctly + labelled unreliable and then used as though it were reliable. A caveat that + does not stop the reasoning it qualifies is decoration. +- **Two eliminated explanations are not proof of a third.** Both original + readings were ruled out by real evidence, and the conclusion drawn was that + something more interesting must be wrong. The actual answer was that the + measurement distinguishing them was broken. + +**If the paste is still wanted**, the question is why GIMP does not take an +`image/png` that is demonstrably on the clipboard: a Wayland clipboard-manager +interaction, or GIMP's own paste path. Neither is this repository's, and neither +needs an item here until it is shown to be. 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 944dfee..82860a3 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 @@ -144,7 +144,7 @@ taking that too literally. | 75 | The tagging rules window forgets its size and its column widths | persistence | S | **done** 2026-08-13, shipped in 0.17.0. The window-kind question is left open, see the closed-items file | | 76 | Every field in the rules dialog is free text, so a rule is easy to get wrong | workflow | M | **done** 2026-08-13, shipped in 0.17.0. See `specs/2026-08-13-rule-builder-design.md` | | 77 | No way to see what a rule would collect, in the thread list | workflow | S | **done** 2026-08-13, shipped in 0.17.0 | -| 78 | No way to build a rule from something visible in a message | workflow | S | open, narrowed 2026-08-14; the search half shipped as item 85, which is the road a rule is made from. Now a shortcut across that road: the menus and the seeded-dialog path both exist. Use 85 first and see which values are worth promoting | +| 78 | No way to build a rule from something visible in a message | workflow | S | **dropped** 2026-08-17 at the user's request. Never a defect: item 85 built the road (right-click any value, search it, save the query) and item 81 the last step (a rule from a saved query), so the whole journey is available. This was only a shortcut across it, and the entry had already said to use 85 for a while before deciding which values were worth promoting. Reopen if that use turns up a value worth a one-click rule | | 80 | A rule with many conditions squeezes the rule list to one visible row | defect | XS | **done** 2026-08-13, shipped in 0.17.0. Follows item 76 | | 79 | Opening the rules dialog and saving destroys the first rule | defect | XS | **fixed on `rule-builder`** 2026-08-13, unreleased. Shipped in 0.16.0; damaged one real rule, repaired by hand | | 81 | No way to turn a saved query into a tagging rule | workflow | S | **done** 2026-08-14, unreleased; see `specs/2026-08-14-query-to-rule-design.md` | @@ -162,11 +162,11 @@ taking that too literally. | 94 | `pinned` has nothing left to decide once the buttons are built-in | maintenance | S | open; **blocked on 93**, and deliberately not part of it. A user-visible removal: the row becomes built-ins only and every saved query lives in the menu | | 96 | A query returning the thread already on display opens onto the placeholder | defect | S | **done** 2026-08-15, unreleased. Split from 66's unverified half, which had a different cause. Reproduced from two screenshots after four measured eliminations | | 97 | An edit made during a sync is reverted in the list when the sync ends | defect | S | **done** 2026-08-15, unreleased. Found by hand-testing item 89's fix. The sync-end refresh ran BEFORE the held-edit flush, so it read a database that still carried the old tag | -| 98 | "Important" adds the tag but cannot remove it, unlike every other toggle | defect | XS | open, found 2026-08-16 in the notes reconciliation | +| 98 | "Important" adds the tag but cannot remove it, unlike every other toggle | defect | XS | **done** 2026-08-17, unreleased. Calls `everySelectedRowHasTag()`, as the entry required. Its reply test needed THREE different states (list-first thread, the reply's own thread, the reply) before it could tell the two wrong answers apart; with the reply defaulted to its thread's state the item 105 mutation stayed green, measured | | 99 | The unread action is labelled "Toggle unread" whichever way it will go | presentation | S | open; depends on 98's toggle shape, and the label is harder than it looks | -| 100 | The message pane offers Back, Forward, Reload and Save page, none of which mean anything | defect | XS | open, found 2026-08-16. Chromium's standard menu is added wholesale | +| 100 | The message pane offers Back, Forward, Reload and Save page, none of which mean anything | defect | XS | **done** 2026-08-17, unreleased. `MessageView::removeBrowserActions()` filters the standard menu by `pageAction()` POINTER, never by text; `ViewSource` went with them, and stranded separators are swept | | 101 | Sync is account-aware for edits but not for the account the user is looking at | workflow | S | open; item 49 built the edit half deliberately. Needs a decision, see the entry | -| 102 | The rules table shows no note, so the field explaining a rule is invisible until it is opened | workflow | XS | open, found 2026-08-16 | +| 102 | The rules table shows no note, so the field explaining a rule is invisible until it is opened | workflow | XS | **done** 2026-08-17, unreleased. A Note column before `ColumnCount`, so the appended Matches column stays last. Found a second defect on the way: `restoreState` REFUSES a header state with a different column count, and the sized flags were being set regardless | | 103 | What Delete does to mail on the server is undocumented and unverified | clarification | S | open; a question first, possibly no code at all | | 104 | Mail visible in Thunderbird never reaches qtmaildir | defect | ? | open, reported 2026-08-16, cause NOT established. Most likely outside this repo; see the entry before writing code | | 109 | A root card's own message is invisible to a message-scoped write | defect | S | **done** 2026-08-16, unreleased. Found by hand-testing 108. `applyMessageTagChange` and `messageById` searched only the loaded replies, and a root's message is never among them, so the ORDINARY gesture repainted nothing and wiped the pane's chip row | @@ -176,6 +176,12 @@ taking that too literally. | 106 | A tag change made on one message during a sync is silently lost | defect | XS | **done** 2026-08-16, unreleased. Found by READING while fixing 105, never reported. `flushHeldEdits` re-sent only thread-scoped edits, so a message-scoped one was shown, counted as pending, and never written | | 107 | A thread-scoped write leaves the loaded replies showing their old tags | defect | XS | **done** 2026-08-16, unreleased. `applyTagChange` updated the summary only, so marking a thread read left its expanded replies bold | | 108 | Acting on a thread root means the whole thread, though it displays one message | workflow | M | **done** 2026-08-16, unreleased. `messageScopeFor()` beside `scopeFor()`; five `*_thread` actions in a "Whole thread" submenu on `Ctrl+Alt+<key>`. User-visible: minor bump, `### Upgrading` written | +| 112 | Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread | defect | S | open, found 2026-08-17. A toggle over a UNION has no direction on a mixed thread | +| 113 | No way to see a message's HTML source | information | S | open, 2026-08-17. Chromium's own View source cannot work here; needs our own plain-text dialog. Item 100 removed the dead entry, which was an overreach: the user had not asked for it | +| 114 | Save image is offered on every image and does nothing | defect | S | open, found 2026-08-17 by right-clicking a real image. No `downloadRequested` handler exists anywhere, so the request is emitted and never answered | +| 115 | A copy from the message pane gives no confirmation | presentation | XS | open, 2026-08-17. Copy link address, Copy image address and Copy image all work, silently. `statusMessage` already exists and already expires | +| 116 | Copy image copies markup instead of the image | defect | XS | **dropped** 2026-08-17, same day. NOT A DEFECT: `wl-paste --list-types` run immediately after a copy reports `image/png`, `application/x-qt-image` and 30 more image flavours. The clipboard is correct and Chromium is behaving. The earlier "text only" reading was taken minutes late off a clipboard that had been overwritten, and a whole cause was theorised on it | +| 117 | The message pane offers no Select all | workflow | XS | open, found 2026-08-17. NOT caused by item 100: verified by hand against a build with that filter reverted, and the menu holds Copy and the search entries either way | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -342,52 +348,6 @@ Those are three different features. **Size: `?`, unspecified**, and out of scope until v2 exists. Ask before designing anything. -## 78. No way to build a rule from something visible in a message - -**Observed.** The user would like to select an address or another piece of a -message in the main window, right-click, and be offered a rule built from it. - -**Cause.** Not a defect, unbuilt. - -**Narrowed 2026-08-14, and most of the work is already done.** The search half -shipped as item 85, which is the road a rule is made from: search for a value, -save the query, create a rule from the saved query. What remains here is a -SHORTCUT across that road, and both of its halves now exist. - -- The menus are built and every surface already extracts its value as a - finished query (`SearchOffer`, `src/searchterm.h`). A rule entry is another - action beside the two search ones, not new plumbing. -- The seeded-dialog path exists from item 81: - `MainWindow::showTagRulesDialog(const TagRule &seed)`. A rule from a message - becomes a second caller of it, with a different seed, which is what item 81's - spec anticipated when it made the seed a whole `TagRule` rather than a query - string. - -**Approach.** Decide it after using item 85 for a while. Which values are worth -promoting straight to a rule is a usage question, and the earlier answer to it -was wrong (see below), so it is worth having the evidence first. - -**Constraints.** The original approach here said to start from the thread -list's context menu "where the sender is already a value the model holds". -**That is false and item 85 verified it.** `ThreadSummary::authors` comes from -`notmuch_thread_get_authors` and is a DISPLAY SUMMARY, reading `Alice, Bob` or -`Alice| Bob`, so a `from:` built from it matches nothing. A real address comes -from `MessageNode::from` or `ParsedMessage::from`, neither of which the thread -list carries. Any thread-list entry needs an address resolved from a message -first. - -JavaScript is disabled in the profile and must stay disabled. Item 85 reads a -body selection with `QWebEnginePage::selectedText()`, which injects no script; -reuse that rather than adding anything. - -The rules file is shared with mailctl, so a rule created here must go through -`TagRules` and preserve unknown fields; see "Changing the shared rule format" -in CLAUDE.md. - -**Size: S**, down from M now that item 85 has built the menus and item 81 the -seeded dialog. - - ## 94. `pinned` has nothing left to decide once the buttons are built-in **Observed (user, 2026-08-15),** thinking past item 93 rather than from the @@ -433,59 +393,6 @@ bump either way: an ignored optional field is not a breaking change. **Size: S.** Removing a field, two UI affordances and their tests. -## 98. "Important" adds the tag but cannot remove it, unlike every other toggle - -**Observed (user, from the notes):** "the add 'Important' action should be a -toggle (like unread)." - -**Cause (verified in the code).** `src/mainwindow.cpp:863` registers `flag` as a -one-way add: - -```cpp -addAction(QStringLiteral("flag"), tr("&Important"), - tr("Mark the selected threads as important"), [this]() { - tagSelected({ QStringLiteral("flagged") }, {}, tr("Mark important")); -}); -``` - -Adding a tag that is already there is a no-op the user cannot see, so pressing -the key or the button on an already-important thread appears to do nothing at -all. Nothing in the UI removes `flagged` except the general tag dialog. - -**The two neighbouring actions are already toggles**, so this is an -inconsistency rather than a missing feature. `delete` -(`src/mainwindow.cpp:825`) and `toggle_unread` (`:867`) both read the current -state and choose a direction, and `delete`'s comment states the rule this should -follow: one direction for the WHOLE selection, flipping only when every selected -thread is already in the target state, because a single keystroke that leaves a -selection in two states is worse than either outcome. - -**Everything needed is already loaded.** `ThreadSummary::isFlagged()` -(`src/types.h:64`) reads the tag off the summary, so the direction can be -decided without a worker round trip, exactly as `isDeleted()` is. - -**Approach, now a two-line change.** Item 105 extracted -`MainWindow::everySelectedRowHasTag()`, which is the whole of the direction -logic: -`everySelectedRowHasTag("flagged") ? tagSelected({}, {"flagged"}, tr("Unmark important")) : the current add`. -The undo stack needs nothing new, since `TagChange::inverted()` already covers -both directions. - -**Constraints.** - -- Call `everySelectedRowHasTag()`, never a hand-rolled loop. Two separate fixes - went into that logic on 2026-08-16 (items 88 and 105) and both were bugs a - copy of the then-current `delete` loop would have inherited: resolving a - reply's row number to the wrong thread, and asking a reply's thread instead - of the reply. -- The action's tooltip says "Mark the selected threads as important" and would - become wrong. Item 99 is the same problem for `toggle_unread` and the two - should be decided together. -- The label question belongs to item 99, not here. This item is the behaviour - only: the key stops being a no-op. - -**Size: XS.** - ## 99. The unread action is labelled "Toggle unread" whichever way it will go **Observed (user, from the notes):** "the label for 'toggle unread' should be @@ -534,50 +441,6 @@ literals `lupdate` can see; a string built by concatenation is not translatable. **Size: S.** Mostly the mixed-selection and toolbar decisions, not the code. -## 100. The message pane offers Back, Forward, Reload and Save page, none of which mean anything - -**Observed (user, from the notes):** "back/forward/save page in the right pane -don't make sense, shouldn't be visible." - -**Cause (verified in the code).** `MessageView::showBodyContextMenu` -(`src/messageview.cpp:619`) starts from Chromium's own menu: - -```cpp -QMenu *menu = m_view->createStandardContextMenu(); -``` - -That menu is built for a browser and carries the navigation and page actions -whole. The pane is not a browser: every document arrives through `setHtml()` -with a fixed base URL, so there is no history to go back to, nothing to reload -from, and the request interceptor blocks everything by default anyway. The -entries are inert as well as meaningless. - -**Deliberate as far as it goes.** The comment above the call says the page's own -menu comes first so "copy, select all and the rest stay exactly as they were", -which is right for the editing actions and wrong for the navigation ones. The -item is that the filter was never applied, not that the base menu was a mistake. - -**Approach.** Keep the menu, drop the actions that cannot apply. Qt names them -as `QWebEnginePage::WebAction` values (`Back`, `Forward`, `Reload`, -`SavePage`, and `ViewSource` is worth the same look), and each has a -`pageAction()` whose pointer can be matched against the standard menu's entries -and removed. Removing by matching the action pointer is safer than matching by -text, which is translated. - -**Constraints.** - -- Do not rebuild the menu from scratch. Copy, Copy link address and Select all - are the reason the standard menu is used, and item 85's search entries are - appended to it. -- `Save page` is not the attachment save. Attachments have their own bar and - their own path-traversal checks (see the web view security notes in - `CLAUDE.md`); nothing here should grow a second way to write a file. -- Verify against a real right-click on a real message. The offscreen platform - builds the menu but a screenshot of it proves nothing, and the entry list - depends on what the page reports as available at that moment. - -**Size: XS.** - ## 101. Sync is account-aware for edits but not for the account the user is looking at **Observed (user, from the notes):** "sync button should be account-aware." @@ -621,40 +484,6 @@ reaches it (item 42), so most of this exists. **Size: S** for the on-demand button, XS for the visibility half. Ask which. -## 102. The rules table shows no note, so the field explaining a rule is invisible until it is opened - -**Observed (user, from the notes):** "add 'notes' column to the filters table." - -**Cause (verified in the code).** `TagRule` carries a `note` field -(`src/tagrules.h:37`, "Why the rule is shaped this way. Shown in the dialog"), -and the editor below the table edits it, but the table itself lists five columns -and none of them is the note (`src/tagrulesdialog.cpp:116`): - -```cpp -m_list->setHeaderLabels({ tr("On"), tr("Stage"), tr("Rule"), tr("Tags"), - tr("Matches") }); -``` - -So the one field written specifically to explain a rule can only be read one rule -at a time, by selecting it. With several rules the note is exactly the thing that -would let the user pick the right one without opening each. - -**Approach.** A sixth column. The column widths already persist (item 75), so a -new column needs a sensible default width and nothing else in the way of state. - -**Constraints.** - -- The note is free text of any length and would stretch the column. Elide it and - put the full text in the tooltip; the `Rule` column already faces the same - problem with a long query and is the pattern to match. -- `ColumnCount + 1` in `setColumnCount` is load-bearing: the enum drives the - column indices and there is a spare. Add the enum value rather than hardcoding - 5, and check every place that indexes a column by number. -- Notes are the user's own words and can be empty. An empty cell is correct - here; do not substitute a placeholder. - -**Size: XS.** - ## 103. What Delete does to mail on the server is undocumented and unverified **Observed (user, from the notes):** "verify how 'delete' works", with two @@ -757,6 +586,265 @@ distinguish three layers, because the fix lives in a different place for each: **Size: `?`** until reproduced. Most likely not a code change here at all. +## 112. Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread + +**Observed (user, 2026-08-17):** clicking a thread root and asking to mark the +whole thread unread does not do it. On a seven-message thread with two unread +replies, the result is that every message is toggled unread **except those +two**, which are left as they were. The user asks for an explicit "mark whole +thread read/unread" rather than a toggle. + +**Cause (verified in code):** the action exists, and its direction is the +defect. `toggle_unread_thread` (`src/mainwindow.cpp:931`, `Ctrl+Alt+U`) chooses +between adding and removing by asking +`everySelectedRowHasTag("unread", TagScope::Thread)`, which reads +`ThreadListModel::threadFor(index).tags`. That is notmuch's **union over the +thread** (`CLAUDE.md`, item 110), so a thread containing even one unread message +answers "unread" and the action picks *Mark thread read*. There is no input a +user can give that reaches *Mark thread unread* on a mixed thread: the only +threads that take that branch are the ones already entirely read, and the only +threads reporting "not unread" are the ones the user does not need the action +for. + +The write itself is absolute and correct. `tagSelected` with `TagScope::Thread` +adds or removes `unread` across every message, so the two unread replies in the +report are not skipped by the write. They are the reason the write ran in the +opposite direction from the one the user wanted. + +**A union is not a state, and a toggle needs a state.** This is the same class +as item 110 and the third time the union has produced a defect. Items 105 and 88 +fixed *which object* a toggle resolved; this one is about a thread having no +single answer to give. `everySelectedRowHasTag` is a two-valued predicate over a +three-valued reality: all read, all unread, or mixed. The mixed case is the one +that has no correct toggle direction, and picking either one silently is what +ships as "the action does the wrong thing". + +**Approach.** The user has already named it: stop toggling at thread scope. + +- Split `toggle_unread_thread` into two explicit actions, **Mark thread read** + and **Mark thread unread**, each with a fixed direction. Both appear in the + "Whole thread" submenu, where an entry always carries text, so a fixed label + is honest in a way a toggle's cannot be. +- The message-scoped `toggle_unread` stays a toggle. One message has a real + two-valued state, so the trap does not exist there. Do not "unify" the two: + the asymmetry is the point. + +**Constraints.** + +- **Adding an action is four places**, all enforced by tests that fail + confusingly: `KeyMap::knownActions()`, `defaultBindings()`, the icon table, + and the no-duplicate-icons exception list. See `CLAUDE.md`. Splitting one + action into two means one new entry in each, and the pair shares the twin's + icon under the existing named exemption for thread actions. +- **`Ctrl+Alt+U` is taken by the action being split**, and the whole-thread + bindings are already one modifier out from their twins because `Ctrl+Shift+U` + was claimed. Two directions need two sequences; if a second chord cannot be + found that is not worse than the menu, bind one and leave the other to the + submenu rather than inventing a three-modifier chord nobody will press. +- **This interacts with items 98 and 99**, which is the reason to decide all + three together. 99 asks for a dynamic label on the message-scoped toggle, + which is the opposite move: keep the toggle, make the label tell the truth. + A thread cannot do that, because on a mixed thread there is no true label to + show. Deciding 99 first will produce the wrong answer here by analogy. +- The undo entry must name the direction that ran (`Mark thread unread`), not + the action. `tagSelected` already takes the text, so this comes free from + splitting. +- **The test needs a MIXED thread**, which is the whole defect: a thread whose + messages are all in one state answers identically whichever way the direction + is computed, so a fixture built from a uniformly-unread thread passes against + the bug. Same trap as item 88's opposite-states requirement, recorded in + `CLAUDE.md`. + +**Size: S.** The write path is already correct and thread-scoped; the work is +the action split, the four registration sites, the binding decision, and a test +over a mixed thread. + + +## 113. No way to see a message's HTML source + +**Observed (user, 2026-08-17):** reviewing item 100's removals, "view source +could be useful, we might have to implement it." + +**This item exists because item 100 removed something it was not asked to.** +The user named Back, Forward, Reload and Save page. `ViewSource` was added to +that list by the agent, on the reasoning that it was "the same kind of thing", +and it is not: the other four have nothing to act on, while view-source has a +real document and a real use, checking what a message actually contains. The +removal is recorded here rather than quietly reverted, because the reasoning +that produced it is the part worth not repeating. + +**Cause (verified in code):** restoring Chromium's entry would not work anyway, +which is why this is an implementation item rather than a one-line revert. +`QWebEnginePage::ViewSource` navigates to `view-source:<url>`. The pane's +document is a `data:` URL (`requestinterceptor.cpp:90-105` records that +`setHtml()` navigates to data: and applies the base URL afterwards), and +`MessagePage::acceptNavigationRequest` accepts only a typed main-frame +navigation, so the attempt is refused before the interceptor even sees it. +Restoring the entry would produce a live-looking menu item that does nothing, +which is the same defect item 100 was reported for. + +**Approach.** Our own action, not Chromium's: a dialog showing the message's +HTML as plain text. The source is already in hand, since `HtmlBuilder` produced +it and `MimeParser` holds the original part; nothing needs fetching. + +**Constraints.** + +- **Plain text is a SECURITY property here, not a style.** `CLAUDE.md` states + it for `MessageDetailsDialog`, and it applies with more force to this: the + content is a stranger's markup, and the whole point of the dialog is to show + it uninterpreted. Set `Qt::PlainText` explicitly on whatever displays it; a + `QLabel` guesses under `Qt::AutoText`. A `QPlainTextEdit` cannot render + markup at all and is the obvious choice. +- Decide which source is shown: the message's ORIGINAL HTML part, or the + document `HtmlBuilder` generated around it. They are different, and the + useful one is almost certainly the original, since the wrapper is ours and + known. Say which in the dialog rather than leaving the user to guess. +- A message with no HTML part needs an answer that is not an empty window. + `messageview.cpp:933` already has the string for this case. +- Reachable from the body context menu, where the removed entry was, so the + gesture the user reached for keeps working. + +**Size: S.** + +## 114. Save image is offered on every image and does nothing + +**Observed (user, 2026-08-17):** right-clicking an image in the message pane +offers "save image", among other entries item 100 never saw because the test +built a menu by hand and no real image was ever clicked. + +**Cause (verified in code):** there is no download handling anywhere in the +tree. `grep -rn "downloadRequested\|DownloadRequest" src/` returns nothing, so +Chromium emits the request and no handler answers it. The entry is present, +looks live, and silently does nothing, which is the same class of defect as +item 100 itself. + +**Approach, and the security question it raised was resolved by the user.** +The first proposal was to scope this to `cid:` parts and refuse remote images, +on the grounds that saving a remote image means a fetch triggered from a +message. **The user pointed out that this is wrong**, and it is: once remote +content has been granted and loaded, the bytes are already fetched and cached. +Saving them is a local copy, not a new request, and blocking it adds no +security while making the entry useless. The reasoning applied to *fetching*, +which has already happened by the time the entry is reachable. + +The real constraint is the neighbouring one: **the save must not itself cause a +fetch.** An image that was never loaded, because it is remote and not granted, +has no bytes to save, and the entry should be unavailable rather than reaching +for the network to satisfy it. + +- Connect `QWebEngineProfile::downloadRequested` and let Chromium write the + file, with the directory chosen through `QFileDialog` as the attachment save + already does. +- The rejected alternative: fetching the bytes ourselves to reuse + `Attachment::saveTo()`. That is a second network request originating from a + message, which is exactly what the interceptor exists to prevent, and it + would bypass the per-render remote grant. `cid:` images would map onto that + path cleanly and remote ones cannot, which is why the uniform route wins. + +**Constraints.** + +- **The path checks are not optional and are already written.** `CLAUDE.md`'s + web-view notes: reduce to basename, strip separators, resolve against the + chosen directory, refuse anything escaping it, and compare resolved paths as + paths rather than with `startsWith`. A filename suggested by a download + request is untrusted input in exactly the way an attachment filename is. +- Do not let this become a second general download route. It saves an image the + user right-clicked, nothing else; `SavePage` stays removed. +- No overwriting. `saveWithoutOverwriting` exists because a silent overwrite + lost six of sixteen files while reporting every one as saved. +- Report the outcome through `statusMessage`, as every other save does. A save + with no feedback is the failure item 13 was about. + +**Size: S.** + +## 115. A copy from the message pane gives no confirmation + +**Observed (user, 2026-08-17):** Copy link address, Copy image address and Copy +image all work, and none of them says so. The user asked for "a small +'link copied' transient that appears and disappears after a few seconds". + +**Cause:** not a defect, unbuilt. These are Chromium's own menu entries and it +does not report success; nothing in this application is listening to them. + +**Copy image was thought to be broken and is not**; see item 116 in the +closed-items file, which is worth reading for how a stale clipboard reading +produced a confident wrong cause. + +All three work. Copy image was briefly filed as broken (item 116) and is not: +the pixels reach the clipboard correctly, and the reading that said otherwise +was taken off a stale clipboard. So this item covers the confirmation for Copy +link address, Copy image address and Copy image alike. + +**Approach.** `MessageView::statusMessage` already exists and the status bar +already expires its messages (item 33), so this is one connection per action and +no new widget, no new timer. `QWebEnginePage::action()` gives each entry's +QAction; connect `triggered` and emit the appropriate string. + +**Constraints.** + +- The message must name what was copied. "Copied" alone is worse than nothing + when three entries sit next to each other in the same menu. +- Do not build a floating overlay for this. The status bar is where this + application reports transient results, and a second mechanism for the same + job is the kind of thing item 45 recorded when two Sync buttons disagreed. +- Depends on nothing; can be built alongside 114 since both touch the same menu. + +**Size: XS.** + +## 117. The message pane offers no Select all + +**Observed (user, 2026-08-17):** right-clicking a body selection offers Copy and +the search entries, and no Select all. Noticed while hand-testing item 100. + +**NOT caused by item 100, and this was verified rather than argued.** The user +ran a build with `src/messageview.cpp` and `src/messageview.h` reverted to HEAD, +so `removeBrowserActions()` did not exist, and reported the same menu: Copy and +the search entries only. Chromium's standard menu for this pane has never +carried Select all. + +**Three wrong theories preceded that measurement**, which is the part worth +recording, because each was plausible and each cost a round trip: + +1. *The separator sweep removed it.* Disproved with a standalone program + reproducing the sweep against a realistic menu: it drops the leading + separator and keeps every action. +2. *Chromium omits it when there is no selection.* Killed by the user, who had a + selection at the time. +3. *A probe will show what the real menu holds.* `createStandardContextMenu()` + returns NULL outside an actual context-menu event, on the offscreen platform + and on a real display alike, so two probe attempts measured nothing. + +The lesson is the one item 100 had already written down and the agent did not +follow: **a menu built by hand proves nothing about the menu Chromium builds.** +`theBodyMenuDropsTheBrowsersOwnActions` constructs its own QMenu, which is right +for testing the filter and useless for testing what is offered. That test now +says so, and deliberately does NOT assert on SelectAll, since a passing +assertion there would read as a guarantee the code does not make. + +**Approach.** Add it explicitly rather than hoping Chromium supplies it. The +action already exists as `QWebEnginePage::SelectAll` and works; only the menu +entry is missing. + +- `menu->addAction(page->action(QWebEnginePage::SelectAll))` in + `showBodyContextMenu`, placed beside Copy rather than appended after the + search entries. +- Worth considering a `Ctrl+A` binding for the pane at the same time, though + note the pane is not the only focusable widget and the query bar has its own + claim on that key. Check `KeyMap::defaultBindings()` before adding one. + +**Constraints.** + +- **A test for this cannot use a hand-built menu.** That is the trap above. The + honest options are asserting the action is in the menu the production code + returns, which needs a real context-menu event, or leaving it to a hand test + and saying so. Do not write a test that constructs a QMenu and calls it + covered. +- Select all selects the rendered body, not the header label, which is a + separate widget with its own selection. That is probably the desired + behaviour but should be looked at rather than assumed. + +**Size: XS.** + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering |