Skip to content

Commit 5da7e59

Browse files
tigCopilot
andauthored
Fixes #4456 - Clear MouseGrabView in App.End (#4460)
* Fixed MouseGrabView bug. Added extensive test coverage for `Keyboard`, `Mouse`, `Timeout`, and `Popover` functionalities, including edge cases and concurrent access. Introduced parameterized and data-driven tests to reduce redundancy and improve clarity. Refactored codebase for modularity and maintainability, introducing new namespaces and reorganizing classes. Enhanced `MouseImpl`, `KeyboardImpl`, and `Runnable` implementations with improved event handling, thread safety, and support for the Terminal.Gui Cancellable Work Pattern (CWP). Removed deprecated code and legacy tests, such as `LogarithmicTimeout` and `SmoothAcceleratingTimeout`. Fixed bugs related to mouse grabbing during drag operations and unbalanced `ApplicationImpl.Begin/End` calls. Improved documentation and code readability with modern C# features. * Code cleanup. * Update Tests/UnitTestsParallelizable/Application/Runnable/RunnableIntegrationTests.cs Co-authored-by: Copilot <[email protected]> * Improve null handling and simplify test setup In `MouseImpl.cs`, added an early `return` after the `UngrabMouse()` call within the `if (view is null)` block to prevent further execution when `view` is `null`, improving null reference handling. In `RunnableIntegrationTests.cs`, removed the initialization of the `IApplication` object (`app`) from the `MultipleRunnables_IndependentResults` test method, simplifying the test setup and focusing on runnable behavior. * Code cleanup * API doc link cleanup --------- Co-authored-by: Copilot <[email protected]>
1 parent 0270183 commit 5da7e59

32 files changed

+791
-768
lines changed

Terminal.Gui/App/ApplicationImpl.Run.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,8 @@ public void End (SessionToken token)
368368
previousRunnable.RaiseIsModalChangedEvent (true);
369369
}
370370

371+
Mouse?.UngrabMouse ();
372+
371373
runnable.RaiseIsRunningChangedEvent (false);
372374

373375
token.Result = runnable.Result;

Terminal.Gui/App/Mouse/MouseImpl.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,11 +266,17 @@ public void ResetState ()
266266
/// <inheritdoc/>
267267
public void GrabMouse (View? view)
268268
{
269-
if (view is null || RaiseGrabbingMouseEvent (view))
269+
if (RaiseGrabbingMouseEvent (view))
270270
{
271271
return;
272272
}
273273

274+
if (view is null)
275+
{
276+
UngrabMouse();
277+
return;
278+
}
279+
274280
RaiseGrabbedMouseEvent (view);
275281

276282
// MouseGrabView is only set if the application is initialized.

Terminal.Gui/ViewBase/Adornment/Border.Arrangment.cs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
using System.ComponentModel;
21
using System.Diagnostics;
32

43
namespace Terminal.Gui.ViewBase;
@@ -766,6 +765,17 @@ internal void HandleDragOperation (MouseEventArgs mouseEvent)
766765
}
767766
}
768767

768+
/// <summary>
769+
/// Cancels <see cref="IMouseGrabHandler.GrabbingMouse"/> events during an active drag to prevent other views from
770+
/// stealing the mouse grab mid-operation.
771+
/// </summary>
772+
/// <remarks>
773+
/// During an Arrange Mode drag (<see cref="_dragPosition"/> has a value), Border owns the mouse grab and
774+
/// must receive all mouse events until Button1Released. If another view (e.g., scrollbar, slider) were allowed
775+
/// to grab the mouse, the drag would freeze, leaving Border in an inconsistent state with no cleanup.
776+
/// Canceling follows the CWP pattern, ensuring Border maintains exclusive mouse control until it explicitly
777+
/// releases via <see cref="IMouseGrabHandler.UngrabMouse"/> in <see cref="OnMouseEvent"/>.
778+
/// </remarks>
769779
private void Application_GrabbingMouse (object? sender, GrabMouseEventArgs e)
770780
{
771781
if (App?.Mouse.MouseGrabView == this && _dragPosition.HasValue)
@@ -774,25 +784,14 @@ private void Application_GrabbingMouse (object? sender, GrabMouseEventArgs e)
774784
}
775785
}
776786

