Fix email links navigating the reader and opening repeated tabs

.github/workflows/checks.yml

@@ -57,6 +57,10 @@ jobs:
       - name: Smoke test published Linux app
         if: runner.os == 'Linux'
         run: xvfb-run -a python3 scripts/smoke-startup.py artifacts/smoke/BetterMail.dll
+      - name: Check email link navigation
+        if: runner.os == 'Linux'
+        timeout-minutes: 2
+        run: xvfb-run -a dotnet run --project tools/BetterMail.UiPreview --configuration Release --no-build -- artifacts/email-links --email-links
       - uses: actions/upload-artifact@v4
         with:
           name: smoke-${{ matrix.platform }}

src/BetterMail.App/BetterMail.App.csproj

@@ -50,5 +50,6 @@
   </ItemGroup>
   <ItemGroup>
     <InternalsVisibleTo Include="BetterMail.Tests" />
+    <InternalsVisibleTo Include="BetterMail.UiPreview" />
   </ItemGroup>
 </Project>

src/BetterMail.App/ConversationThreadView.axaml

@@ -220,7 +220,6 @@
 
       <NativeWebView Grid.Row="1"
                      x:Name="MessageWebView"
-                     NavigationStarted="MessageWebView_NavigationStarted"
                      AutomationProperties.Name="Selected conversation message content" />
     </Grid>
 

src/BetterMail.App/ConversationThreadView.axaml.cs

@@ -1,5 +1,4 @@
 using System.ComponentModel;
-using System.Diagnostics;
 using System.Text;
 using Avalonia;
 using Avalonia.Controls;
@@ -27,6 +26,7 @@ public sealed partial class ConversationThreadView : UserControl
     public ConversationThreadView()
     {
         InitializeComponent();
+        _ = new ReadOnlyWebViewLinks(MessageWebView);
         KeyDown += HandleKeyDown;
         ThreadHeaderScroll.AddHandler(PointerWheelChangedEvent, (_, _) => _scrollInputVersion++, Avalonia.Interactivity.RoutingStrategies.Tunnel);
         ThreadHeaderScroll.AddHandler(PointerPressedEvent, (_, _) => _scrollInputVersion++, Avalonia.Interactivity.RoutingStrategies.Tunnel);
@@ -260,26 +260,4 @@ Key.R when args.KeyModifiers.HasFlag(KeyModifiers.Shift) => viewModel.ReplyAllCo
             args.Handled = true;
         }
     }
-
-    private void MessageWebView_NavigationStarted(
-        object? sender,
-        WebViewNavigationStartingEventArgs args)
-    {
-        var request = args.Request;
-        if (request is null ||
-            (request.Scheme != Uri.UriSchemeHttp && request.Scheme != Uri.UriSchemeHttps))
-        {
-            return;
-        }
-
-        try
-        {
-            args.Cancel = Process.Start(
-                new ProcessStartInfo(request.AbsoluteUri) { UseShellExecute = true }) is not null;
-        }
-        catch
-        {
-            args.Cancel = false;
-        }
-    }
 }

src/BetterMail.App/GtkExternalNavigationGuard.cs

