From 4863929555a0f20cfcaa25f1e8fce437484aea8a Mon Sep 17 00:00:00 2001 From: David Date: Wed, 8 Jul 2020 16:30:08 -0400 Subject: [PATCH] fix: DataContext is wrongly propagated to all DependencyObject when set into a DependencyProperty This might drives to invalid DataContext changes when an element only wants to keep a reference on a parent. As the propagation is expected on almost all DP declared by the Framework, the inheritance is enabled by default for those properties (when using FrameworkPropertyMetadata), and disabled for properties defined in application (PropertyMetadata) --- .../Given_DependencyProperty.DataContext.cs | 44 +++++++++++++++++++ .../UI/Xaml/DependencyObjectStore.Binder.cs | 2 +- .../UI/Xaml/FrameworkPropertyMetadata.cs | 18 ++++---- .../Xaml/FrameworkPropertyMetadataOptions.cs | 14 +++--- ...meworkPropertyMetadataOptionsExtensions.cs | 7 +++ 5 files changed, 70 insertions(+), 15 deletions(-) diff --git a/src/Uno.UI.Tests/DependencyProperty/Given_DependencyProperty.DataContext.cs b/src/Uno.UI.Tests/DependencyProperty/Given_DependencyProperty.DataContext.cs index 286248907404..dc83a565b6a4 100644 --- a/src/Uno.UI.Tests/DependencyProperty/Given_DependencyProperty.DataContext.cs +++ b/src/Uno.UI.Tests/DependencyProperty/Given_DependencyProperty.DataContext.cs @@ -199,6 +199,38 @@ public void When_DataContext_Inherited_And_Parent_Changed() Assert.AreEqual(42, SUT.DataContext); } + + [TestMethod] + public void When_DataContext_Inherited_And_Child_Removed() + { + var SUT = new MyControl(); + + var independentChild = new MyControl(); + var parent = new Grid + { + Children = { independentChild, SUT }, + DataContext = 42 + }; + + // And here is the trick: the SUT does have a reference to the child + SUT.InnerControl = independentChild; + + int parentCtxChanged = 0, childCtxChanged = 0, SUTCtxChanged = 0; + parent.RegisterPropertyChangedCallback(Control.DataContextProperty, (snd, dp) => parentCtxChanged++); + independentChild.RegisterPropertyChangedCallback(Control.DataContextProperty, (snd, dp) => childCtxChanged++); + SUT.RegisterPropertyChangedCallback(Control.DataContextProperty, (snd, dp) => SUTCtxChanged++); + + Assert.AreEqual(42, SUT.DataContext); + + // And here the issue: when we remove the SUT from the parent, + // it will propagate to the independentChild the DataContext change ... while it should not! + parent.Children.Remove(SUT); + + Assert.AreEqual(null, SUT.DataContext); + Assert.AreEqual(0, parentCtxChanged); + Assert.AreEqual(1, SUTCtxChanged); + Assert.AreEqual(0, childCtxChanged); + } } public partial class MyBasicListType : DependencyObject @@ -264,6 +296,18 @@ private void OnInnerObjectChanged(DependencyPropertyChangedEventArgs e) } #endregion + } + public partial class MyControl : Control + { + // Just a standard DP defined by a project / third party component + public static readonly DependencyProperty InnerControlProperty = DependencyProperty.Register( + "InnerControl", typeof(MyControl), typeof(MyControl), new PropertyMetadata(default(MyControl))); + + public MyControl InnerControl + { + get { return (MyControl)GetValue(InnerControlProperty); } + set { SetValue(InnerControlProperty, value); } + } } } diff --git a/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs b/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs index 022ccdcc4f08..1a760ee7c73b 100644 --- a/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs +++ b/src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs @@ -494,7 +494,7 @@ private void OnDependencyPropertyChanged(DependencyPropertyDetails propertyDetai var (hasValueInherits, hasValueDoesNotInherit) = GetPropertyInheritanceConfiguration(propertyDetails); - if (!hasValueDoesNotInherit && (hasValueInherits || propertyDetails.Property.HasAutoDataContextInherit)) + if (!hasValueDoesNotInherit && hasValueInherits) { if(args.NewValue is IDependencyObjectStoreProvider provider) { diff --git a/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadata.cs b/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadata.cs index ecad3a614f6b..a362ee5b2194 100644 --- a/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadata.cs +++ b/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadata.cs @@ -31,7 +31,7 @@ object defaultValue FrameworkPropertyMetadataOptions options ) : base(defaultValue) { - Options = options; + Options = options.WithDefault(); } public FrameworkPropertyMetadata( @@ -40,7 +40,7 @@ FrameworkPropertyMetadataOptions options PropertyChangedCallback propertyChangedCallback ) : base(defaultValue, propertyChangedCallback) { - Options = options; + Options = options.WithDefault(); } internal FrameworkPropertyMetadata( @@ -50,7 +50,7 @@ PropertyChangedCallback propertyChangedCallback BackingFieldUpdateCallback backingFieldUpdateCallback ) : base(defaultValue, propertyChangedCallback, backingFieldUpdateCallback) { - Options = options; + Options = options.WithDefault(); } internal FrameworkPropertyMetadata( @@ -66,7 +66,7 @@ BackingFieldUpdateCallback backingFieldUpdateCallback BackingFieldUpdateCallback backingFieldUpdateCallback ) : base(defaultValue, null, backingFieldUpdateCallback) { - Options = options; + Options = options.WithDefault(); } internal FrameworkPropertyMetadata( @@ -76,7 +76,7 @@ BackingFieldUpdateCallback backingFieldUpdateCallback CoerceValueCallback coerceValueCallback ) : base(defaultValue, propertyChangedCallback, coerceValueCallback, null) { - Options = options; + Options = options.WithDefault(); } internal FrameworkPropertyMetadata( @@ -87,7 +87,7 @@ CoerceValueCallback coerceValueCallback BackingFieldUpdateCallback backingFieldUpdateCallback ) : base(defaultValue, propertyChangedCallback, coerceValueCallback, backingFieldUpdateCallback) { - Options = options; + Options = options.WithDefault(); } internal FrameworkPropertyMetadata( @@ -128,11 +128,11 @@ CoerceValueCallback coerceValueCallback UpdateSourceTrigger defaultUpdateSourceTrigger ) : base(defaultValue, propertyChangedCallback, coerceValueCallback, null) { - Options = options; + Options = options.WithDefault(); DefaultUpdateSourceTrigger = defaultUpdateSourceTrigger; } - - public FrameworkPropertyMetadataOptions Options { get; set; } + + public FrameworkPropertyMetadataOptions Options { get; set; } = FrameworkPropertyMetadataOptions.Default; public UpdateSourceTrigger DefaultUpdateSourceTrigger { diff --git a/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadataOptions.cs b/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadataOptions.cs index 00809da37979..27178fcc4d3b 100644 --- a/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadataOptions.cs +++ b/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadataOptions.cs @@ -18,10 +18,6 @@ public enum FrameworkPropertyMetadataOptions /// /// Specifies that this property's value or values will inherit the DataContext of its or their parent. /// - /// - /// This property is ignored and ValueInheritsDataContext is interpreted as true for properties of type . - /// Use ValueDoesNotInheritDataContext to prevent this default behavior. - /// ValueInheritsDataContext = 1 << 1, /// @@ -33,7 +29,8 @@ public enum FrameworkPropertyMetadataOptions /// Prevents this property's value or values from inheriting the DataContext of its or their parent. /// /// - /// This is useful for properties of type for which ValueInheritsDataContext is always interpreted as true. + /// This is useful for framework properties of type for which ValueInheritsDataContext is enabled by default + /// (cf. ). /// ValueDoesNotInheritDataContext = 1 << 3, @@ -62,5 +59,12 @@ public enum FrameworkPropertyMetadataOptions /// AffectsRender = 1 << 8, + /// + /// The default options set when creating a . + /// + /// + /// By default all DP declared by the framework will inherit the DataContext while DP declared by application won't. + /// + Default = ValueInheritsDataContext, } } diff --git a/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadataOptionsExtensions.cs b/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadataOptionsExtensions.cs index 6ee7d289d0f1..611f81353752 100644 --- a/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadataOptionsExtensions.cs +++ b/src/Uno.UI/UI/Xaml/FrameworkPropertyMetadataOptionsExtensions.cs @@ -67,5 +67,12 @@ public static bool HasLogicalChild(this FrameworkPropertyMetadataOptions options public static bool HasWeakStorage(this FrameworkPropertyMetadataOptions options) => (options & FrameworkPropertyMetadataOptions.WeakStorage) != 0; + /// + /// Defines the default flags for a FrameworkPropertyMetadata if not explicitly opt-out (cf. ). + /// + internal static FrameworkPropertyMetadataOptions WithDefault(this FrameworkPropertyMetadataOptions options) + => (options & (FrameworkPropertyMetadataOptions.ValueDoesNotInheritDataContext | FrameworkPropertyMetadataOptions.ValueInheritsDataContext)) == default + ? options | FrameworkPropertyMetadataOptions.ValueInheritsDataContext // == FrameworkPropertyMetadataOptions.Default + : options; } }