From 3304d646c007d3cbe72784bde5227ed76133d377 Mon Sep 17 00:00:00 2001 From: Fusion86 Date: Sun, 15 Nov 2020 18:49:23 +0100 Subject: [PATCH 1/4] Fix crash when KeyBindings change while they are being handled --- src/Avalonia.Input/KeyboardDevice.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Input/KeyboardDevice.cs b/src/Avalonia.Input/KeyboardDevice.cs index 6f4cb7a35c7..9a42e3d3a5e 100644 --- a/src/Avalonia.Input/KeyboardDevice.cs +++ b/src/Avalonia.Input/KeyboardDevice.cs @@ -211,12 +211,18 @@ public void ProcessRawEvent(RawInputEventArgs e) { var bindings = (currentHandler as IInputElement)?.KeyBindings; if (bindings != null) - foreach (var binding in bindings) + { + // Create a copy of the KeyBindings list. + // 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. + var cpy = bindings.ToArray(); + foreach (var binding in cpy) { if (ev.Handled) break; binding.TryHandle(ev); } + } currentHandler = currentHandler.VisualParent; } From 6b94cc4ed99dd9b2ee86b2396da5d9a7a6e788ab Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 14 Jun 2021 12:56:22 +0200 Subject: [PATCH 2/4] Added failing test for #5054. --- .../KeyboardDeviceTests.cs | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.Input.UnitTests/KeyboardDeviceTests.cs b/tests/Avalonia.Input.UnitTests/KeyboardDeviceTests.cs index df0a077c7f4..5b39e23ba60 100644 --- 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,38 @@ public void TextInput_Should_Be_Sent_To_Focused_Element() focused.Verify(x => x.RaiseEvent(It.IsAny())); } + + [Fact] + public void Can_Change_KeyBindings_In_Keybinding_Event_Handler() + { + var target = new KeyboardDevice(); + var button = new Button(); + var root = new TestRoot(button); + + button.KeyBindings.Add(new KeyBinding + { + Gesture = new KeyGesture(Key.O, KeyModifiers.Control), + Command = new DelegateCommand(() => button.KeyBindings.Clear()), + }); + + target.SetFocusedElement(button, NavigationMethod.Pointer, 0); + target.ProcessRawEvent( + new RawKeyEventArgs( + target, + 0, + root, + RawKeyEventType.KeyDown, + Key.O, + RawInputModifiers.Control)); + } + + 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(); + } } } From 4285c3d0d1981993eda9fd9fe977e87fdcf5acf0 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 14 Jun 2021 13:04:56 +0200 Subject: [PATCH 3/4] Don't create a copy of the array unless necessary. --- src/Avalonia.Input/KeyboardDevice.cs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/Avalonia.Input/KeyboardDevice.cs b/src/Avalonia.Input/KeyboardDevice.cs index bf2f6897857..a159b19026f 100644 --- a/src/Avalonia.Input/KeyboardDevice.cs +++ b/src/Avalonia.Input/KeyboardDevice.cs @@ -218,15 +218,28 @@ public void ProcessRawEvent(RawInputEventArgs e) var bindings = (currentHandler as IInputElement)?.KeyBindings; if (bindings != null) { - // Create a copy of the KeyBindings list. + 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. - var cpy = bindings.ToArray(); - foreach (var binding in cpy) + 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; From a11270b07e5edc062bbe37b4b356598bf574c0c7 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Mon, 14 Jun 2021 13:07:28 +0200 Subject: [PATCH 4/4] Add a sanity check to the test. --- tests/Avalonia.Input.UnitTests/KeyboardDeviceTests.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.Input.UnitTests/KeyboardDeviceTests.cs b/tests/Avalonia.Input.UnitTests/KeyboardDeviceTests.cs index 5b39e23ba60..7730cee78c8 100644 --- a/tests/Avalonia.Input.UnitTests/KeyboardDeviceTests.cs +++ b/tests/Avalonia.Input.UnitTests/KeyboardDeviceTests.cs @@ -97,11 +97,16 @@ 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()), + Command = new DelegateCommand(() => + { + button.KeyBindings.Clear(); + ++raised; + }), }); target.SetFocusedElement(button, NavigationMethod.Pointer, 0); @@ -113,6 +118,8 @@ public void Can_Change_KeyBindings_In_Keybinding_Event_Handler() RawKeyEventType.KeyDown, Key.O, RawInputModifiers.Control)); + + Assert.Equal(1, raised); } private class DelegateCommand : ICommand