Merge pull request ppy#6345 from smoogipoo/fix-focus-change-during-click

Do not unfocus on click if the focused drawable changed during the click

osu.Framework.Tests/Visual/Drawables/TestSceneFocus.cs

@@ -10,6 +10,7 @@
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Graphics.Sprites;
+using osu.Framework.Graphics.UserInterface;
 using osu.Framework.Input;
 using osu.Framework.Input.Events;
 using osu.Framework.Testing;
@@ -286,6 +287,48 @@ public void TestDrawableWithNoFocusChangeOnClick()
             AddAssert("no focus change box received click", () => noFocusChangeBox.ClickCount, () => Is.GreaterThan(0));
         }
 
+        [Test]
+        public void TestChangeFocusDuringInputHandling_ShouldRetainFocus()
+        {
+            BasicButton button = null!;
+            FocusBox box = null!;
+
+            AddStep("setup", () =>
+            {
+                FocusBox b = new FocusBox
+                {
+                    Position = new Vector2(0, 75)
+                };
+
+                Children =
+                [
+                    button = new BasicButton
+                    {
+                        Size = new Vector2(150, 50),
+                        Text = "Focus the box",
+                        Action = () => b.GetContainingFocusManager()!.ChangeFocus(b)
+                    },
+                    box = b
+                ];
+            });
+
+            AddStep("click button", () =>
+            {
+                InputManager.MoveMouseTo(button);
+                InputManager.Click(MouseButton.Left);
+            });
+
+            AddAssert("box is focused", () => box.HasFocus, () => Is.True);
+
+            AddStep("click button again", () =>
+            {
+                InputManager.MoveMouseTo(button);
+                InputManager.Click(MouseButton.Left);
+            });
+
+            AddAssert("box is still focused", () => box.HasFocus, () => Is.True);
+        }
+
         private void checkFocused(Func<Drawable> d) => AddAssert("check focus", () => d().HasFocus);
         private void checkNotFocused(Func<Drawable> d) => AddAssert("check not focus", () => !d().HasFocus);
 

osu.Framework/Graphics/Drawable.cs

@@ -1491,15 +1491,15 @@ protected internal virtual bool ShouldBeAlive
         /// </summary>
         /// <returns>The first parent <see cref="InputManager"/>.</returns>
         [CanBeNull]
-        protected InputManager GetContainingInputManager() => this.FindClosestParent<InputManager>();
+        protected internal InputManager GetContainingInputManager() => this.FindClosestParent<InputManager>();
 
         /// <summary>
         /// Retrieve the first parent in the tree which implements <see cref="IFocusManager"/>.
         /// As this is performing an upward tree traversal, avoid calling every frame.
         /// </summary>
         /// <returns>The first parent <see cref="IFocusManager"/>.</returns>
         [CanBeNull]
-        protected IFocusManager GetContainingFocusManager() => this.FindClosestParent<IFocusManager>();
+        protected internal IFocusManager GetContainingFocusManager() => this.FindClosestParent<IFocusManager>();
 
         private CompositeDrawable parent;
 

osu.Framework/Input/InputManager.cs

@@ -51,6 +51,12 @@ public abstract partial class InputManager : Container, IInputStateChangeHandler
         /// </summary>
         public Drawable FocusedDrawable { get; internal set; }
 
+        /// <summary>
+        /// Any drawable that was focused directly via <see cref="ChangeFocus(Drawable, InputState)"/> during the handling of a click,
+        /// and <i>not</i> as a result of the automatic post-process change of focus from the click.
+        /// </summary>
+        internal Drawable FocusedDrawableThisClick;
+
         protected abstract ImmutableArray<InputHandler> InputHandlers { get; }
 
         private double keyboardRepeatTime;
@@ -400,7 +406,10 @@ public void TriggerFocusContention(Drawable triggerSource)
         protected bool ChangeFocus(Drawable potentialFocusTarget, InputState state)
         {
             if (potentialFocusTarget == FocusedDrawable)
+            {
+                FocusedDrawableThisClick = FocusedDrawable;
                 return true;
+            }
 
             if (potentialFocusTarget != null && (!isDrawableValidForFocus(potentialFocusTarget) || !potentialFocusTarget.AcceptsFocus))
                 return false;
@@ -427,6 +436,8 @@ protected bool ChangeFocus(Drawable potentialFocusTarget, InputState state)
                 FocusedDrawable.TriggerEvent(new FocusEvent(state, previousFocus));
             }
 
+            FocusedDrawableThisClick = FocusedDrawable;
+
             return true;
         }
 

osu.Framework/Input/MouseButtonEventManager.cs

@@ -154,12 +154,19 @@ private void handleClick(InputState state, List<Drawable>? targets)
             var drawables = targets.Intersect(InputQueue)
                                    .Where(t => t.IsAlive && t.IsPresent && t.ReceivePositionalInputAt(state.Mouse.Position));
 
-            Drawable? clicked = PropagateButtonEvent(drawables, new ClickEvent(state, Button, MouseDownPosition));
+            InputManager.FocusedDrawableThisClick = null;
 
+            Drawable? clicked = PropagateButtonEvent(drawables, new ClickEvent(state, Button, MouseDownPosition));
             ClickedDrawable.SetTarget(clicked!);
 
-            if (ChangeFocusOnClick && clicked?.ChangeFocusOnClick != false)
-                InputManager.ChangeFocusFromClick(clicked);
+            // Focus shall only change if it wasn't explicitly changed during the click (for example, using a button to open a menu).
+            if (InputManager.FocusedDrawableThisClick == null)
+            {
+                if (ChangeFocusOnClick && clicked?.ChangeFocusOnClick != false)
+                    InputManager.ChangeFocusFromClick(clicked);
+            }
+
+            InputManager.FocusedDrawableThisClick = null;
 
             if (clicked != null)
                 Logger.Log($"MouseClick handled by {clicked}.", LoggingTarget.Runtime, LogLevel.Debug);