2 files changed, 177 insertions, 1 deletions

diff --git a/src/messageview.cpp b/src/messageview.cpp
index a7d340a..3dbd0f1 100644
--- a/src/messageview.cpp
+++ b/src/messageview.cpp
@@ -58,6 +58,29 @@
 
 namespace {
 
+} // namespace
+
+MessageView::LinkOpener &linkOpenerRef()
+{
+    static MessageView::LinkOpener opener;
+    return opener;
+}
+
+void MessageView::setLinkOpener(LinkOpener opener)
+{
+    linkOpenerRef() = std::move(opener);
+}
+
+void MessageView::openExternally(const QUrl &url)
+{
+    if (const LinkOpener &opener = linkOpenerRef())
+        opener(url);
+    else
+        QDesktopServices::openUrl(url);
+}
+
+namespace {
+
 /// Intercepts link clicks so a message can never navigate the pane.
 class MessagePage : public QWebEnginePage
 {
@@ -105,7 +128,7 @@ protected:
                 return false;
             }
 
-            QDesktopServices::openUrl(url);
+            MessageView::openExternally(url);
             return false;
         }
 
@@ -114,12 +137,103 @@ protected:
         return !isMainFrame;
     }
 
+    /// Item 126. An anchor carrying target="_blank" never reaches
+    /// acceptNavigationRequest: Chromium asks for a new window instead, and
+    /// the base implementation returns nullptr, so the click is discarded with
+    /// no error and nothing on screen. Marketing HTML sets _blank on
+    /// practically every link, which is what made "HTML mail" look broken
+    /// while a plain-text mail's links worked.
+    ///
+    /// The obvious override cannot work: createWindow() is handed a
+    /// WebWindowType and NO url. The target arrives afterwards, as a
+    /// navigation on whatever page is returned, so returning nullptr throws it
+    /// away before it can be read.
+    ///
+    /// So return a page whose only job is to receive that navigation. It
+    /// reuses the same handler the plain-link path uses rather than sourcing
+    /// the URL a second way, which is what keeps the two kinds of link from
+    /// drifting apart. No view is ever created and nothing is ever fetched:
+    /// the page refuses the navigation, and deleteLater() disposes of it once
+    /// the URL has been handed on.
+    QWebEnginePage *createWindow(WebWindowType type) override
+    {
+        return makeRelay(type);
+    }
+
+public:
+    /// The same call Chromium makes, reachable from a test. See
+    /// MessageView::relayBlankTargetForTest().
+    QWebEnginePage *createWindowForTest(WebWindowType type)
+    {
+        return makeRelay(type);
+    }
+
+    /// The same call Chromium makes for a clicked anchor. See
+    /// MessageView::clickLinkForTest().
+    bool clickLinkForTest(const QUrl &url)
+    {
+        return acceptNavigationRequest(url, NavigationTypeLinkClicked, true);
+    }
+
+protected:
+
 private:
+    QWebEnginePage *makeRelay(WebWindowType)
+    {
+        return new LinkRelayPage(profile(), this);
+    }
+
+    /// Receives the navigation createWindow() could not see, hands the URL to
+    /// the external browser, and refuses. Never shown, never given a view.
+    class LinkRelayPage : public QWebEnginePage
+    {
+    public:
+        LinkRelayPage(QWebEngineProfile *profile, QObject *parent)
+            : QWebEnginePage(profile, parent) {}
+
+    protected:
+        bool acceptNavigationRequest(const QUrl &url, NavigationType,
+                                     bool) override
+        {
+            // Whatever the type, this page exists for exactly one URL and is
+            // finished the moment it has it.
+            if (url.isValid() && !url.scheme().isEmpty())
+                MessageView::openExternally(url);
+            deleteLater();
+            return false;
+        }
+    };
+
     QueryHandler m_onQuery;
 };
 
 } // namespace
 
+bool MessageView::clickLinkForTest(const QUrl &url)
+{
+    auto *page = static_cast<MessagePage *>(m_view->page());
+    if (!page)
+        return false;
+    return page->clickLinkForTest(url);
+}
+
+bool MessageView::relayBlankTargetForTest(const QUrl &url)
+{
+    // static_cast, not qobject_cast: MessagePage carries no Q_OBJECT, and the
+    // page is one this class constructed itself, so the type is not in doubt.
+    auto *page = static_cast<MessagePage *>(m_view->page());
+    if (!page)
+        return false;
+    // Chromium's own sequence: ask for the window, then navigate it. The type
+    // is what a target="_blank" anchor produces.
+    QWebEnginePage *relay = page->createWindowForTest(
+        QWebEnginePage::WebBrowserTab);
+    if (!relay)
+        return false;
+    relay->setUrl(url);
+    return true;
+}
+
 MessageView::MessageView(QWidget *parent)
     : QWidget(parent)
 {
@@ -691,6 +805,26 @@ void MessageView::removeBrowserActions(QMenu *menu, QWebEnginePage *page)
         QWebEnginePage::Forward,
         QWebEnginePage::Reload,
         QWebEnginePage::SavePage,
+        // Item 127. Chromium adds these only when the menu is raised over a
+        // LINK, so the four above, which are page actions, were the whole list
+        // until now and this was tested by right-clicking the page.
+        //
+        // None of the three can be honoured. There are no tabs, and a window
+        // means a second QWebEngineView, which the pane deliberately never
+        // creates: one view per message is one Chromium render process per
+        // message. "In this window" would navigate the pane away from the
+        // message, which no message may do.
+        //
+        // They became MORE dangerous with item 126, not less: that fix gives
+        // the page a real createWindow(), so an entry that used to be merely
+        // dead would now do something, and what it would do is open a link the
+        // user asked to open in a tab that does not exist.
+        //
+        // CopyLinkToClipboard is deliberately NOT here. It works, and it is
+        // the fallback for any link that still will not open.
+        QWebEnginePage::OpenLinkInNewTab,
+        QWebEnginePage::OpenLinkInNewWindow,
+        QWebEnginePage::OpenLinkInThisWindow,
     };
 
     for (const QWebEnginePage::WebAction which : kUnwanted) {
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);