diff --git a/src/Avalonia.Controls/Button.cs b/src/Avalonia.Controls/Button.cs index 91eef3947b..c779e4b0cb 100644 --- a/src/Avalonia.Controls/Button.cs +++ b/src/Avalonia.Controls/Button.cs @@ -80,6 +80,7 @@ namespace Avalonia.Controls private ICommand _command; private bool _commandCanExecute = true; + private KeyGesture _hotkey; /// /// Initializes static members of the class. @@ -207,6 +208,11 @@ namespace Avalonia.Controls protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e) { + if (_hotkey != null) // Control attached again, set Hotkey to create a hotkey manager for this control + { + HotKey = _hotkey; + } + base.OnAttachedToLogicalTree(e); if (Command != null) @@ -217,6 +223,13 @@ namespace Avalonia.Controls protected override void OnDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs e) { + // This will cause the hotkey manager to dispose the observer and the reference to this control + if (HotKey != null) + { + _hotkey = HotKey; + HotKey = null; + } + base.OnDetachedFromLogicalTree(e); if (Command != null) diff --git a/src/Avalonia.Controls/MenuItem.cs b/src/Avalonia.Controls/MenuItem.cs index d034f54df2..71ac0fa523 100644 --- a/src/Avalonia.Controls/MenuItem.cs +++ b/src/Avalonia.Controls/MenuItem.cs @@ -102,6 +102,7 @@ namespace Avalonia.Controls private ICommand? _command; private bool _commandCanExecute = true; private Popup? _popup; + private KeyGesture _hotkey; /// /// Initializes static members of the class. @@ -337,6 +338,11 @@ namespace Avalonia.Controls protected override void OnAttachedToLogicalTree(LogicalTreeAttachmentEventArgs e) { + if (_hotkey != null) // Control attached again, set Hotkey to create a hotkey manager for this control + { + HotKey = _hotkey; + } + base.OnAttachedToLogicalTree(e); if (Command != null) @@ -347,6 +353,13 @@ namespace Avalonia.Controls protected override void OnDetachedFromLogicalTree(LogicalTreeAttachmentEventArgs e) { + // This will cause the hotkey manager to dispose the observer and the reference to this control + if (HotKey != null) + { + _hotkey = HotKey; + HotKey = null; + } + base.OnDetachedFromLogicalTree(e); if (Command != null) diff --git a/src/Avalonia.Controls/Utils/AncestorFinder.cs b/src/Avalonia.Controls/Utils/AncestorFinder.cs index 529aec1393..42e0d85dcd 100644 --- a/src/Avalonia.Controls/Utils/AncestorFinder.cs +++ b/src/Avalonia.Controls/Utils/AncestorFinder.cs @@ -47,6 +47,8 @@ namespace Avalonia.Controls.Utils public void Dispose() { + _child?.Dispose(); + _subject.Dispose(); _disposable.Dispose(); } } diff --git a/tests/Avalonia.Controls.UnitTests/Utils/HotKeyManagerTests.cs b/tests/Avalonia.Controls.UnitTests/Utils/HotKeyManagerTests.cs index 151c3070f4..2a3db9992c 100644 --- a/tests/Avalonia.Controls.UnitTests/Utils/HotKeyManagerTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Utils/HotKeyManagerTests.cs @@ -1,9 +1,14 @@ -using Moq; +using System; +using System.Collections.Generic; +using Avalonia.Collections; using Avalonia.Controls.Presenters; using Avalonia.Controls.Templates; +using Avalonia.Data; using Avalonia.Input; using Avalonia.Platform; using Avalonia.Styling; +using Avalonia.UnitTests; +using Moq; using Xunit; using System; using Avalonia.Input.Raw; @@ -53,7 +58,115 @@ namespace Avalonia.Controls.UnitTests.Utils HotKeyManager.SetHotKey(button, null); Assert.Empty(tl.KeyBindings); + } + } + + [Fact] + public void HotKeyManager_Should_Release_Reference_When_Control_Detached() + { + using (AvaloniaLocator.EnterScope()) + { + var styler = new Mock(); + + AvaloniaLocator.CurrentMutable + .Bind().ToConstant(new WindowingPlatformMock()) + .Bind().ToConstant(styler.Object); + + var gesture1 = new KeyGesture(Key.A, KeyModifiers.Control); + + WeakReference reference = null; + + var tl = new Window(); + new Action(() => + { + var button = new Button(); + reference = new WeakReference(button, true); + tl.Content = button; + tl.Template = CreateWindowTemplate(); + tl.ApplyTemplate(); + tl.Presenter.ApplyTemplate(); + HotKeyManager.SetHotKey(button, gesture1); + + // Detach the button from the logical tree, so there is no reference to it + tl.Content = null; + tl.ApplyTemplate(); + })(); + + + // The button should be collected since it's detached from the listbox + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + + Assert.Null(reference?.Target); + } + } + + [Fact] + public void HotKeyManager_Should_Release_Reference_When_Control_In_Item_Template_Detached() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var styler = new Mock(); + + AvaloniaLocator.CurrentMutable + .Bind().ToConstant(new WindowingPlatformMock()) + .Bind().ToConstant(styler.Object); + + var gesture1 = new KeyGesture(Key.A, KeyModifiers.Control); + + var weakReferences = new List(); + var tl = new Window { SizeToContent = SizeToContent.WidthAndHeight, IsVisible = true }; + var lm = tl.LayoutManager; + + var keyGestures = new AvaloniaList { gesture1 }; + var listBox = new ListBox + { + Width = 100, + Height = 100, + VirtualizationMode = ItemVirtualizationMode.None, + // Create a button with binding to the KeyGesture in the template and add it to references list + ItemTemplate = new FuncDataTemplate(typeof(KeyGesture), (o, scope) => + { + var keyGesture = o as KeyGesture; + var button = new Button + { + DataContext = keyGesture, [!Button.HotKeyProperty] = new Binding("") + }; + weakReferences.Add(new WeakReference(button, true)); + return button; + }) + }; + // Add the listbox and render it + tl.Content = listBox; + lm.ExecuteInitialLayoutPass(); + listBox.Items = keyGestures; + lm.ExecuteLayoutPass(); + + // Let the button detach when clearing the source items + keyGestures.Clear(); + lm.ExecuteLayoutPass(); + + // Add it again to double check,and render + keyGestures.Add(gesture1); + lm.ExecuteLayoutPass(); + + keyGestures.Clear(); + lm.ExecuteLayoutPass(); + + // The button should be collected since it's detached from the listbox + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + + Assert.True(weakReferences.Count > 0); + foreach (var weakReference in weakReferences) + { + Assert.Null(weakReference.Target); + } } }