fix(app): the share invitation holds only what it should, and its fallback is reached when it is needed

From the independent review of this pull request:
- The links are built from the parsed address (scheme, host, path); an address
  with a user name, a query or a fragment, and a version with a control
  character, are refused.
- An invitation too long for a link (FitsInALink) is shown for copying instead
  of being handed to a mail program that might cut it short.
- A launch that throws is treated like one that found no mail program.
- The log says what happened after it happened, including a dialog that could
  not be shown because another one is open.
- The clipboard copy is flushed, so the text survives gclo closing.
- The box that holds the text tells a screen reader why the dialog is there.
- The message no longer sends its reader to the issue tracker: filing an issue
  there needs an account, and registration is closed. The reader is asked to
  tell the sender.

Refs #163

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

README.md

@@ -118,7 +118,7 @@ On launch, gclo shows a brief branded splash, then the **Quick Sync** connect ca
 
 **Activity log:** View > Activity log… shows the tail of today's log and links to the logs folder. Logs live under `%LOCALAPPDATA%\KofTwentyTwo\gclo\logs`, one file per day, kept for 30 days; they record run parameters and per-repo failures and never contain your token (see [SECURITY.md](SECURITY.md)).
 
-**Share:** Help > Share gclo by email… opens a new message in your email program with an invitation already written: what gclo is, where to download the latest release, how to install it, and where to send feedback. Type an address and send; gclo itself sends nothing. If no email program is set up, the text is shown and copied to the clipboard, to paste into a message by hand.
+**Share:** Help > Share gclo by email… opens a new message in your email program with an invitation already written: what gclo is, where to download the latest release, and how to install it. Type an address and send; gclo itself sends nothing. If no email program is set up, the text is shown and copied to the clipboard, to paste into a message by hand.
 
 **Updates:** Help > Check for updates… downloads and applies the latest release for your channel. Self-update is only available in installed builds (not the portable zip or a local debug build).
 

gclo.Engine.Tests/ShareInvitationTests.cs

@@ -23,7 +23,7 @@ public sealed class ShareInvitationTests
 
 
    [Fact]
-   public void TheBody_SaysWhatItIs_WhereToGetIt_HowToInstallIt_AndWhereToSendFeedback()
+   public void TheBody_SaysWhatItIs_WhereToGetIt_AndHowToInstallIt()
    {
       ShareInvitation invitation = Invitation();
 
@@ -34,7 +34,10 @@ public sealed class ShareInvitationTests
       Assert.Contains("scoop bucket add gclo " + Repository, invitation.Body);
       Assert.Contains("scoop install gclo", invitation.Body);
       Assert.Contains(Repository + "#readme", invitation.Body);
-      Assert.Contains(Repository + "/issues/new/choose", invitation.Body);
+      // Filing an issue needs an account on the project's server; the message does
+      // not send a stranger to a sign-in page. They answer the person who wrote.
+      Assert.DoesNotContain("/issues", invitation.Body);
+      Assert.Contains("tell me what you think", invitation.Body);
       Assert.EndsWith("(sent from gclo 1.2.3 (abc1234))", invitation.Body);
    }
 
@@ -62,6 +65,7 @@ public sealed class ShareInvitationTests
       string link = Invitation().MailtoUri.OriginalString;
 
       Assert.True(link.Length < ShareInvitation.MaxLinkLength, $"the link is {link.Length} characters long");
+      Assert.True(Invitation().FitsInALink);
       Assert.DoesNotContain(' ', link);
       Assert.DoesNotContain('\n', link);
       Assert.DoesNotContain('\r', link);
@@ -103,17 +107,50 @@ public sealed class ShareInvitationTests
 
 
 
