From a066af60bf6e3ec03ce21a301158325e9e199266 Mon Sep 17 00:00:00 2001 From: bcbetterninja <327058824+bcbetterninja@users.noreply.github.com> Date: Thu, 1 Oct 2026 07:24:26 +0000 Subject: [PATCH] Keep email links outside the reader and stop repeated browser tabs --- .github/workflows/checks.yml | 4 + src/BetterMail.App/BetterMail.App.csproj | 1 + .../ConversationThreadView.axaml | 1 - .../ConversationThreadView.axaml.cs | 24 +--- .../GtkExternalNavigationGuard.cs | 75 ++++++++++ src/BetterMail.App/NotesWorkspaceView.axaml | 3 +- .../NotesWorkspaceView.axaml.cs | 22 +-- src/BetterMail.App/ReadOnlyWebViewLinks.cs | 54 +++++++ tests/BetterMail.Tests/ExternalLinkTests.cs | 43 ++++++ .../Program.EmailLinks.cs | 134 ++++++++++++++++++ tools/BetterMail.UiPreview/Program.cs | 2 +- 11 files changed, 315 insertions(+), 48 deletions(-) create mode 100644 src/BetterMail.App/GtkExternalNavigationGuard.cs create mode 100644 src/BetterMail.App/ReadOnlyWebViewLinks.cs create mode 100644 tests/BetterMail.Tests/ExternalLinkTests.cs create mode 100644 tools/BetterMail.UiPreview/Program.EmailLinks.cs diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index ceb3f75..f143d6d 100644 --- a/.github/workflows/checks.yml +++ b/.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 }} diff --git a/src/BetterMail.App/BetterMail.App.csproj b/src/BetterMail.App/BetterMail.App.csproj index f394590..e1a958f 100644 --- a/src/BetterMail.App/BetterMail.App.csproj +++ b/src/BetterMail.App/BetterMail.App.csproj @@ -50,5 +50,6 @@ + diff --git a/src/BetterMail.App/ConversationThreadView.axaml b/src/BetterMail.App/ConversationThreadView.axaml index e72ce4a..7983d67 100644 --- a/src/BetterMail.App/ConversationThreadView.axaml +++ b/src/BetterMail.App/ConversationThreadView.axaml @@ -220,7 +220,6 @@ diff --git a/src/BetterMail.App/ConversationThreadView.axaml.cs b/src/BetterMail.App/ConversationThreadView.axaml.cs index 9e4954c..3e8811b 100644 --- a/src/BetterMail.App/ConversationThreadView.axaml.cs +++ b/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; - } - } } diff --git a/src/BetterMail.App/GtkExternalNavigationGuard.cs b/src/BetterMail.App/GtkExternalNavigationGuard.cs new file mode 100644 index 0000000..9f0ed70 --- /dev/null +++ b/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 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 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); +} diff --git a/src/BetterMail.App/NotesWorkspaceView.axaml b/src/BetterMail.App/NotesWorkspaceView.axaml index 2e5f997..26a581e 100644 --- a/src/BetterMail.App/NotesWorkspaceView.axaml +++ b/src/BetterMail.App/NotesWorkspaceView.axaml @@ -143,10 +143,9 @@ - { 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; - } - } } diff --git a/src/BetterMail.App/ReadOnlyWebViewLinks.cs b/src/BetterMail.App/ReadOnlyWebViewLinks.cs new file mode 100644 index 0000000..4958460 --- /dev/null +++ b/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 _open; + private GtkExternalNavigationGuard? _gtk; + + public ReadOnlyWebViewLinks(NativeWebView view, Action? 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 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 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); + } + } +} diff --git a/tests/BetterMail.Tests/ExternalLinkTests.cs b/tests/BetterMail.Tests/ExternalLinkTests.cs new file mode 100644 index 0000000..b96075c --- /dev/null +++ b/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(); + 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); + } +} diff --git a/tools/BetterMail.UiPreview/Program.EmailLinks.cs b/tools/BetterMail.UiPreview/Program.EmailLinks.cs new file mode 100644 index 0000000..e450ff8 --- /dev/null +++ b/tools/BetterMail.UiPreview/Program.EmailLinks.cs @@ -0,0 +1,134 @@ +using System.Diagnostics; +using System.Net; +using System.Net.Sockets; +using System.Text; +using Avalonia; +using Avalonia.Controls; +using BetterMail.App; + +internal static partial class Program +{ + private static async Task CheckEmailLinksAsync() + { + // Fictional loopback redirect trap. Never launch a browser or send a remote request. + using var stop = new CancellationTokenSource(); + var server = new TcpListener(IPAddress.Loopback, 0); + server.Start(); + var target = new Uri($"http://127.0.0.1:{((IPEndPoint)server.LocalEndpoint).Port}/redirect"); + var requests = 0; + var clients = new System.Collections.Concurrent.ConcurrentBag(); + var serving = Task.Run(async () => + { + try + { + while (!stop.IsCancellationRequested) + { + var client = await server.AcceptTcpClientAsync(stop.Token); + clients.Add(Respond(client)); + } + } + catch (OperationCanceledException) when (stop.IsCancellationRequested) { } + }); + var view = new NativeWebView(); + var launches = new List(); + var failLaunch = false; + _ = new ReadOnlyWebViewLinks(view, uri => + { + launches.Add(uri); + if (failLaunch) throw new InvalidOperationException("Synthetic unavailable browser"); + }); + var window = new Window { Content = view, Width = 640, Height = 400, Position = new(0, 0), + WindowDecorations = WindowDecorations.None, WindowStartupLocation = WindowStartupLocation.Manual }; + var html = new MailContentRenderer().RenderDocument($"Fictional email link", isHtml: true); + try + { + window.Show(); window.Activate(); + await Load(); + await ClickAndCheck(1); + // Explicitly exercise new-window navigation as well as ordinary navigation. + await view.InvokeScript("document.querySelector('a').target='_blank'"); + await ClickAndCheck(2); + failLaunch = true; + await view.InvokeScript("document.querySelector('a').removeAttribute('target')"); + await ClickAndCheck(3); + failLaunch = false; + // No user gesture: a redirect or a script must not open another external tab. + await view.InvokeScript($"location.href={System.Text.Json.JsonSerializer.Serialize(target.AbsoluteUri)}"); + await Task.Delay(400); + await Check(3); + // Detach/re-attach must disconnect the old native policy handler. + window.Content = null; + await Task.Delay(200); + window.Content = view; + await Load(); + await ClickAndCheck(4); + Console.WriteLine("Email links passed: one launch per click, same email retained, zero redirect requests, popup and failed-launch handling, reattachment."); + } + finally + { + window.Close(); + stop.Cancel(); + await serving; + await Task.WhenAll(clients); + server.Stop(); + } + + async Task Respond(TcpClient client) + { + using (client) + { + try + { + // Hover may preconnect without navigating; count actual HTTP requests. + if (await client.GetStream().ReadAsync(new byte[4096], stop.Token) == 0) return; + Interlocked.Increment(ref requests); + var response = Encoding.ASCII.GetBytes("HTTP/1.1 302 Found\r\nLocation: /another-redirect\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"); + await client.GetStream().WriteAsync(response, stop.Token); + } + catch (OperationCanceledException) when (stop.IsCancellationRequested) { } + catch (IOException) { /* WebKit can close an unused speculative connection. */ } + } + } + + async Task Load() + { + var loaded = new TaskCompletionSource(); + void Completed(object? sender, WebViewNavigationCompletedEventArgs args) + { + if (args.IsSuccess) loaded.TrySetResult(); + } + view.NavigationCompleted += Completed; + try + { + view.NavigateToString(html, new Uri("about:blank")); + await loaded.Task.WaitAsync(TimeSpan.FromSeconds(15)); + await Task.Delay(300); + } + finally { view.NavigationCompleted -= Completed; } + } + + async Task ClickAndCheck(int expected) + { + var point = view.PointToScreen(new Point(65, 35)); + var start = new ProcessStartInfo("python3"); + foreach (var arg in new[] { "tools/BetterMail.UiPreview/input.py", "none", "click", point.X.ToString(), point.Y.ToString() }) start.ArgumentList.Add(arg); + using var process = Process.Start(start)!; + await process.WaitForExitAsync(); + if (process.ExitCode != 0) throw new InvalidOperationException("Mouse injection failed."); + var deadline = DateTime.UtcNow.AddSeconds(5); + while (launches.Count < expected && DateTime.UtcNow < deadline) await Task.Delay(50); + await Task.Delay(300); + await Check(expected); + } + + async Task Check(int expected) + { + if (launches.Count != expected || launches.Any(uri => uri != target)) + throw new InvalidOperationException($"Expected {expected} external launches; received {launches.Count}."); + if (Volatile.Read(ref requests) != 0) + throw new InvalidOperationException("The email WebView contacted the external redirect endpoint."); + if (await view.InvokeScript("document.body.textContent.includes('Fictional email link')") != "true") + throw new InvalidOperationException("External navigation replaced the email."); + } + } +} diff --git a/tools/BetterMail.UiPreview/Program.cs b/tools/BetterMail.UiPreview/Program.cs index 2190de9..f7b01df 100644 --- a/tools/BetterMail.UiPreview/Program.cs +++ b/tools/BetterMail.UiPreview/Program.cs @@ -23,7 +23,7 @@ public static void Main(string[] args) using var stop = new CancellationTokenSource(); Dispatcher.UIThread.Post(async () => { - try { if (args.Contains("--compose-input")) await CheckComposeInputAsync(); else if (args.Contains("--busy")) await CaptureBusyAsync(output); else if (args.Contains("--workspaces")) await CaptureWorkspaceFixesAsync(output); else if (args.Contains("--search")) await CaptureSearchAsync(output); else await CaptureAsync(output); } + try { if (args.Contains("--email-links")) await CheckEmailLinksAsync(); else if (args.Contains("--compose-input")) await CheckComposeInputAsync(); else if (args.Contains("--busy")) await CaptureBusyAsync(output); else if (args.Contains("--workspaces")) await CaptureWorkspaceFixesAsync(output); else if (args.Contains("--search")) await CaptureSearchAsync(output); else await CaptureAsync(output); } catch (Exception ex) { Console.Error.WriteLine(ex); Environment.ExitCode = 1; } finally { stop.Cancel(); } });