aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
diff options
context:
space:
mode:
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md')
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md294
1 files changed, 294 insertions, 0 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.