KeyboardAccelerator event fire multiple times

[oden3d]

Describe the bug

Control (a Button for instance) fire multiple events (as long as hold buttons pressed) with KeyboardAccelerator attached.

Steps to reproduce the bug

1. Create simple WinUI app from template;
2. Place button with KeyboardAccelerator attached;

<Button Content="Click Me" x:Name="myButton" Click="myButton_Click">
    <Button.KeyboardAccelerators>
        <KeyboardAccelerator Key="A" Invoked="KeyboardAccelerator_Invoked"/>
    </Button.KeyboardAccelerators>
</Button>

private void myButton_Click(object sender, RoutedEventArgs e)
{
    Debug.WriteLine("Clicked");
}

private void KeyboardAccelerator_Invoked(KeyboardAccelerator sender, KeyboardAcceleratorInvokedEventArgs args)
{
    Debug.WriteLine("Invoked");

    //args.Handled = true;
}

3. Run app then press and hold 'A'. 'Clicked' and 'Invoked' will bubble as long as hold 'A' button;
4. Try 'args.Handled = true;'.
5. 'Invoked' will bubble as long as hold 'A' button but 'Clicked' newer called;
6. The same issue with ICommand;

KeyboardAccelerator is complete broken and useless!

Expected behavior

Button click event should fire once.

Screenshots

No response

NuGet package version

WinUI 3 - Windows App SDK 1.5.5: 1.5.240627000

Windows version

Windows 11 (22H2): Build 22621, Windows 11 (21H2): Build 22000, Windows 10 (21H2): Build 19044

Additional context

No response

[github-actions]

Hi I'm an AI powered bot that finds similar issues based off the issue title.

Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one. Thank you!

Closed similar issues:

