diff options
Diffstat (limited to 'docs/superpowers')
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 171 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 139 |
2 files changed, 173 insertions, 137 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 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 |