diff --git a/ModernWpf.Controls/Repeater/ItemsRepeater/ViewManager.cs b/ModernWpf.Controls/Repeater/ItemsRepeater/ViewManager.cs index 94a1f17b..f71f9b0b 100644 --- a/ModernWpf.Controls/Repeater/ItemsRepeater/ViewManager.cs +++ b/ModernWpf.Controls/Repeater/ItemsRepeater/ViewManager.cs @@ -98,10 +98,31 @@ public void ClearElement(UIElement element, bool isClearedDueToCollectionChange) } } + // We need to clear the datacontext to prevent crashes from happening, + // however we only do that if we were the ones setting it. + // That is when one of the following is the case (numbering taken from line ~642): + // 1.2 No ItemTemplate, data is not a UIElement + // 2.1 ItemTemplate, data is not FrameworkElement + // 2.2.2 Itemtemplate, data is FrameworkElement, ElementFactory returned Element different to data + // + // In all of those three cases, we the ItemTemplateShim is NOT null. + // Luckily when we create the items, we store whether we were the once setting the DataContext. public void ClearElementToElementFactory(UIElement element) { m_owner.OnElementClearing(element); + var virtInfo = ItemsRepeater.GetVirtualizationInfo(element); + virtInfo.MoveOwnershipToElementFactory(); + + // During creation of this object, we were the one setting the DataContext, so clear it now. + if (virtInfo.MustClearDataContext) + { + if (element is FrameworkElement elementAsFE) + { + elementAsFE.DataContext = null; + } + } + if (m_owner.ItemTemplateShim != null) { if (m_ElementFactoryRecycleArgs == null) @@ -133,8 +154,6 @@ public void ClearElementToElementFactory(UIElement element) children.RemoveAt(childIndex); } - var virtInfo = ItemsRepeater.GetVirtualizationInfo(element); - virtInfo.MoveOwnershipToElementFactory(); if (m_lastFocusedElement == element) { // Focused element is going away. Remove the tracked last focused element @@ -575,6 +594,8 @@ ElementFactoryGetArgs initArgs() // View obtained from ElementFactory already has a VirtualizationInfo attached to it // which means that the element has been recycled and not created from scratch. } + // Clear flag + virtInfo.MustClearDataContext = false; if (data != element) { @@ -599,6 +620,7 @@ object initElementDataContext() var elementDataContext = initElementDataContext(); elementAsFE.DataContext = elementDataContext; + virtInfo.MustClearDataContext = true; } else { diff --git a/ModernWpf.Controls/Repeater/Layouts/VirtualizationInfo.cs b/ModernWpf.Controls/Repeater/Layouts/VirtualizationInfo.cs index ac9eb879..084b1788 100644 --- a/ModernWpf.Controls/Repeater/Layouts/VirtualizationInfo.cs +++ b/ModernWpf.Controls/Repeater/Layouts/VirtualizationInfo.cs @@ -50,6 +50,8 @@ public VirtualizationInfo() public bool IsInUniqueIdResetPool => Owner == ElementOwner.UniqueIdResetPool; + public bool MustClearDataContext { get; set; } + public void MoveOwnershipToLayoutFromElementFactory(int index, string uniqueId) { Debug.Assert(Owner == ElementOwner.ElementFactory); diff --git a/test/ModernWpfTestApp/ApiTests/RepeaterTests/ViewManagerTests.cs b/test/ModernWpfTestApp/ApiTests/RepeaterTests/ViewManagerTests.cs index 2e8506ea..193d7d3e 100644 --- a/test/ModernWpfTestApp/ApiTests/RepeaterTests/ViewManagerTests.cs +++ b/test/ModernWpfTestApp/ApiTests/RepeaterTests/ViewManagerTests.cs @@ -760,6 +760,47 @@ public void ValidateFocusMoveOnElementClearedWithUniqueIds() () => { ValidateCurrentFocus(repeater, 0 /*expectedIndex */, "3" /* expectedContent */); } }); } + + [TestMethod] + // Why does this test work? + // When the elements get created from the RecyclingElementFactory, we get already "existing" data templates. + // However, the reason for the crash in #2384 is that those "empty" data templates actually still had their data context + // If that data context is not null, that means it did not get cleared when the element was recycled, which is the wrong behavior. + // To check if the clearing is working correctly, we are checking this inside the ElementFactory's RecycleElement function. + public void ValidateElementClearingClearsDataContext() + { + ItemsRepeater repeater = null; + MockElementFactory elementFactory = null; + int elementClearingRaisedCount = 0; + Log.Comment("Initialize ItemsRepeater"); + RunOnUIThread.Execute(() => + { + elementFactory = new MockElementFactory() { + GetElementFunc = delegate (int index, UIElement owner) { + return new Button() { Content = index }; + }, + + ClearElementFunc = delegate (UIElement element, UIElement owner) { + elementClearingRaisedCount++; + Verify.IsNull((element as FrameworkElement).DataContext); + } + }; + + repeater = CreateRepeater(Enumerable.Range(0, 100), + elementFactory); + + repeater.Layout = new StackLayout(); + + Content = repeater; + repeater.UpdateLayout(); + + repeater.ItemsSource = null; + + Log.Comment("Verify ItemsRepeater cleared data contexts correctly"); + Verify.IsTrue(elementClearingRaisedCount > 0, "ItemsRepeater should have cleared some elements"); + }); + } + private void MoveFocusToIndex(ItemsRepeater repeater, int index) { var element = repeater.TryGetElement(index) as Control;