aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-17 13:49:55 +0200
committerDanilo M. <danix@danix.xyz>2026-08-17 13:49:55 +0200
commit53e7c1d4497909dc3aa9871e0e0fafb071fc8734 (patch)
treef86e45454c3eeade3a7aa0ce54e593093ba82c6f /docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
parenteb009da9c9f4e2e80ddb0f1b66f8bc9c02d52d89 (diff)
downloadqtmaildir-53e7c1d4497909dc3aa9871e0e0fafb071fc8734.tar.gz
qtmaildir-53e7c1d4497909dc3aa9871e0e0fafb071fc8734.zip
docs: reconcile the backlog, close 98/100/102, drop 78 and 116
The reconciliation against the user's own notes found ZERO unrecorded entries, the first clean pass: the 2026-08-16 sweep added items 98 to 104 and those absorbed the whole current "Not done yet" list. Closed this session, sections moved to the closed-items file: 98, 100, 102. Dropped: - 78, at the user's request. Never a defect. Items 85, 23 and 81 already give the whole journey (right-click a value, search it, save the query, make a rule from it); this was only a shortcut across it, and the entry had already said to gather usage evidence first. That evidence never appeared. - 116, the same day it was raised, and its section is kept for the process failure rather than the non-bug. Copy image was reported as copying markup instead of pixels. Two explanations were eliminated 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. A wl-paste reading taken minutes after the copy showed text flavours only, was explicitly labelled unreliable in the entry, and was then reasoned from anyway. Run immediately after a copy it reports image/png and 30 more, and pasting into GIMP immediately works. A caveat that does not stop the reasoning it qualifies is decoration. Opened: - 112, Toggle unread on a whole thread cannot reach "all unread" on a partly-read thread. The direction comes from notmuch's UNION over the thread, so one unread message anywhere makes the action pick "mark read" and no input reaches the other branch. Third defect from that union after 110. - 113, view source as our own plain-text dialog. - 114, Save image is offered and does nothing: no downloadRequested handler exists anywhere. The user corrected the first proposal, which would have refused remote images on security grounds; once remote content is granted the bytes are already fetched, so saving them is a local copy and blocking it protects nothing. - 115, no confirmation when a copy succeeds. - 117, the pane offers no Select all. NOT caused by item 100: verified against a build with that filter reverted. Three wrong theories preceded that measurement, and the lesson is one item 100 had already written down: a menu built by hand proves nothing about the menu Chromium builds. The changelog's Unreleased section gains Important-as-a-toggle, the rules Note column, the menu fix, and two Upgrading notes.
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.