Merge pull request ppy#28363 from peppy/external-link-open-trusted

Bypass external link dialog for links on the trusted osu! domain
MingxuanGame · May 31, 2024 · b76ec96 · b76ec96
2 parents 8196c00 + 69990c3
commit b76ec96
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 16 deletions.
diff --git a/osu.Game.Tests/Visual/Menus/TestSceneMainMenu.cs b/osu.Game.Tests/Visual/Menus/TestSceneMainMenu.cs
@@ -6,6 +6,7 @@
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Testing;
 using osu.Game.Online.API.Requests.Responses;
+using osu.Game.Overlays;
 using osu.Game.Screens.Menu;
 using osuTK.Input;
 
@@ -15,8 +16,43 @@ public partial class TestSceneMainMenu : OsuGameTestScene
     {
         private OnlineMenuBanner onlineMenuBanner => Game.ChildrenOfType<OnlineMenuBanner>().Single();
 
+        public override void SetUpSteps()
+        {
+            base.SetUpSteps();
+            AddStep("don't fetch online content", () => onlineMenuBanner.FetchOnlineContent = false);
+        }
+
+        [Test]
+        public void TestOnlineMenuBannerTrusted()
+        {
+            AddStep("set online content", () => onlineMenuBanner.Current.Value = new APIMenuContent
+            {
+                Images = new[]
+                {
+                    new APIMenuImage
+                    {
+                        Image = @"https://assets.ppy.sh/main-menu/project-loved-2@2x.png",
+                        Url = $@"{API.WebsiteRootUrl}/home/news/2023-12-21-project-loved-december-2023",
+                    }
+                }
+            });
+            AddAssert("system title not visible", () => onlineMenuBanner.State.Value, () => Is.EqualTo(Visibility.Hidden));
+            AddStep("enter menu", () => InputManager.Key(Key.Enter));
+            AddUntilStep("system title visible", () => onlineMenuBanner.State.Value, () => Is.EqualTo(Visibility.Visible));
+            AddUntilStep("image loaded", () => onlineMenuBanner.ChildrenOfType<OnlineMenuBanner.MenuImage>().FirstOrDefault()?.IsLoaded, () => Is.True);
+
+            AddStep("click banner", () =>
+            {
+                InputManager.MoveMouseTo(onlineMenuBanner);
+                InputManager.Click(MouseButton.Left);
+            });
+
+            // Might not catch every occurrence due to async nature, but works in manual testing and saves annoying test setup.
+            AddAssert("no dialog", () => Game.ChildrenOfType<DialogOverlay>().SingleOrDefault()?.CurrentDialog == null);
+        }
+
         [Test]
-        public void TestOnlineMenuBanner()
+        public void TestOnlineMenuBannerUntrustedDomain()
         {
             AddStep("set online content", () => onlineMenuBanner.Current.Value = new APIMenuContent
             {
@@ -25,13 +61,22 @@ public void TestOnlineMenuBanner()
                     new APIMenuImage
                     {
                         Image = @"https://assets.ppy.sh/main-menu/project-loved-2@2x.png",
-                        Url = @"https://osu.ppy.sh/home/news/2023-12-21-project-loved-december-2023",
+                        Url = @"https://google.com",
                     }
                 }
             });
             AddAssert("system title not visible", () => onlineMenuBanner.State.Value, () => Is.EqualTo(Visibility.Hidden));
             AddStep("enter menu", () => InputManager.Key(Key.Enter));
             AddUntilStep("system title visible", () => onlineMenuBanner.State.Value, () => Is.EqualTo(Visibility.Visible));
+            AddUntilStep("image loaded", () => onlineMenuBanner.ChildrenOfType<OnlineMenuBanner.MenuImage>().FirstOrDefault()?.IsLoaded, () => Is.True);
+
+            AddStep("click banner", () =>
+            {
+                InputManager.MoveMouseTo(onlineMenuBanner);
+                InputManager.Click(MouseButton.Left);
+            });
+
+            AddUntilStep("wait for dialog", () => Game.ChildrenOfType<DialogOverlay>().SingleOrDefault()?.CurrentDialog != null);
         }
     }
 }