@@ -0,0 +1,75 @@
+using System.Runtime.InteropServices;
+using Avalonia.Platform;
+
+namespace BetterMail.App;
+
+// Work around Avalonia WebView 12.1.0's GTK cancellation bug at WebKit's policy boundary.
+// Returning TRUE alone does not reject a WebKit decision: an undecided decision defaults to use().
+internal sealed class GtkExternalNavigationGuard : IDisposable
+{
+    private readonly IntPtr _webView;
+    private readonly DecidePolicyCallback _callback; // Keep the native callback rooted until disconnect.
+    private nuint _handler;
+
+    private GtkExternalNavigationGuard(IntPtr webView, Action<Uri> open)
+    {
+        _webView = webView;
+        _callback = (_, decision, type, _) =>
+        {
+            if (type is not (0 or 1)) return 0; // Navigation / new-window actions only; images still load normally.
+            var action = webkit_navigation_policy_decision_get_navigation_action(decision);
+            var request = action == IntPtr.Zero ? IntPtr.Zero : webkit_navigation_action_get_request(action);
+            var value = request == IntPtr.Zero ? null : Marshal.PtrToStringUTF8(webkit_uri_request_get_uri(request));
+            var external = Uri.TryCreate(value, UriKind.Absolute, out var uri) && ReadOnlyWebViewLinks.IsExternal(uri);
+            if (!external && type != 1) return 0; // Allow application-supplied about:blank / local documents.
+
+            webkit_policy_decision_ignore(decision);
+            // A redirect or script-initiated navigation must never create browser tabs.
+            if (external && webkit_navigation_action_get_navigation_type(action) == 0 /* LINK_CLICKED */ &&
+                webkit_navigation_action_is_user_gesture(action) != 0)
+            {
+                try { open(uri!); }
+                catch (Exception exception)
+                {
+                    // Exceptions must not cross the native callback boundary.
+                    System.Diagnostics.Trace.TraceWarning("Could not dispatch external link: {0}", exception.GetType().Name);
+                }
+            }
+            return 1;
+        };
+        _handler = g_signal_connect_data(_webView, "decide-policy", _callback, IntPtr.Zero, IntPtr.Zero, 0);
+        if (_handler == 0) throw new InvalidOperationException("Could not protect embedded content navigation.");
+    }
+
+    public static GtkExternalNavigationGuard? Attach(IPlatformHandle? handle, Action<Uri> open) =>
+        OperatingSystem.IsLinux() && handle is IGtkWebViewPlatformHandle gtk && gtk.WebKitWebView != IntPtr.Zero
+            ? new(gtk.WebKitWebView, open) : null;
+
+    public void Dispose()
+    {
+        if (_handler == 0) return;
+        g_signal_handler_disconnect(_webView, _handler);
+        _handler = 0;
+        GC.KeepAlive(_callback);
+    }
+
+    [UnmanagedFunctionPointer(CallingConvention.Cdecl)]
+    private delegate int DecidePolicyCallback(IntPtr webView, IntPtr decision, int type, IntPtr data);
+
+    [DllImport("libgobject-2.0.so.0", CallingConvention = CallingConvention.Cdecl)]
+    private static extern nuint g_signal_connect_data(IntPtr instance, string signal, DecidePolicyCallback callback, IntPtr data, IntPtr destroy, int flags);
+    [DllImport("libgobject-2.0.so.0", CallingConvention = CallingConvention.Cdecl)]
+    private static extern void g_signal_handler_disconnect(IntPtr instance, nuint handler);
+    [DllImport("libwebkit2gtk-4.1.so.0", CallingConvention = CallingConvention.Cdecl)]
+    private static extern IntPtr webkit_navigation_policy_decision_get_navigation_action(IntPtr decision);
+    [DllImport("libwebkit2gtk-4.1.so.0", CallingConvention = CallingConvention.Cdecl)]
+    private static extern IntPtr webkit_navigation_action_get_request(IntPtr action);
+    [DllImport("libwebkit2gtk-4.1.so.0", CallingConvention = CallingConvention.Cdecl)]
+    private static extern int webkit_navigation_action_is_user_gesture(IntPtr action);
+    [DllImport("libwebkit2gtk-4.1.so.0", CallingConvention = CallingConvention.Cdecl)]
+    private static extern int webkit_navigation_action_get_navigation_type(IntPtr action);
+    [DllImport("libwebkit2gtk-4.1.so.0", CallingConvention = CallingConvention.Cdecl)]
+    private static extern IntPtr webkit_uri_request_get_uri(IntPtr request);
+    [DllImport("libwebkit2gtk-4.1.so.0", CallingConvention = CallingConvention.Cdecl)]
+    private static extern void webkit_policy_decision_ignore(IntPtr decision);
+}

src/BetterMail.App/NotesWorkspaceView.axaml

@@ -143,10 +143,9 @@
         </Grid>
       </Border>
 
-      <NativeWebView Grid.Row="2"
+      <NativeWebView x:Name="PageWebView" Grid.Row="2"
                      Source="{Binding PageBodyUri}"
                      IsVisible="{Binding HasSelectedPage}"
-                     NavigationStarted="PageWebView_NavigationStarted"
                      AutomationProperties.Name="OneNote page content" />
       <ProgressBar Grid.Row="2"
                    IsIndeterminate="True"

src/BetterMail.App/NotesWorkspaceView.axaml.cs

@@ -1,4 +1,3 @@
-using System.Diagnostics;
 using System.ComponentModel;
 using Avalonia;
 using Avalonia.Controls;
@@ -17,6 +16,7 @@ public sealed partial class NotesWorkspaceView : UserControl
     public NotesWorkspaceView()
     {
         InitializeComponent();
+        _ = new ReadOnlyWebViewLinks(PageWebView);
         NotesTree.AddHandler(TreeViewItem.ExpandedEvent, async (_, args) =>
         {
             if (args.Source is TreeViewItem { DataContext: NoteTreeNode node } && DataContext is NotesWorkspaceViewModel vm)
@@ -230,24 +230,4 @@ Key.N when args.KeyModifiers.HasFlag(KeyModifiers.Control) => viewModel.NewPageC
             args.Handled = true;
         }
     }
-
-    private void PageWebView_NavigationStarted(
-        object? sender, WebViewNavigationStartingEventArgs args)
-    {
-        var request = args.Request;
-        if (request is null ||
-            (request.Scheme != Uri.UriSchemeHttp && request.Scheme != Uri.UriSchemeHttps))
-        {
-            return;
-        }
-        try
-        {
-            args.Cancel = Process.Start(
-                new ProcessStartInfo(request.AbsoluteUri) { UseShellExecute = true }) is not null;
-        }
-        catch
-        {
-            args.Cancel = false;
-        }
-    }
 }