• MenuFlyoutItem.KeyboardAccelerators fires Invoked event twice (#6488), similarity score: 0.82

Note: You can give me feedback by thumbs upping or thumbs downing this comment.

[JJBrychell]

This appears to be functioning as designed, in fact, by specifying handled=true on the invoked event, you are explicitly telling it not to raise the click event.

Let's take a quick look at how the key is processed. Starting with just a basic scene:

<StackPanel Orientation="Vertical">
<Button>Click Me</Button>
</StackPanel>

When you press the character key (e.g. 'a') a series of events get fired to multiple objects: In this case:

• StackPanel Preview Key Down
• Button Preview Key Down
• Button Key Down
• StackPanel Key Down
• Button Character Received
• StackPanel Character Received

And then when you release it:

• StackPanel Preview Key Up
• Button Preview Key Up
• Button Key Up
• StackPanel Key Up

At any point in these two sequences, the event can be marked as handled and the remaining events in that sequence are not raised. For example, if I mark the event as handled in Button Preview key Down and in Button PreviewKey up, the sequence for pressing and release a character key would be:

• StackPanel Preview Key Down
• Button Preview Key Down

and

• StackPanel Preview Key Up
• Button Preview Key Up

Notice the sequences just stopped once I said it was handled.

Now, when you add the accelerator to the button, there is one more step that is taken right after the Preview Key events have been raised. If the key matches an accelerator, it will then:

1. Raises the Invoked event on the accelerator to see if it wants to do some work.
2. And if the event is not handled in step 1, it will check for a default action to take. For a button, this is to raise the Click event AND mark the event as handled. So with the accelerator in place when you press the key and release it you get (nothing being marked handled in application code):

• StackPanel Preview Key Down
• Button Preview Key Down
• Invoked
• Clicked

and

• StackPanel Preview Key Up
• Button Preview Key Up
• Button Key Up
• StackPanel Key Up

Based on this, you can see that if you were to mark the event has handled in Invoked, then we would not attempt to take the default action of raising the Click event.

The final piece to this is the repeating aspect when holding down the key. You mentioned the term bubbling in your description, but I think you have the confused bubbling with repeating. Bubbling is the fact that when a key is pressed with focus on the button that when events are fired, they can be bubbled to the parent (e.g. the stack panel). Repeating is autogeneration of input notifications to the UI framework from the OS. This keyboard repeating is controlled outside of the UI framework, and the autogenerated input is passed into the UI Framework as if the user had generated. So in this scenario, what happens is that the UI framework is notified repeatedly about the key down until the key is released which we will then process as a normal key up. This causes the initial keydown sequence of events to be repeated, but with all the same rules as if the key had actually been pressed multiple times. So in our accelerator case, if you press and hold the accelerator key we would get:

• StackPanel Preview Key Down
• Button Preview Key Down
• Invoked
• Clicked
• StackPanel Preview Key Down
• Button Preview Key Down
• Invoked
• Clicked
• StackPanel Preview Key Down
• Button Preview Key Down
• Invoked
  ...
  until finally:
• StackPanel Preview Key Up
• Button Preview Key Up
• Button Key Up
• StackPanel Key Up

And if you mark the event as handled in the Invoked event, the Click events will not be fired.

Hopefully this gives you a bit more insight into how the key event and accelerator processing works. I was unable to decern from the comments exactly what you are attempting to do and/or where this pattern doesn't work for you, but maybe with this additional information you can make it work or if there is some additional feature that you need in this area you can post it as a request.

For now, I am going to close this issue as by design since the symptoms described appear to match the designed behavior.

[oden3d]

Thank you for the detailed answer!

I expect that event will fire only once.
For example when user press Ctrl-Z we need to run only one step back.

At present got multiple events.

[oden3d]

I believe one event is the by design behavior, not multiple.

[JJBrychell]

So a typical ctrl-Z (press and release) should generate one set of sequences and thus if you do your work in the invoked handler and mark it as handled, then it should only execute once.

If what you are trying to is to prevent the repeating if the user holds the key down, that is not a scenario that we have considered with an accelerator. Typically, this kind of thing is done within a control, where you can listen for the key down event or character received and then check the KeyRoutedEventArgs or CharacterReceivedEventArgs KeyStatus::RepeatCount and only perform an action when the value is 1. There is nothing to prevent you from "rolling your own accelerator", by listening for these events at the root of your scene, but be careful of conflicts (e.g. you don't want an accelerator 'a' to be fired when attempting to type in a text box.

I don't know what you mean by "one event is the by design behavior, not multiple". The design behavior for an accelerator is invoke the defined action (either via the invoked event or default action) once, for each key press of the accelerator. The OS/hardware is telling Xaml that multiple key presses occurred which we handle. Certainly, we could add a feature to tell accelerators not to do this, by looking at the key status, but that would require an api change and I just don't have a good feel for how useful it would really be. But if this is something that you think is needed, feel free to submit a feature request and see if the community has any interest in it. One of the factors in how we prioritize work is how active the feature/bug is within the community.

[oden3d]

So a typical ctrl-Z (press and release) should generate one set of sequences and thus if you do your work in the invoked handler and mark it as handled, then it should only execute once.

Mark as handled is not working and keep events going till key release. This was described in topic start message.

        <Button x:Name="myButton" Click="myButton_Click" Content="Click Me">
            <Button.KeyboardAccelerators>
                <KeyboardAccelerator Key="A" Invoked="KeyboardAccelerator_Invoked"/>
            </Button.KeyboardAccelerators>
        </Button>

        private void KeyboardAccelerator_Invoked(KeyboardAccelerator sender, KeyboardAcceleratorInvokedEventArgs args)
        {
            System.Diagnostics.Debug.WriteLine("Invoked");

            args.Handled = true;
        }

Output:

Invoked
Invoked
Invoked
Invoked
.......

[oden3d]

Also documentation describe accelerators as keyboard shortcuts.
https://learn.microsoft.com/en-us/windows/apps/design/input/keyboard-accelerators

Accelerator keys (or keyboard accelerators) are keyboard shortcuts that improve the usability and accessibility of your Windows applications by providing an intuitive way for users to invoke common actions or commands without navigating the app UI.

[oden3d]

So a typical ctrl-Z (press and release) should generate one set of sequences

My grand farther does not agree with that as he hold keys a bit longer and got 2-3 undo events.

[JJBrychell]

Yes, I understand that. But unless there is something else going on you aren't pressing and releasing the ctrl-z, you are holding it down. That invokes the system hardware behavior of repeatedly sending key down messages. The UI framework is just responding to them in the designed fashion. In fact, if you read further down in the keyboard accelerator page link you posted you will find:

"An accelerator auto-repeats (for example, when the user presses Ctrl+Shift and then holds down M, the accelerator is invoked repeatedly until M is released). This behavior cannot be modified."

That being said, I understand that some people have different needs when it comes to typing. I presume that your grandfather also has issues with other keys as well. There is a way to slow down the repeat effect produced by the OS.

Depending on your OS version and keyboard dirver your milage may differ, but you can search for "Change keyboard repeat wait windows" and find a number of articles. For example, on my machine.

1. Press win+r
2. Type 'control keyboard'
3. select the Speed tab

this gets me:

Where I can control how long of a delay before the repeat starts and how fast the repeat is.

There is also supposed to be a way to turn it off completly via accessibility, but I haven't been able to track that down.

[oden3d]

So a typical ctrl-Z (press and release) should generate one set of sequences and thus if you do your work in the invoked handler and mark it as handled, then it should only execute once.

Mark as handled is not working and keep events going till key release. This was described in topic start message.

        <Button x:Name="myButton" Click="myButton_Click" Content="Click Me">
            <Button.KeyboardAccelerators>
                <KeyboardAccelerator Key="A" Invoked="KeyboardAccelerator_Invoked"/>
            </Button.KeyboardAccelerators>
        </Button>

        private void KeyboardAccelerator_Invoked(KeyboardAccelerator sender, KeyboardAcceleratorInvokedEventArgs args)
        {
            System.Diagnostics.Debug.WriteLine("Invoked");

            args.Handled = true;
        }

Output:

Invoked
Invoked
Invoked
Invoked
.......

Also documentation describe accelerators as keyboard shortcuts.
https://learn.microsoft.com/en-us/windows/apps/design/input/keyboard-accelerators

Accelerator keys (or keyboard accelerators) are keyboard shortcuts that improve the usability and accessibility of your Windows applications by providing an intuitive way for users to invoke common actions or commands without navigating the app UI.

[oden3d]

Is any way/workaround for shortcuts handle only one event?

[JJBrychell]

Again marking as handled stops the current routing of the key input in the current sequence, it DOES NOT stop the OS from generating repeating key input. Every time the OS tells the UI Framework that a key was pressed (whether because it was manually pressed or via auto repeat), the UI framework will process it and if it matches an accelerator it will take that action. Hence, if you hold the key down long enough for the OS's autorepeat to kick in you will get multiple events, because the application is getting multiple inputs.

Again from the documentation link you provided (emphasis mine):

"An accelerator auto-repeats (for example, when the user presses Ctrl+Shift and then holds down M, the accelerator is invoked repeatedly until M is released). This behavior cannot be modified."

So, no, there is no official way to prevent this from happening. However, as mentioned previously, you could not use accelerators and roll your own by handling the key down and key up events, in which case you can determine whether the key input is generated due to direct user interaction or due to repeating.

Another hack, might be to set a flag in the invoked handler on the first call to it and then ignore any subsequent calls until the scene sees a keyup (I would probably use PreviewKeyUp on the top level element in the scene) at which time you would reset the flag so the next key down would cause the invoked event to execute the application code again.

[oden3d]

Ok, thank you for your help!