Merge pull request #5055 from Fusion86/fix-keybindings-foreach-crash

Fix crash when KeyBindings change while they are being handled
AvaloniaUI · Jun 14, 2021 · e2a5bef · e2a5bef
2 parents 3b8e89f + 2d10536
commit e2a5bef
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 3 deletions.
diff --git a/src/Avalonia.Input/KeyboardDevice.cs b/src/Avalonia.Input/KeyboardDevice.cs
@@ -217,12 +217,31 @@ public void ProcessRawEvent(RawInputEventArgs e)
                         {
                             var bindings = (currentHandler as IInputElement)?.KeyBindings;
                             if (bindings != null)
+                            {
+                                KeyBinding[]? bindingsCopy = null;
+
+                                // Create a copy of the KeyBindings list if there's a binding which matches the event.
+                                // If we don't do this the foreach loop will throw an InvalidOperationException when the KeyBindings list is changed.
+                                // This can happen when a new view is loaded which adds its own KeyBindings to the handler.
                                 foreach (var binding in bindings)
                                 {
-                                    if (ev.Handled)
+                                    if (binding.Gesture?.Matches(ev) == true)
+                                    {
+                                        bindingsCopy = bindings.ToArray();
                                         break;
-                                    binding.TryHandle(ev);
+                                    }
+                                }
+
+                                if (bindingsCopy is object)
+                                {
+                                    foreach (var binding in bindingsCopy)
+                                    {
+                                        if (ev.Handled)
+                                            break;
+                                        binding.TryHandle(ev);
+                                    }
                                 }
+                            }
                             currentHandler = currentHandler.VisualParent;
                         }
 

diff --git a/tests/Avalonia.Input.UnitTests/KeyboardDeviceTests.cs b/tests/Avalonia.Input.UnitTests/KeyboardDeviceTests.cs
@@ -1,5 +1,9 @@
-using Avalonia.Input.Raw;
+using System;
+using System.Windows.Input;
+using Avalonia.Controls;
+using Avalonia.Input.Raw;
 using Avalonia.Interactivity;
+using Avalonia.UnitTests;
 using Moq;
 using Xunit;
 
@@ -86,5 +90,45 @@ public void TextInput_Should_Be_Sent_To_Focused_Element()
 
             focused.Verify(x => x.RaiseEvent(It.IsAny<TextInputEventArgs>()));
         }
+
+        [Fact]
+        public void Can_Change_KeyBindings_In_Keybinding_Event_Handler()
+        {
+            var target = new KeyboardDevice();
+            var button = new Button();
+            var root = new TestRoot(button);
+            var raised = 0;
+
+            button.KeyBindings.Add(new KeyBinding
+            {
+                Gesture = new KeyGesture(Key.O, KeyModifiers.Control),
+                Command = new DelegateCommand(() =>
+                {
+                    button.KeyBindings.Clear();
+                    ++raised;
+                }),
+            });
+
+            target.SetFocusedElement(button, NavigationMethod.Pointer, 0);
+            target.ProcessRawEvent(
+                new RawKeyEventArgs(
+                    target,
+                    0,
+                    root,
+                    RawKeyEventType.KeyDown,
+                    Key.O,
+                    RawInputModifiers.Control));
+
+            Assert.Equal(1, raised);
+        }
+
+        private class DelegateCommand : ICommand
+        {
+            private readonly Action _action;
+            public DelegateCommand(Action action) => _action = action;
+            public event EventHandler CanExecuteChanged;
+            public bool CanExecute(object parameter) => true;
+            public void Execute(object parameter) => _action();
+        }
     }
 }