+   [Fact]
+   public void SpaceAroundTheAddress_IsNotPartOfTheLinks()
+   {
+      var invitation = ShareInvitation.Create("  " + Repository + "  ", "1.2.3");
+
+      Assert.Contains("\r\n" + Repository + "/releases/latest\r\n", invitation.Body);
+   }
+
+
+
+   [Fact]
+   public void AnInvitationTooLongForALink_SaysSo()
+   {
+      var invitation = ShareInvitation.Create("https://example.test/" + new string('a', 1200), "1.2.3");
+
+      Assert.True(invitation.MailtoUri.OriginalString.Length > ShareInvitation.MaxLinkLength);
+      Assert.False(invitation.FitsInALink);
+   }
+
+
+
    [Theory]
    [InlineData("")]
    [InlineData("   ")]
    [InlineData("not a web address")]
    [InlineData("ftp://example.test/gclo")]
    [InlineData("/relative/path")]
-   public void AnAddressThatIsNotAWebAddress_IsRefused(string repositoryUrl)
+   [InlineData("https://someone:secret@example.test/gclo")] // a name and a password would travel in every link
+   [InlineData("https://example.test/gclo?tab=readme")]
+   [InlineData("https://example.test/gclo#readme")]
+   public void AnAddressThatIsNotAPlainWebAddress_IsRefused(string repositoryUrl)
        => Assert.ThrowsAny<ArgumentException>(() => ShareInvitation.Create(repositoryUrl, "1.2.3"));
 
 
 
