diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-20 10:45:33 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-20 10:45:33 +0200 |
| commit | 3f256acda192fbe2b9fad28d09cd74dca9a69418 (patch) | |
| tree | e77e97c9f23425f2fc767361562ab3f1dd3730ca /src/messageview.h | |
| parent | 5a71cbbbc2326586aa4ef326a4ffa5dc7f5e285a (diff) | |
| download | qtmaildir-3f256acda192fbe2b9fad28d09cd74dca9a69418.tar.gz qtmaildir-3f256acda192fbe2b9fad28d09cd74dca9a69418.zip | |
fix(pane): open a target="_blank" link, and drop the dead link actions
Items 126 and 127, in one sitting because the second is only safe after
the first.
126: an anchor carrying target="_blank" did nothing when clicked, with no
error and nothing on screen. Chromium routes such a click to
QWebEnginePage::createWindow() rather than to acceptNavigationRequest, and
MessagePage did not override it, so the base implementation returned
nullptr and the URL was discarded before any of our code saw it. Plain
anchors were unaffected and already worked, which is why this presented as
"HTML mail is broken" while a text mail's links opened: marketing HTML sets
_blank on practically every anchor.
createWindow() receives a WebWindowType and no URL, so an override cannot
simply read the target: it arrives afterwards as a navigation on whatever
page is returned. LinkRelayPage is that page. It has no view, hands the URL
to the same handler the plain-link path uses, refuses the navigation, and
deletes itself. Nothing is ever fetched and no second QWebEngineView is
created.
127: OpenLinkInNewTab, OpenLinkInNewWindow and OpenLinkInThisWindow join
removeBrowserActions()'s list. Item 100's list is the PAGE actions and was
tested by right-clicking the page; these appear only over a link, so it
never saw them. CopyLinkToClipboard stays, being the fallback for any link
that will not open. The order matters: 126 gives the page a working
createWindow(), so those entries would have stopped being dead and started
opening links into a tab that does not exist.
Testing needed two seams. The click cannot be synthesised, since JavaScript
is off in this profile (measured: runJavaScript returns an invalid QVariant)
and a synthetic press would depend on the anchor's rect and the desktop's
fonts; setUrl() is no substitute because it arrives as NavigationTypeTyped.
clickLinkForTest() and relayBlankTargetForTest() drive the real overrides on
the real page, and setLinkOpener() substitutes a recorder for
QDesktopServices::openUrl.
Both routes are asserted rather than only the broken one, since they share a
handler now. Three mutations checked and caught, including the filter also
removing CopyLinkToClipboard, which a later sweep of "dead link actions"
would otherwise take silently.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YDq53rMd3AQp7QmcZzpuBM
Diffstat (limited to 'src/messageview.h')
| -rw-r--r-- | src/messageview.h | 42 |
1 files changed, 42 insertions, 0 deletions
diff --git a/src/messageview.h b/src/messageview.h index 09fc905..3cc1604 100644 --- a/src/messageview.h +++ b/src/messageview.h @@ -20,6 +20,8 @@ #include <QList> #include <QUrl> + +#include <functional> #include <QTimer> #include <QWidget> @@ -57,6 +59,46 @@ public: /// interceptor fails closed and the pane renders nothing at all. static QUrl documentUrl() { return QUrl(QStringLiteral("qtmaildir://message")); } + /// How a clicked link reaches the outside world. + /// + /// A seam, because the alternative is untestable: the call sits inside + /// MessagePage, ends in QDesktopServices::openUrl(), and a passing test + /// would have to launch a real browser. Item 126's regression is about + /// WHICH clicks arrive here, not about what openUrl does, so a test + /// substitutes a recorder and asserts on the URLs it collects. + /// + /// Production never sets this; the default opens the system browser. + using LinkOpener = std::function<void(const QUrl &)>; + static void setLinkOpener(LinkOpener opener); + static void openExternally(const QUrl &url); + + /// Asks the pane's page for the window a target="_blank" click wants, and + /// drives the returned page with `url` exactly as Chromium would. + /// + /// A test hook, and it exists because the alternative proves nothing. + /// MessagePage lives in an anonymous namespace so createWindow() cannot be + /// called directly, and the click itself cannot be synthesised: JavaScript + /// is off in this profile (verified, runJavaScript returns an invalid + /// QVariant), so `element.click()` does nothing, and a synthetic mouse + /// press would have to land on the anchor's rect, which depends on the + /// desktop's fonts. This drives the real override on the real page. + /// + /// Returns false when the page declined to provide one at all, which is + /// the pre-item-126 behaviour and the regression worth catching. + bool relayBlankTargetForTest(const QUrl &url); + + /// Drives the pane's page with a link click, as + /// acceptNavigationRequest() sees one. + /// + /// The same reasoning as relayBlankTargetForTest(): the click cannot be + /// synthesised. setUrl() is no substitute, because it arrives as + /// NavigationTypeTyped and takes the branch that accepts our own document + /// load, never the link branch. + /// + /// Returns what the page decided: false means the navigation was refused, + /// which is what a link click must always produce here. + bool clickLinkForTest(const QUrl &url); + /// Renders a whole thread, oldest first. Items whose expanded flag is /// false collapse to a one-line stub. void showThread(const QList<ThreadRenderItem> &items); |
