`View.Mouse` raises `MouseClick` 3 times when it should just raise it once

[BDisp]

There is one situation in Terminal.Gui which make me crazy. A simple Accepting event which will allow the developer to Cancel the default behavior (but has the Handled property) will repeat the event 3 times by mouse if the developer doesn't Cancel it. UICatalog has a bunch of Button's that if they are activated by mouse will repeat the event 3 times, no matter the user press Key.Enter or Key.Esc. Despite by adding Handled = true fix this, there is no necessity of this extra code for a normal Accepting event. We can't handle mouse event as key event because only the only the view where the mouse is will be executed and doesn't not propagate to other views and with key event it can be propagate to other views that may want to handle it. I agree the leverage of keybindings, commands and context to achieve this behavior but we need to handle the mouse with much more careful.

Originally posted by @BDisp in #4126 (comment)

I understand that the current implementation is correct to handle adornments and all the views under the mouse but the Button must be responsible to set e.Handled = true; after the MouseClickevent. to prevent further processing to itself.

[BDisp]

Also fix this.

with this code:

public class ExampleWindow : Window
{
    public static string UserName { get; set; }

    public ExampleWindow ()
    {
        Title = $"Example App ({Application.QuitKey} to quit)";

        // Create input components and labels
        var usernameLabel = new Label { Text = "Username:" };

        var userNameText = new TextField
        {
            // Position text field adjacent to the label
            X = Pos.Right (usernameLabel) + 1,

            // Fill remaining horizontal space
            Width = Dim.Fill ()
        };

        var passwordLabel = new Label
        {
            Text = "Password:", X = Pos.Left (usernameLabel), Y = Pos.Bottom (usernameLabel) + 1
        };

        var passwordText = new TextField
        {
            Secret = true,

            // align with the text box above
            X = Pos.Left (userNameText),
            Y = Pos.Top (passwordLabel),
            Width = Dim.Fill ()
        };

        // Create login button
        var btnLogin = new Button
        {
            Text = "Login",
            Y = Pos.Bottom (passwordLabel) + 1,

            // center the login button horizontally
            X = Pos.Center (),
            IsDefault = true
        };

        // When login button is clicked display a message popup
        btnLogin.Accepting += (s, e) =>
                           {
                               if (userNameText.Text == "admin" && passwordText.Text == "password")
                               {
                                   MessageBox.Query ("Logging In", "Login Successful", "Ok");
                                   UserName = userNameText.Text;
                                   Application.RequestStop ();
                               }
                               else
                               {
                                   MessageBox.ErrorQuery ("Logging In", "Incorrect username or password", "Ok");
                               }
                           };

        // Create StatusBar
        StatusBar statusBar = new ()
        {
            Visible = true,
            AlignmentModes = AlignmentModes.IgnoreFirstOrLast,
            CanFocus = false
        };

        Shortcut shortcut = new ()
        {
            Title = "Shortcut",
            CanFocus = false
        };
        statusBar.Add (shortcut);

        // Add the views to the Window
        Add (usernameLabel, userNameText, passwordLabel, passwordText, btnLogin, statusBar);
    }

[tig]

I believe the fix to the MouseClick being raised 3 times is this, in HandleMouseGrab()

-            if (MouseGrabView?.NewMouseEvent (viewRelativeMouseEvent) is true)
-            {
-                return true;
-            }
+            if (MouseGrabView?.NewMouseEvent (viewRelativeMouseEvent) is true || viewRelativeMouseEvent.IsSingleDoubleOrTripleClicked)
+            {
+               // If the view that grabbed the mouse handled the event OR it was a click we are done.
+               return true;
+            }

The other issue, where any other peer view (in this case a StatusBar) of an IsDefault button that raises Accept causing the button to Accept is actually BY DESIGN. This is literally what Button.IsDefault means.

That said, I think the design of IsDefault is confusing.

[tznind]

(move discussion)

[tig]

This test reproduces:

 [Theory]
 [InlineData (MouseState.None)]
 [InlineData (MouseState.In)]
 [InlineData (MouseState.Pressed)]
 [InlineData (MouseState.PressedOutside)]
 public void RaiseMouseEvent_ButtonClicked_Raises_MouseClick_Once (MouseState states)
 {
     Application.Init (new FakeDriver ());

     Application.Top = new Toplevel ()
     {
         Id = "top",
         Height = 10,
         Width = 10
     };

     var view = new View
     {
         Width = 1,
         Height = 1,
         WantContinuousButtonPressed = false,
         HighlightStates = states
     };
     Application.Top.Add (view);
     Application.LayoutAndDraw ();

     var clickedCount = 0;
     view.MouseClick += (s, e) => clickedCount++;

     var me = new MouseEventArgs ();
     Application.RaiseMouseEvent (new MouseEventArgs () { Flags = MouseFlags.Button1Pressed, });
     Application.RaiseMouseEvent (new MouseEventArgs () { Flags = MouseFlags.Button1Released, });
     Application.RaiseMouseEvent (new MouseEventArgs () { Flags = MouseFlags.Button1Clicked, });

     Application.Top.Dispose ();
     Application.ResetState (true);

     Assert.Equal (1, clickedCount);
 }

Fails for all except MouseState.None (clickedCount == 3).

[BDisp]

I think this is more correct by using MouseBindings and Accepting event.

    [Theory]
    [InlineData (MouseState.None)]
    [InlineData (MouseState.In)]
    [InlineData (MouseState.Pressed)]
    [InlineData (MouseState.PressedOutside)]
    public void RaiseMouseEvent_ButtonClicked_Raises_MouseClick_Once (MouseState states)
    {
        Application.Init (new FakeDriver ());

        Application.Top = new Toplevel ()
        {
            Id = "top",
            Height = 10,
            Width = 10
        };

        var view = new View
        {
            Width = 1,
            Height = 1,
            WantContinuousButtonPressed = false,
            HighlightStates = states
        };

        view.MouseBindings.ReplaceCommands (MouseFlags.Button1Clicked, Command.Accept);

        Application.Top.Add (view);
        Application.LayoutAndDraw ();

        var clickedCount = 0;
        view.Accepting += (s, e) => clickedCount++;

        var me = new MouseEventArgs ();
        Application.RaiseMouseEvent (new MouseEventArgs () { Flags = MouseFlags.Button1Pressed, });
        Application.RaiseMouseEvent (new MouseEventArgs () { Flags = MouseFlags.Button1Released, });
        Application.RaiseMouseEvent (new MouseEventArgs () { Flags = MouseFlags.Button1Clicked, });

        Application.Top.Dispose ();
        Application.ResetState (true);

        Assert.Equal (1, clickedCount);
    }

[tig]

I think this is more correct by using MouseBindings and Accepting event.

Not really. The bug is in the MouseClick code. It is independent of how Accept works. Sure, your version shows there's a bug, and passes when the bug is fixed, but it is operating at a higher-level.

Note that there's actually TWO bugs:

1. HandleMouseGrab as described above

Fixing that reduces the # of times MouseClick is raised by one to 2.

2. This code in View.WhenGrabbedHandleClicked():

- // If mouse is still in bounds, generate a click
+ // If mouse is still in bounds, return false to indicate a click should be raised.
if (!WantMousePositionReports && Viewport.Contains (mouseEvent.Position))
{
-     return RaiseMouseClickEvent (mouseEvent);
+         return false;
}

By changing both of these, my new test, as well as yours pass.

There's yet another bug in here that I don't yet have a fix for, but leads to confusion:

The reason Button needs to subscribe to MouseClick instead of registering a MouseBinding is InvokeCommandsBoundToMouse is never called in the WantContinuousButtonPressed code path. So if WantContinuousButtonPressed is true (e.g. for NumericUpDown) using MouseBindings doesn't work.