+   [Theory]
+   [InlineData("1.2.3\r\nBcc: someone@example.test")]
+   [InlineData("1.2.3\nmore")]
+   [InlineData("1.2.3\t(x)")]
+   public void AVersionWithAControlCharacter_IsRefused(string version)
+       => Assert.ThrowsAny<ArgumentException>(() => ShareInvitation.Create(Repository, version));
+
+
+
    [Fact]
    public void AMissingVersion_IsRefused()
    {

gclo.ViewModels/ShareInvitation.cs

@@ -9,16 +9,17 @@ namespace gclo.ViewModels;
 /// <summary>
 /// An invitation to try the app, written as an email that the user's own mail
 /// program opens ready to send: a subject, a plain-text body that says what the app
-/// is, where to download the latest release, how to install it, and where to send
-/// feedback, and the <c>mailto:</c> link that carries both. The link names no
-/// recipient, so the user types an address and sends. Nothing is sent by the app,
-/// and nothing about the user, their accounts, or their repositories is in the text.
+/// is, where to download the latest release, and how to install it, and the
+/// <c>mailto:</c> link that carries both. The link names no recipient, so the user
+/// types an address and sends. Nothing is sent by the app, and nothing about the
+/// user, their accounts, or their repositories is in the text.
 /// </summary>
 public sealed class ShareInvitation
 {
    /// <summary>
-   /// The longest <c>mailto:</c> link that is handed to a mail program. Some of them
-   /// cut a longer one short; the text is written to stay well below this.
+   /// The longest <c>mailto:</c> link worth handing to a mail program: some of them
+   /// cut a longer one short. The text is written to stay well below this; see
+   /// <see cref="FitsInALink"/> for an invitation that does not.
    /// </summary>
    public const int MaxLinkLength = 2000;
 
@@ -42,25 +43,51 @@ public sealed class ShareInvitation
    /// <summary>The link that opens a new message with the subject and the body filled in, and no recipient.</summary>
    public Uri MailtoUri { get; }
 
-   /// <summary>Subject and body as one text, for the clipboard when no mail program takes the link.</summary>
+   /// <summary>Subject and body as one text, to show and copy when no mail program takes the link.</summary>
    public string AsPlainText => "Subject: " + Subject + "\r\n\r\n" + Body;
 
+   /// <summary>
+   /// False when <see cref="MailtoUri"/> is longer than <see cref="MaxLinkLength"/>:
+   /// a mail program might then open with half a message, so the text is better shown
+   /// for copying.
+   /// </summary>
+   public bool FitsInALink => MailtoUri.OriginalString.Length <= MaxLinkLength;
+
 
 
    /// <summary>Writes the invitation.</summary>
-   /// <param name="repositoryUrl">The project's public page (https), which the release, documentation, and feedback links hang off.</param>
+   /// <param name="repositoryUrl">
+   /// The project's public page, which the release and documentation links hang off:
+   /// an http(s) address made of a host and a path, nothing else.
+   /// </param>
    /// <param name="version">The running app's version, named at the end so the recipient knows what the sender uses.</param>
-   /// <exception cref="ArgumentException"><paramref name="repositoryUrl"/> is not an absolute http(s) address.</exception>
+   /// <exception cref="ArgumentException">
+   /// <paramref name="repositoryUrl"/> is not an absolute http(s) address, or carries
+   /// a user name, a query, or a fragment; or <paramref name="version"/> holds a
+   /// control character. Either would put something into the message that the links
+   /// and lines of it are not meant to hold.
+   /// </exception>
    public static ShareInvitation Create(string repositoryUrl, string version)
    {
       ArgumentException.ThrowIfNullOrWhiteSpace(repositoryUrl);
       ArgumentException.ThrowIfNullOrWhiteSpace(version);
-      if(!Uri.TryCreate(repositoryUrl, UriKind.Absolute, out Uri? page) || page.Scheme is not ("https" or "http"))
+      if(!Uri.TryCreate(repositoryUrl.Trim(), UriKind.Absolute, out Uri? page) || page.Scheme is not ("https" or "http"))
       {
          throw new ArgumentException($"'{repositoryUrl}' is not a web address.", nameof(repositoryUrl));
       }
+      if(page.UserInfo.Length > 0 || page.Query.Length > 0 || page.Fragment.Length > 0)
+      {
+         throw new ArgumentException(
+             "The project's page is a host and a path: no user name, no query, no fragment.", nameof(repositoryUrl));
+      }
+      if(version.Any(char.IsControl))
+      {
+         throw new ArgumentException("The version is one line of text.", nameof(version));
+      }
 
-      string repository = repositoryUrl.TrimEnd('/');
+      // From the parsed address, not from the text as given: what goes into the
+      // message is the scheme, the host, and the path, and nothing more.
+      string repository = page.GetLeftPart(UriPartial.Path).TrimEnd('/');
       string[] lines =
       [
           "Hi,",
@@ -77,7 +104,6 @@ public sealed class ShareInvitation
           "scoop install gclo",
           "",
           "How to use it: " + repository + "#readme",
-          "Problems and ideas: " + repository + "/issues/new/choose",
           "",
           "If you try it, tell me what you think.",
           "",

gclo/MainWindow.xaml.cs

@@ -1166,7 +1166,7 @@ public sealed partial class MainWindow : Window
 
    /// <summary>
    /// Help > Share gclo by email: hands an invitation (what gclo is, where to download
-   /// it, how to install it, where to send feedback) to the user's email program as a
+   /// it, how to install it) to the user's email program as a
    /// new message with no recipient, so they type an address and send. gclo sends
    /// nothing itself. When no email program takes it, the text is shown, and copied
    /// to the clipboard when the clipboard can be had, to be pasted into a message by
@@ -1178,23 +1178,25 @@ public sealed partial class MainWindow : Window
       {
          var invitation = ShareInvitation.Create(RepoUrl, BuildVersion.Describe(typeof(MainWindow).Assembly));
 
-         // The UI suite must not open an email program on the machine it runs on:
-         // under its fixture the invitation is only shown.
-         bool opened = !UiTestFixture.IsEnabled && await Windows.System.Launcher.LaunchUriAsync(invitation.MailtoUri);
-         if(opened)
+         // The UI suite must not open an email program on the machine it runs on,
+         // nor touch the clipboard, which belongs to the whole desktop (it would
+         // overwrite what the person at the machine copied): under its fixture the
+         // invitation is only shown.
+         bool isUnderTest = UiTestFixture.IsEnabled;
+         if(!isUnderTest && invitation.FitsInALink && await TryOpenInEmailProgramAsync(invitation))
          {
             _log.Info("Share: the invitation was handed to the email program.");
             return;
          }
 
-         // The clipboard belongs to the whole desktop: the UI suite leaves it alone
-         // (it would overwrite what the person at the machine copied), and when
-         // another program holds it the text is still shown, to be copied by hand.
-         bool copied = !UiTestFixture.IsEnabled && TryCopyToClipboard(invitation.AsPlainText);
-         _log.Info(copied
-             ? "Share: no email program took the invitation; it was copied to the clipboard and shown."
-             : "Share: no email program took the invitation; it was shown.");
-         await DialogGuard.ShowAsync(NewShareDialog(invitation, copied));
+         // When another program holds the clipboard the text is still shown, to be
+         // copied by hand; the dialog says which of the two it is.
+         bool copied = !isUnderTest && TryCopyToClipboard(invitation.AsPlainText);
+         ContentDialogResult? shown = await DialogGuard.ShowAsync(NewShareDialog(invitation, copied));
+         _log.Info(
+             (isUnderTest ? "Share: under the UI test fixture no email program is asked; " : "Share: no email program took the invitation; ")
+             + (copied ? "it was copied to the clipboard and " : "it was ")
+             + (shown is null ? "could not be shown, because another dialog is open." : "shown."));
       }
       catch(Exception ex)
       {
@@ -1205,6 +1207,25 @@ public sealed partial class MainWindow : Window
 
 
 
+   /// <summary>
+   /// Asks Windows to open the invitation in the email program; false when none took
+   /// it, or when asking failed, which is said in the log and treated the same way.
+   /// </summary>
+   private async Task<bool> TryOpenInEmailProgramAsync(ShareInvitation invitation)
+   {
+      try
+      {
+         return await Windows.System.Launcher.LaunchUriAsync(invitation.MailtoUri);
+      }
+      catch(Exception ex)
+      {
+         _log.Error($"Share: the email program could not be asked ({ex.Message}).", ex);
+         return false;
+      }
+   }
+
+
+
    /// <summary>Puts text on the clipboard; false, with a line in the log, when the clipboard cannot be had.</summary>
    private bool TryCopyToClipboard(string text)
    {
@@ -1213,6 +1234,8 @@ public sealed partial class MainWindow : Window
          var package = new Windows.ApplicationModel.DataTransfer.DataPackage();
          package.SetText(text);
          Windows.ApplicationModel.DataTransfer.Clipboard.SetContent(package);
+         // Hand the text over for good: without this it is gone when gclo closes.
+         Windows.ApplicationModel.DataTransfer.Clipboard.Flush();
          return true;
       }
       catch(Exception ex)
@@ -1240,18 +1263,17 @@ public sealed partial class MainWindow : Window
          MaxHeight = 320,
          Text = invitation.AsPlainText,
       };
+      string explanation = copied
+          ? "No email program opened the invitation. It has been copied to the clipboard: paste it into a new message, add an address, and send."
+          : "No email program opened the invitation. Copy the text below into a new message, add an address, and send.";
       Microsoft.UI.Xaml.Automation.AutomationProperties.SetAutomationId(text, "ShareInvitationText");
       Microsoft.UI.Xaml.Automation.AutomationProperties.SetName(text, "Invitation text");
+      // Focus lands in the box: a screen reader then also says why the dialog is there.
+      Microsoft.UI.Xaml.Automation.AutomationProperties.SetHelpText(text, explanation);
       ScrollViewer.SetVerticalScrollBarVisibility(text, ScrollBarVisibility.Auto);
 
       var panel = new StackPanel { Spacing = 12 };
-      panel.Children.Add(new TextBlock
-      {
-         Text = copied
-             ? "No email program opened the invitation. It has been copied to the clipboard: paste it into a new message, add an address, and send."
-             : "No email program opened the invitation. Copy the text below into a new message, add an address, and send.",
-         TextWrapping = TextWrapping.Wrap,
-      });
+      panel.Children.Add(new TextBlock { Text = explanation, TextWrapping = TextWrapping.Wrap });
       panel.Children.Add(text);
 
       return new ContentDialog