diff options
Diffstat (limited to 'docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 139 |
1 files changed, 2 insertions, 137 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index f56f037..3848a6d 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 @@ -195,8 +195,8 @@ taking that too literally. | 125 | A skipped sync leaves the spinner running for ever | defect | S | open, 2026-08-20, found by hand. `mailsync.sh` exits 75 (EX_TEMPFAIL) when another run holds the lock; the indicator never clears, and a held edit waits for a completion that never comes | -| 126 | A link with `target="_blank"` does nothing when clicked | defect | S | open, 2026-08-20. `createWindow()` is not overridden, so Chromium drops the click before `acceptNavigationRequest` sees it. Plain links work; marketing HTML uses `_blank` almost universally | -| 127 | A link's context menu offers four browser actions that cannot work | defect | XS | open, 2026-08-20. Item 100 removed the page-level actions and never saw these: they appear only on a link. Follows 126, which decides which of them should survive | +| 126 | A link with `target="_blank"` does nothing when clicked | defect | S | **done** 2026-08-20, unreleased. `createWindow()` returns a relay page that receives the navigation, hands the URL to the browser and refuses. The URL cannot be read in `createWindow()` itself, which is why a relay rather than a lookup | +| 127 | A link's context menu offers four browser actions that cannot work | defect | XS | **done** 2026-08-20, unreleased. Three Open-in actions removed, `CopyLinkToClipboard` kept. Item 126 made them more dangerous rather than less: with a real `createWindow()` they would have started working | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1056,141 +1056,6 @@ Then Delete a message. Verified by hand on 2026-08-20; this is how it was found. **Size: S.** -## 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. - ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering |