Merge pull request ppy#31442 from peppy/supporter-button-always-warns

Always show dialog when clicking supporter icon before opening browser
cdwcgt · Jan 7, 2025 · d0a39a7 · d0a39a7
2 parents 5431a08 + b8a10d9
commit d0a39a7
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 35 deletions.
diff --git a/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs b/osu.Game.Tests/Visual/SongSelect/TestSceneBeatmapRecommendations.cs
@@ -12,7 +12,6 @@
 using osu.Framework.Testing;
 using osu.Game.Beatmaps;
 using osu.Game.Database;
-using osu.Game.Extensions;
 using osu.Game.Graphics.Sprites;
 using osu.Game.Online.API;
 using osu.Game.Online.API.Requests;
@@ -85,6 +84,7 @@ decimal getNecessaryPP(int? rulesetID)
         }
 
         [Test]
+        [FlakyTest]
         public void TestPresentedBeatmapIsRecommended()
         {
             List<BeatmapSetInfo> beatmapSets = null;
@@ -106,6 +106,7 @@ public void TestPresentedBeatmapIsRecommended()
         }
 
         [Test]
+        [FlakyTest]
         public void TestCurrentRulesetIsRecommended()
         {
             BeatmapSetInfo catchSet = null, mixedSet = null;
@@ -142,6 +143,7 @@ public void TestBestRulesetIsRecommended()
         }
 
         [Test]
+        [FlakyTest]
         public void TestSecondBestRulesetIsRecommended()
         {
             BeatmapSetInfo osuSet = null, mixedSet = null;
@@ -159,6 +161,7 @@ public void TestSecondBestRulesetIsRecommended()
         }
 
         [Test]
+        [FlakyTest]
         public void TestCorrectStarRatingIsUsed()
         {
             BeatmapSetInfo osuSet = null, maniaSet = null;
@@ -176,6 +179,7 @@ public void TestCorrectStarRatingIsUsed()
         }
 
         [Test]
+        [FlakyTest]
         public void TestBeatmapListingFilter()
         {
             AddStep("set playmode to taiko", () => ((DummyAPIAccess)API).LocalUser.Value.PlayMode = "taiko");
@@ -245,7 +249,7 @@ private void presentAndConfirm(Func<BeatmapSetInfo> getImport, int expectedDiff)
             AddStep("present beatmap", () => Game.PresentBeatmap(getImport()));
 
             AddUntilStep("wait for song select", () => Game.ScreenStack.CurrentScreen is Screens.Select.SongSelect select && select.BeatmapSetsLoaded);
-            AddUntilStep("recommended beatmap displayed", () => Game.Beatmap.Value.BeatmapInfo.MatchesOnlineID(getImport().Beatmaps[expectedDiff - 1]));
+            AddUntilStep("recommended beatmap displayed", () => Game.Beatmap.Value.BeatmapInfo.OnlineID, () => Is.EqualTo(getImport().Beatmaps[expectedDiff - 1].OnlineID));
         }
 
         protected override TestOsuGame CreateTestGame() => new NoBeatmapUpdateGame(LocalStorage, API);

diff --git a/osu.Game/Online/Chat/ExternalLinkOpener.cs b/osu.Game/Online/Chat/ExternalLinkOpener.cs
@@ -4,13 +4,16 @@
 using System;
 using osu.Framework.Allocation;
 using osu.Framework.Bindables;
+using osu.Framework.Extensions;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Sprites;
 using osu.Framework.Platform;
 using osu.Game.Configuration;
 using osu.Game.Localisation;
+using osu.Game.Online.API;
 using osu.Game.Overlays;
 using osu.Game.Overlays.Dialog;
+using osu.Game.Overlays.Notifications;
 using WebCommonStrings = osu.Game.Resources.Localisation.Web.CommonStrings;
 
 namespace osu.Game.Online.Chat
@@ -23,9 +26,15 @@ public partial class ExternalLinkOpener : Component
         [Resolved]
         private Clipboard clipboard { get; set; } = null!;
 
-        [Resolved(CanBeNull = true)]
+        [Resolved]
         private IDialogOverlay? dialogOverlay { get; set; }
 
+        [Resolved]
+        private INotificationOverlay? notificationOverlay { get; set; }
+
+        [Resolved]
+        private IAPIProvider api { get; set; } = null!;
+
         private Bindable<bool> externalLinkWarning = null!;
 
         [BackgroundDependencyLoader(true)]
@@ -34,9 +43,51 @@ private void load(OsuConfigManager config)
             externalLinkWarning = config.GetBindable<bool>(OsuSetting.ExternalLinkWarning);
         }
 
-        public void OpenUrlExternally(string url, bool bypassWarning = false)
+        public void OpenUrlExternally(string url, LinkWarnMode warnMode = LinkWarnMode.Default)
         {
-            if (!bypassWarning && externalLinkWarning.Value && dialogOverlay != null)
+            bool isTrustedDomain;
+
+            if (url.StartsWith('/'))
+            {
+                url = $"{api.WebsiteRootUrl}{url}";
+                isTrustedDomain = true;
+            }
+            else
+            {
+                isTrustedDomain = url.StartsWith(api.WebsiteRootUrl, StringComparison.Ordinal);
+            }
+
+            if (!url.CheckIsValidUrl())
+            {
+                notificationOverlay?.Post(new SimpleErrorNotification
+                {
+                    Text = NotificationsStrings.UnsupportedOrDangerousUrlProtocol(url),
+                });
+
+                return;
+            }
+
+            bool shouldWarn;
+
+            switch (warnMode)
+            {
+                case LinkWarnMode.Default:
+                    shouldWarn = externalLinkWarning.Value && !isTrustedDomain;
+                    break;
+
+                case LinkWarnMode.AlwaysWarn:
+                    shouldWarn = true;
+                    break;
+
+                case LinkWarnMode.NeverWarn:
+                    shouldWarn = false;
+                    break;
+
+                default:
+                    throw new ArgumentOutOfRangeException(nameof(warnMode), warnMode, null);
+            }
+
+            if (dialogOverlay != null && shouldWarn)
                 dialogOverlay.Push(new ExternalLinkDialog(url, () => host.OpenUrlExternally(url), () => clipboard.SetText(url)));
             else
                 host.OpenUrlExternally(url);

diff --git a/osu.Game/Online/Chat/LinkWarnMode.cs b/osu.Game/Online/Chat/LinkWarnMode.cs
@@ -0,0 +1,23 @@
+// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+namespace osu.Game.Online.Chat
+{
+    public enum LinkWarnMode
+    {
+        /// <summary>
+        /// Will show a dialog when opening a URL that is not on a trusted domain.
+        /// </summary>
+        Default,
+
+        /// <summary>
+        /// Will always show a dialog when opening a URL.
+        /// </summary>
+        AlwaysWarn,
+
+        /// <summary>
+        /// Will never show a dialog when opening a URL.
+        /// </summary>
+        NeverWarn,
+    }
+}
diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs
@@ -18,7 +18,6 @@
 using osu.Framework.Audio;
 using osu.Framework.Bindables;
 using osu.Framework.Configuration;
-using osu.Framework.Extensions;
 using osu.Framework.Extensions.IEnumerableExtensions;
 using osu.Framework.Extensions.TypeExtensions;
 using osu.Framework.Graphics;
@@ -516,32 +515,7 @@ public void CopyUrlToClipboard(string url) => waitForReady(() => onScreenDisplay
             onScreenDisplay.Display(new CopyUrlToast());
         });
 
-        public void OpenUrlExternally(string url, bool forceBypassExternalUrlWarning = false) => waitForReady(() => externalLinkOpener, _ =>
-        {
-            bool isTrustedDomain;
-
-            if (url.StartsWith('/'))
-            {
-                url = $"{API.WebsiteRootUrl}{url}";
-                isTrustedDomain = true;
-            }
-            else
-            {
-                isTrustedDomain = url.StartsWith(API.WebsiteRootUrl, StringComparison.Ordinal);
-            }
-
-            if (!url.CheckIsValidUrl())
-            {
-                Notifications.Post(new SimpleErrorNotification
-                {
-                    Text = NotificationsStrings.UnsupportedOrDangerousUrlProtocol(url),
-                });
-
-                return;
-            }
-
-            externalLinkOpener.OpenUrlExternally(url, forceBypassExternalUrlWarning || isTrustedDomain);
-        });
+        public void OpenUrlExternally(string url, LinkWarnMode warnMode = LinkWarnMode.Default) => waitForReady(() => externalLinkOpener, _ => externalLinkOpener.OpenUrlExternally(url, warnMode));
 
         /// <summary>
         /// Open a specific channel in chat.
@@ -1340,7 +1314,7 @@ private void forwardTabletLogsToNotifications()
                         IconColour = Colours.YellowDark,
                         Activated = () =>
                         {
-                            OpenUrlExternally("https://opentabletdriver.net/Tablets", true);
+                            OpenUrlExternally("https://opentabletdriver.net/Tablets", LinkWarnMode.NeverWarn);
                             return true;
                         }
                     }));

diff --git a/osu.Game/Overlays/AccountCreation/ScreenEntry.cs b/osu.Game/Overlays/AccountCreation/ScreenEntry.cs
@@ -18,6 +18,7 @@
 using osu.Game.Graphics.UserInterface;
 using osu.Game.Localisation;
 using osu.Game.Online.API;
+using osu.Game.Online.Chat;
 using osu.Game.Overlays.Settings;
 using osu.Game.Resources.Localisation.Web;
 using osuTK;
@@ -213,7 +214,7 @@ private void performRegistration()
                                 if (!string.IsNullOrEmpty(errors.Message))
                                     passwordDescription.AddErrors(new[] { errors.Message });
 
-                                game?.OpenUrlExternally($"{errors.Redirect}?username={usernameTextBox.Text}&email={emailTextBox.Text}", true);
+                                game?.OpenUrlExternally($"{errors.Redirect}?username={usernameTextBox.Text}&email={emailTextBox.Text}", LinkWarnMode.NeverWarn);
                             }
                         }
                         else

diff --git a/osu.Game/Overlays/Profile/Header/Components/SupporterIcon.cs b/osu.Game/Overlays/Profile/Header/Components/SupporterIcon.cs
@@ -11,6 +11,7 @@
 using osu.Framework.Localisation;
 using osu.Game.Graphics;
 using osu.Game.Graphics.Containers;
+using osu.Game.Online.Chat;
 using osu.Game.Resources.Localisation.Web;
 
 namespace osu.Game.Overlays.Profile.Header.Components
@@ -87,7 +88,8 @@ private void load(OsuGame? game)
         {
             background.Colour = colours.Pink;
 
-            Action = () => game?.OpenUrlExternally(@"/home/support");
+            // Easy to accidentally click so let's always show the open URL popup.
+            Action = () => game?.OpenUrlExternally(@"/home/support", LinkWarnMode.AlwaysWarn);
         }
 
         protected override bool OnHover(HoverEvent e)