777-
private void Application_UnGrabbingMouse (object? sender, GrabMouseEventArgs e)
778-
{
779-
if (App?.Mouse.MouseGrabView == this && _dragPosition.HasValue)
780-
{
781-
e.Cancel = true;
782-
}
783-
}
784-
785787
#endregion Mouse Support
786788

787-
788-
789789
/// <inheritdoc/>
790790
protected override void Dispose (bool disposing)
791791
{
792792
if (App is { })
793793
{
794794
App.Mouse.GrabbingMouse -= Application_GrabbingMouse;
795-
App.Mouse.UnGrabbingMouse -= Application_UnGrabbingMouse;
796795
}
797796

798797
_dragPosition = null;

Terminal.Gui/ViewBase/Adornment/Border.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ public override void BeginInit ()
111111
if (App is { })
112112
{
113113
App.Mouse.GrabbingMouse += Application_GrabbingMouse;
114-
App.Mouse.UnGrabbingMouse += Application_UnGrabbingMouse;
115114
}
116115

117116
if (Parent is null)

Terminal.Gui/ViewBase/Runnable/Runnable.cs renamed to Terminal.Gui/Views/Runnable/Runnable.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
namespace Terminal.Gui.ViewBase;
1+
namespace Terminal.Gui.Views;
22

33
/// <summary>
44
/// Base implementation of <see cref="IRunnable"/> for views that can be run as blocking sessions without returning a result.

Terminal.Gui/ViewBase/Runnable/RunnableTResult.cs renamed to Terminal.Gui/Views/Runnable/RunnableTResult.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
namespace Terminal.Gui.ViewBase;
1+
namespace Terminal.Gui.Views;
22

33
/// <summary>
44
/// Base implementation of <see cref="IRunnable{TResult}"/> for views that can be run as blocking sessions.

Terminal.Gui/ViewBase/Runnable/RunnableWrapper.cs renamed to Terminal.Gui/Views/Runnable/RunnableWrapper.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
namespace Terminal.Gui.ViewBase;
1+
namespace Terminal.Gui.Views;
22

33
/// <summary>
44
/// Wraps any <see cref="View"/> to make it runnable with a typed result, similar to how

Terminal.Gui/ViewBase/Runnable/ViewRunnableExtensions.cs renamed to Terminal.Gui/Views/Runnable/ViewRunnableExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
namespace Terminal.Gui.ViewBase;
1+
namespace Terminal.Gui.Views;
22

33
/// <summary>
44
/// Extension methods for making any <see cref="View"/> runnable with typed results.

Tests/UnitTests/View/Mouse/MouseTests.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,10 @@ public void ButtonPressed_In_Border_Starts_Drag (int marginThickness, int border
4949

5050
Application.RaiseMouseEvent (new () { ScreenPosition = new (xy + 1, xy + 1), Flags = MouseFlags.Button1Pressed | MouseFlags.ReportMousePosition });
5151
AutoInitShutdownAttribute.RunIteration ();
52-
5352
Assert.Equal (expectedMoved, new Point (5, 5) == testView.Frame.Location);
53+
// The above grabbed the mouse. Need to ungrab.
54+
Application.Mouse.UngrabMouse ();
55+
5456
top.Dispose ();
5557
}
5658

Tests/UnitTests/View/ViewCommandTests.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,9 @@ public void Button_CanFocus_False_Raises_Accepted_Correctly ()
152152
Assert.Equal (1, btnAcceptedCount);
153153
Assert.Equal (0, wAcceptedCount);
154154

155+
// The above grabbed the mouse. Need to ungrab.
156+
Application.Mouse.UngrabMouse ();
157+
155158
w.Dispose ();
156159
Application.ResetState (true);
157160
}

0 commit comments

Comments
 (0)