diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-20 10:35:05 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-20 10:35:05 +0200 |
| commit | 5a71cbbbc2326586aa4ef326a4ffa5dc7f5e285a (patch) | |
| tree | 26649641d2bd4d416160230a9235ad08b4adb4b6 /docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | |
| parent | c1e150b08360ff312732528fefb8bbc6cc7db588 (diff) | |
| download | qtmaildir-5a71cbbbc2326586aa4ef326a4ffa5dc7f5e285a.tar.gz qtmaildir-5a71cbbbc2326586aa4ef326a4ffa5dc7f5e285a.zip | |
docs: correct item 126, the cause is createWindow, not the interceptor
The first diagnosis was wrong and the user's own follow-up disproved it: a
plain-text GitHub mail opens its links correctly while an HTML newsletter
does not. If RequestInterceptor blocking https were the cause, neither
would work.
Verified against the two messages named. The difference is target="_blank".
An anchor with no target navigates the main frame and reaches
acceptNavigationRequest, which hands it to QDesktopServices::openUrl; that
path works today. An anchor asking for a new window is routed by Chromium
to QWebEnginePage::createWindow(), which MessagePage does not override, so
the base implementation returns nullptr and the click is discarded before
any existing code observes it. Marketing HTML uses _blank almost
universally, which is what makes it read as "HTML mail is broken".
Both messages render HTML, so this was never a text-versus-HTML
distinction: the GitHub mail is multipart/alternative and its HTML part is
what the pane shows.
The entry also drops the proposal to let main-frame navigations through the
interceptor. That would have weakened the remote-content protection to fix
something it was not causing. Nothing here needs m_allowRemote relaxed: the
URL goes to an external browser and the pane fetches nothing.
Records the trap that decides the fix's shape: createWindow() receives no
URL, only a WebWindowType, so an override returning nullptr discards the
target before it can be read.
Item 127 is updated to match. OpenLinkInNewTab and OpenLinkInNewWindow fail
through the same missing createWindow(), so fixing 126 may make them start
working, which is worse rather than better.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
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 | 140 |
1 files changed, 80 insertions, 60 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 2227cc6..f56f037 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,7 +195,7 @@ 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 | Clicking a link in a message does nothing | defect | S | open, 2026-08-20. The interceptor blocks the request before `acceptNavigationRequest` can hand it to the browser, so the `QDesktopServices::openUrl` already in the code is never reached | +| 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 | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -1056,64 +1056,81 @@ Then Delete a message. Verified by hand on 2026-08-20; this is how it was found. **Size: S.** -## 126. Clicking a link in a message does nothing +## 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. The only way now is to copy that link and paste it in a -browser." - -**Cause (verified in the code, and it is not the missing handler it looks -like).** `MessagePage::acceptNavigationRequest` (`src/messageview.cpp:89-109`) -already handles this correctly: a `NavigationTypeLinkClicked` is passed to -`QDesktopServices::openUrl(url)` and then refused as a navigation, so the pane -cannot follow it. That code is right and has presumably never run. - -**`RequestInterceptor` blocks the request first.** `shouldAllow()` -(`src/requestinterceptor.cpp:73-77`) denies `http` and `https` whenever -`m_allowRemote` is false, which is the default for every message and is reset -per message by `resetForNewMessage()`. The interceptor runs on the REQUEST, -before the page is asked whether to accept the navigation, so the click is -dropped at the network layer and `acceptNavigationRequest` is never consulted. -The user sees nothing at all: no error, no navigation, no browser. - -**This is the remote-content protection working as designed**, and the fix must -not weaken it. Blocking `https` is what stops a tracking pixel or a remote -stylesheet phoning home when a message is merely displayed. The bug is that a -deliberate CLICK is indistinguishable, at that layer, from a resource the -document fetched on its own. - -**Approach.** The distinction the interceptor cannot make is available one level -up: a click arrives as `ResourceTypeMainFrame` with a `NavigationTypeLinkClicked`, -while a tracking pixel arrives as `ResourceTypeImage` and never as a navigation -at all. Two shapes are worth considering, and the choice is the user's: - -- Let the main-frame navigation through the interceptor so - `acceptNavigationRequest` can see it, hand it to `QDesktopServices::openUrl`, - and still refuse the navigation. Nothing is ever fetched by the pane; the URL - only reaches the external browser. -- Or intercept the click before it becomes a request at all, which avoids - relaxing anything in the interceptor but needs the link target from the - context-menu data or a `linkHovered` cache. +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.** -- **`m_allowRemote` must stay false by default**, and this must not become a - reason to loosen it. Opening a link in an external browser is the user's - explicit act; fetching a resource into the pane is not. -- **The pane must still never navigate.** `acceptNavigationRequest` returning - false for a link click is what protects that, and it stays whatever else - changes. -- **A `mailto:` link is a separate question** and probably belongs to item 123, - since composing is v2. Until then it should behave like any other scheme - rather than being special-cased into silence. -- **`qtmaildir-query:` links must keep their existing path.** They are the - placeholder's own helper lines and are handled before the `openUrl` call; - a message body carrying one is already refused by the view. - -**Verification.** Not testable through the offscreen platform end to end, since -it ends in an external browser. The reachable assertion is that a link click -reaches `acceptNavigationRequest` at all, which is what fails today; the -`openUrl` call itself is one line beyond that and is better confirmed by hand. +- **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.** @@ -1143,17 +1160,20 @@ sweep: - `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 currently the user's entire - workaround for item 126, by their own description. +- `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.** If 126 makes a click -open the system browser, then "Open link" becomes a meaningful entry and the -right menu is one entry that does what the click does, rather than two Chromium -entries that do nothing. Removing them first and adding one back afterwards is +**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.** |