diff --git a/...sation/DeleteConfirmationDialogStrings.cs → osu.Game/Localisation/DialogStrings.cs b/...sation/DeleteConfirmationDialogStrings.cs → osu.Game/Localisation/DialogStrings.cs
@@ -5,14 +5,14 @@
 
 namespace osu.Game.Localisation
 {
-    public static class DeleteConfirmationDialogStrings
+    public static class DialogStrings
     {
-        private const string prefix = @"osu.Game.Resources.Localisation.DeleteConfirmationDialog";
+        private const string prefix = @"osu.Game.Resources.Localisation.Dialog";
 
         /// <summary>
         /// "Caution"
         /// </summary>
-        public static LocalisableString HeaderText => new TranslatableString(getKey(@"header_text"), @"Caution");
+        public static LocalisableString Caution => new TranslatableString(getKey(@"header_text"), @"Caution");
 
         /// <summary>
         /// "Yes. Go for it."

diff --git a/osu.Game/Online/Chat/ExternalLinkOpener.cs b/osu.Game/Online/Chat/ExternalLinkOpener.cs
@@ -8,8 +8,10 @@
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.Platform;
 using osu.Game.Configuration;
+using osu.Game.Localisation;
 using osu.Game.Overlays;
 using osu.Game.Overlays.Dialog;
+using CommonStrings = osu.Game.Resources.Localisation.Web.CommonStrings;
 
 namespace osu.Game.Online.Chat
 {
@@ -44,26 +46,26 @@ public partial class ExternalLinkDialog : PopupDialog
         {
             public ExternalLinkDialog(string url, Action openExternalLinkAction, Action copyExternalLinkAction)
             {
-                HeaderText = "Just checking...";
-                BodyText = $"You are about to leave osu! and open the following link in a web browser:\n\n{url}";
+                HeaderText = DialogStrings.Caution;
+                BodyText = $"Are you sure you want to open the following link in a web browser?\n\n{url}";
 
                 Icon = FontAwesome.Solid.ExclamationTriangle;
 
                 Buttons = new PopupDialogButton[]
                 {
                     new PopupDialogOkButton
                     {
-                        Text = @"Yes. Go for it.",
+                        Text = @"Open in browser",
                         Action = openExternalLinkAction
                     },
                     new PopupDialogCancelButton
                     {
-                        Text = @"Copy URL to the clipboard instead.",
+                        Text = @"Copy URL to the clipboard",
                         Action = copyExternalLinkAction
                     },
                     new PopupDialogCancelButton
                     {
-                        Text = @"No! Abort mission!"
+                        Text = CommonStrings.ButtonsCancel,
                     },
                 };
             }

diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs
@@ -485,10 +485,19 @@ public void HandleLink(LinkDetails link) => Schedule(() =>
             }
         });
 
-        public void OpenUrlExternally(string url, bool bypassExternalUrlWarning = false) => waitForReady(() => externalLinkOpener, _ =>
+        public void OpenUrlExternally(string url, bool forceBypassExternalUrlWarning = false) => waitForReady(() => externalLinkOpener, _ =>
         {
+            bool isTrustedDomain;
+
             if (url.StartsWith('/'))
-                url = $"{API.APIEndpointUrl}{url}";
+            {
+                url = $"{API.WebsiteRootUrl}{url}";
+                isTrustedDomain = true;
+            }
+            else
+            {
+                isTrustedDomain = url.StartsWith(API.WebsiteRootUrl, StringComparison.Ordinal);
+            }
 
             if (!url.CheckIsValidUrl())
             {
@@ -500,7 +509,7 @@ public void OpenUrlExternally(string url, bool bypassExternalUrlWarning = false)
                 return;
             }
 
-            externalLinkOpener.OpenUrlExternally(url, bypassExternalUrlWarning);
+            externalLinkOpener.OpenUrlExternally(url, forceBypassExternalUrlWarning || isTrustedDomain);
         });
 
         /// <summary>

diff --git a/osu.Game/Overlays/Dialog/DangerousActionDialog.cs b/osu.Game/Overlays/Dialog/DangerousActionDialog.cs
@@ -30,20 +30,20 @@ public abstract partial class DangerousActionDialog : PopupDialog
 
         protected DangerousActionDialog()
         {
-            HeaderText = DeleteConfirmationDialogStrings.HeaderText;
+            HeaderText = DialogStrings.Caution;
 
             Icon = FontAwesome.Regular.TrashAlt;
 
             Buttons = new PopupDialogButton[]
             {
                 new PopupDialogDangerousButton
                 {
-                    Text = DeleteConfirmationDialogStrings.Confirm,
+                    Text = DialogStrings.Confirm,
                     Action = () => DangerousAction?.Invoke()
                 },
                 new PopupDialogCancelButton
                 {
-                    Text = DeleteConfirmationDialogStrings.Cancel,
+                    Text = DialogStrings.Cancel,
                     Action = () => CancelAction?.Invoke()
                 }
             };

diff --git a/osu.Game/Screens/Menu/OnlineMenuBanner.cs b/osu.Game/Screens/Menu/OnlineMenuBanner.cs
@@ -73,6 +73,9 @@ private void checkForUpdates()
             Task.Run(() => request.Perform())
                 .ContinueWith(r =>
                 {
+                    if (!FetchOnlineContent)
+                        return;
+
                     if (r.IsCompletedSuccessfully)
                         Schedule(() => Current.Value = request.ResponseObject);
 
@@ -170,6 +173,11 @@ public partial class MenuImage : OsuClickableContainer
 
             private Sprite flash = null!;
 
+            /// <remarks>
+            /// Overridden as a safety for <see cref="openUrlAction"/> functioning correctly.
+            /// </remarks>
+            public override bool IsPresent => base.IsPresent || Scheduler.HasPendingTasks;
+
             private ScheduledDelegate? openUrlAction;
 
             public MenuImage(APIMenuImage image)