diff options
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.md | 171 |
1 files changed, 171 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 4a4b1ff..4f361bf 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 @@ -5140,6 +5140,177 @@ set explicitly. And the procedure's own first step, holding `/tmp/mbsync.lock`, conflicts with hand-testing Delete, which auto-syncs; that surfaced item 125. The procedure lives outside this repo, in the user's own documents. +## 126. A link with `target="_blank"` does nothing when clicked + +**Observed (user, 2026-08-20):** "if I click on a link in a mail in the message +pane, nothing happens." Then, on checking: a plain-text mail (a GitHub PR reply) +opens its links correctly, while an HTML mail showing the link as a button does +not. + +**That second observation overturned the first diagnosis and is the whole +item.** This entry originally blamed `RequestInterceptor` blocking `https`. +That was wrong: the interceptor does deny `https`, but only for resources the +document FETCHES. A clicked link never becomes a request, because +`acceptNavigationRequest` calls `QDesktopServices::openUrl` and returns false +before anything is issued. If the interceptor were the cause, the GitHub link +could not work either, and it does. + +**Cause (verified against the two messages the user named).** The difference is +`target="_blank"`: + +| message | `target` | route taken | result | +|---|---|---|---| +| GitHub PR reply | none on any anchor | `acceptNavigationRequest` | opens correctly | +| HTML newsletter | `_blank` on every anchor | `createWindow()` | silently dropped | + +An anchor with no target navigates the main frame, so it reaches +`MessagePage::acceptNavigationRequest` as `NavigationTypeLinkClicked` and is +handed to the browser. An anchor asking for a new window does NOT: Chromium +routes it to `QWebEnginePage::createWindow()`, which `MessagePage` does not +override, so the base implementation returns `nullptr` and the click is +discarded. `acceptNavigationRequest` is never consulted, which is why no +existing code sees it and nothing at all happens. + +Both messages render HTML, so this is not a text-versus-HTML distinction: the +GitHub mail is `multipart/alternative` and its HTML part is what the pane shows. +Marketing HTML uses `target="_blank"` almost universally, which is why it reads +as "HTML mail is broken". + +**Approach.** Override `createWindow()` in `MessagePage` to hand the URL to the +same path a plain link takes and return `nullptr`, so no window is ever created. + +**One trap, and it decides the shape.** `createWindow()` is called WITHOUT the +target URL in Qt: the signature carries only a `WebWindowType`. The URL arrives +afterwards, as a navigation request on the page the override is expected to +return. Returning `nullptr` therefore discards the URL before it can be seen, so +the override cannot simply read it. Two known ways around it, and this needs +measuring before choosing: + +- Keep a `linkHovered` cache and use the last hovered URL. Cheap, and wrong if + the click arrives without a hover (keyboard activation, synthetic click). +- Return a throwaway `QWebEnginePage` whose `acceptNavigationRequest` hands the + URL to `openUrl` and refuses, then deletes itself. Correct by construction, + since the URL arrives through the normal path, at the cost of a short-lived + page object. + +The second is the one to verify first: it reuses the code that already works for +plain links rather than adding a second, differently-sourced route to the same +action. + +**Constraints.** + +- **No second `QWebEngineView` may be created**, whatever shape this takes. The + pane renders a list into one view precisely to avoid one Chromium render + process per message. +- **The pane must never navigate.** Whatever handles the URL must still refuse + the navigation, exactly as the plain-link path does. +- **`m_allowRemote` stays false.** Nothing here needs the interceptor relaxed: + the URL goes to an external browser and the pane fetches nothing. The earlier + draft of this item proposed loosening the interceptor and would have weakened + the remote-content protection for no reason. +- **A `mailto:` link is still item 123's question**, and marketing HTML carries + those with `target="_blank"` too. + +**Verification.** A test can assert that a `target="_blank"` anchor reaches +whatever handler is chosen; it cannot assert the browser opened. The regression +that matters is the routing, and it is invisible today because nothing observes +`createWindow()` at all. + +**Size: S.** + +## 127. A link's context menu offers four browser actions that cannot work + +**Observed (user, 2026-08-20):** right-clicking a link still offers "Open in new +tab", "open in new window", "save link", "copy link" and "select all". The user +identified them as probable survivors of an earlier removal, which is exactly +what they are. + +**Cause (verified).** Item 100 removed the page-level browser actions, and its +list is explicit (`src/messageview.cpp:689-694`): `Back`, `Forward`, `Reload`, +`SavePage`. Those four are what a standard menu offers on the PAGE. The +link-specific actions are different `WebAction` values entirely +(`QWebEnginePage::OpenLinkInNewTab`, `OpenLinkInNewWindow`, `DownloadLinkToDisk`, +`CopyLinkToClipboard`), and Chromium adds them only when the menu is raised over +a link. Item 100 was tested by right-clicking the page, so they were never in +the menu it was filtering and were never considered. + +**Two of them are dead and two are not**, which is why this is not a single +sweep: + +- `OpenLinkInNewTab` and `OpenLinkInNewWindow` cannot work at all. There are no + tabs, and a new window means a second `QWebEngineView`, which the pane + deliberately does not create (one Chromium render process per message is the + reason the pane renders a list into one view). Both are dead UI today. +- `DownloadLinkToDisk` needs a `downloadRequested` handler, which item 114 + records does not exist anywhere. It is dead for the same reason Save image is, + and should be decided WITH item 114 rather than separately. +- `CopyLinkToClipboard` works and is useful. It is the user's entire workaround + for item 126 on the messages that fail, by their own description. + +**Approach.** Remove `OpenLinkInNewTab` and `OpenLinkInNewWindow` by pointer, +the way `removeBrowserActions()` already does, so the removal survives +translation. Keep `CopyLinkToClipboard`. Leave `DownloadLinkToDisk` to item 114. + +**This is downstream of item 126 and should follow it.** Note that +`OpenLinkInNewTab` and `OpenLinkInNewWindow` fail through the SAME missing +`createWindow()` that 126 is about, so fixing 126 may well make both of them +start working. That would be worse rather than better: the pane must not open +windows, and two menu entries silently doing what one click should do is not the +design. Decide their fate after 126 lands, when it is known what they do rather +than what they fail to do. Removing them first and adding one back afterwards is +two changes to the same menu. + +**Constraints.** + +- **Filter by `pageAction()` POINTER, never by text.** Item 100 established + this and the reason is translation: the menu is Italian under `LANG=it_IT` + and a text match would silently stop matching. +- **Stranded separators must still be swept**, which `removeBrowserActions()` + already handles; removing two adjacent entries is exactly the case that + leaves one behind. +- **The call site cannot be tested**, per item 117: `createStandardContextMenu()` + returns nothing outside a real context-menu event. Assert on the filter + function against a menu built by hand, and state the gap rather than faking + coverage. + +**Size: XS**, and smaller still if done in the same sitting as 126. + +**Closed 2026-08-20, unreleased, both items in one sitting.** + +126: `MessagePage::createWindow()` returns a `LinkRelayPage`, a page with no +view whose `acceptNavigationRequest` hands the URL to +`MessageView::openExternally()` and refuses, then deletes itself. The relay +exists because `createWindow()` receives only a `WebWindowType`: the URL +arrives afterwards as a navigation on the returned page, so an override +returning `nullptr` discards it before it can be read. Routing it through the +same handler the plain-link path uses is what keeps the two kinds of link from +drifting apart. + +127: three actions added to `removeBrowserActions()`'s list, +`CopyLinkToClipboard` deliberately left. The order mattered: 126 gives the page +a real `createWindow()`, so those entries would have stopped being merely dead +and started opening links into a tab that does not exist. + +**Testing needed two seams, and the reason is worth keeping.** The click cannot +be synthesised: JavaScript is off in this profile, so `element.click()` does +nothing (measured, `runJavaScript` returns an invalid `QVariant`), and a +synthetic mouse press would have to land on the anchor's rect, which depends on +the desktop's fonts. `setUrl()` is no substitute either, since it arrives as +`NavigationTypeTyped` and takes the branch that accepts our own document load. +So `clickLinkForTest()` and `relayBlankTargetForTest()` drive the real overrides +on the real page, and `setLinkOpener()` substitutes a recorder for +`QDesktopServices::openUrl`, which would otherwise launch a browser. + +Both routes are asserted, not just the broken one: they end at the same handler +now, so breaking the working one while fixing the other was the plausible +regression. Three mutations checked, all caught: `createWindow` returning +`nullptr` fails the `_blank` test with the message naming the real defect, the +filter losing its link actions fails one direction, and the filter also removing +`CopyLinkToClipboard` fails the other. That last one matters because Copy link +is the user's fallback for any link that will not open, and a future sweep of +"dead link actions" would otherwise take it silently. + + ## 90. A saved-query button clears the account selection **Observed (user, notes):** "select an account and hit the 'unread' button, the |