src/BetterMail.App/ReadOnlyWebViewLinks.cs

@@ -0,0 +1,54 @@
+using System.Diagnostics;
+using Avalonia.Controls;
+using Avalonia.Threading;
+
+namespace BetterMail.App;
+
+internal sealed class ReadOnlyWebViewLinks
+{
+    private readonly Action<Uri> _open;
+    private GtkExternalNavigationGuard? _gtk;
+
+    public ReadOnlyWebViewLinks(NativeWebView view, Action<Uri>? open = null)
+    {
+        _open = open ?? (uri => { using var process = Process.Start(new ProcessStartInfo(uri.AbsoluteUri) { UseShellExecute = true }); });
+        view.AdapterCreated += (_, args) =>
+        {
+            _gtk?.Dispose();
+            _gtk = GtkExternalNavigationGuard.Attach(args.TryGetPlatformHandle(),
+                uri => Dispatcher.UIThread.Post(() => Open(uri, _open)));
+        };
+        view.AdapterDestroyed += (_, _) => { _gtk?.Dispose(); _gtk = null; };
+        view.NavigationStarted += (_, args) =>
+        {
+            // GTK 12.1.0 returns TRUE for Cancel without calling policy_decision_ignore.
+            // Let our following native handler reject the decision and identify user clicks.
+            if (_gtk is null) CancelNavigation(args, _open);
+        };
+        view.NewWindowRequested += (_, args) =>
+        {
+            if (_gtk is not null) return;
+            args.Handled = true;
+            if (IsExternal(args.Request)) Open(args.Request!, _open);
+        };
+    }
+
+    internal static bool IsExternal(Uri? uri) => uri?.Scheme is "http" or "https" or "mailto";
+
+    internal static void CancelNavigation(WebViewNavigationStartingEventArgs args, Action<Uri> open)
+    {
+        if (!IsExternal(args.Request)) return;
+        args.Cancel = true; // Independent of OS launch success or a returned Process handle.
+        Open(args.Request!, open);
+    }
+
+    private static void Open(Uri uri, Action<Uri> open)
+    {
+        try { open(uri); }
+        catch (Exception exception)
+        {
+            // Never fall back to loading a website inside the email if the OS handler fails.
+            Trace.TraceWarning("Could not open external link: {0}", exception.GetType().Name);
+        }
+    }
+}

tests/BetterMail.Tests/ExternalLinkTests.cs

@@ -0,0 +1,43 @@
+using Avalonia.Controls;
+using BetterMail.App;
+
+namespace BetterMail.Tests;
+
+public sealed class ExternalLinkTests
+{
+    [Theory]
+    [InlineData("https://example.test/path")]
+    [InlineData("http://example.test/path")]
+    [InlineData("mailto:person@example.test")]
+    public void ExternalNavigationIsCancelledEvenWithoutAProcessHandle(string address)
+    {
+        var args = new WebViewNavigationStartingEventArgs { Request = new Uri(address) };
+        var opened = new List<Uri>();
+        ReadOnlyWebViewLinks.CancelNavigation(args, uri =>
+        {
+            Assert.True(args.Cancel); // Cancel before invoking the OS handler.
+            opened.Add(uri);
+        });
+        Assert.True(args.Cancel);
+        Assert.Equal(args.Request, Assert.Single(opened));
+    }
+
+    [Fact]
+    public void FailedBrowserLaunchNeverFallsBackToNavigation()
+    {
+        var args = new WebViewNavigationStartingEventArgs { Request = new Uri("https://example.test") };
+        ReadOnlyWebViewLinks.CancelNavigation(args, _ => throw new InvalidOperationException("No browser"));
+        Assert.True(args.Cancel);
+    }
+
+    [Theory]
+    [InlineData("about:blank")]
+    [InlineData("file:///tmp/fictional-message.html")]
+    [InlineData("data:text/html,fictional")]
+    public void ApplicationDocumentsDoNotOpenBrowserTabs(string address)
+    {
+        var args = new WebViewNavigationStartingEventArgs { Request = new Uri(address) };
+        ReadOnlyWebViewLinks.CancelNavigation(args, _ => throw new Xunit.Sdk.XunitException("Unexpected launch"));
+        Assert.False(args.Cancel);
+    }
+}