Skip to content

Commit

Permalink
fix: DataContext is wrongly propagated to all DependencyObject when s…
Browse files Browse the repository at this point in the history
…et 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)
  • Loading branch information
dr1rrb committed Jul 15, 2020
1 parent 5559442 commit 4863929
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 15 deletions.
Expand Up @@ -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
Expand Down Expand Up @@ -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); }
}
}
}
2 changes: 1 addition & 1 deletion src/Uno.UI/UI/Xaml/DependencyObjectStore.Binder.cs
Expand Up @@ -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)
{
Expand Down
18 changes: 9 additions & 9 deletions src/Uno.UI/UI/Xaml/FrameworkPropertyMetadata.cs
Expand Up @@ -31,7 +31,7 @@ object defaultValue
FrameworkPropertyMetadataOptions options
) : base(defaultValue)
{
Options = options;
Options = options.WithDefault();
}

public FrameworkPropertyMetadata(
Expand All @@ -40,7 +40,7 @@ FrameworkPropertyMetadataOptions options
PropertyChangedCallback propertyChangedCallback
) : base(defaultValue, propertyChangedCallback)
{
Options = options;
Options = options.WithDefault();
}

internal FrameworkPropertyMetadata(
Expand All @@ -50,7 +50,7 @@ PropertyChangedCallback propertyChangedCallback
BackingFieldUpdateCallback backingFieldUpdateCallback
) : base(defaultValue, propertyChangedCallback, backingFieldUpdateCallback)
{
Options = options;
Options = options.WithDefault();
}

internal FrameworkPropertyMetadata(
Expand All @@ -66,7 +66,7 @@ BackingFieldUpdateCallback backingFieldUpdateCallback
BackingFieldUpdateCallback backingFieldUpdateCallback
) : base(defaultValue, null, backingFieldUpdateCallback)
{
Options = options;
Options = options.WithDefault();
}

internal FrameworkPropertyMetadata(
Expand All @@ -76,7 +76,7 @@ BackingFieldUpdateCallback backingFieldUpdateCallback
CoerceValueCallback coerceValueCallback
) : base(defaultValue, propertyChangedCallback, coerceValueCallback, null)
{
Options = options;
Options = options.WithDefault();
}

internal FrameworkPropertyMetadata(
Expand All @@ -87,7 +87,7 @@ CoerceValueCallback coerceValueCallback
BackingFieldUpdateCallback backingFieldUpdateCallback
) : base(defaultValue, propertyChangedCallback, coerceValueCallback, backingFieldUpdateCallback)
{
Options = options;
Options = options.WithDefault();
}

internal FrameworkPropertyMetadata(
Expand Down Expand Up @@ -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
{
Expand Down
14 changes: 9 additions & 5 deletions src/Uno.UI/UI/Xaml/FrameworkPropertyMetadataOptions.cs
Expand Up @@ -18,10 +18,6 @@ public enum FrameworkPropertyMetadataOptions
/// <summary>
/// Specifies that this property's value or values will inherit the DataContext of its or their parent.
/// </summary>
/// <remarks>
/// This property is ignored and ValueInheritsDataContext is interpreted as true for properties of type <see cref="DependencyObject"/>.
/// Use ValueDoesNotInheritDataContext to prevent this default behavior.
/// </remarks>
ValueInheritsDataContext = 1 << 1,

/// <summary>
Expand All @@ -33,7 +29,8 @@ public enum FrameworkPropertyMetadataOptions
/// Prevents this property's value or values from inheriting the DataContext of its or their parent.
/// </summary>
/// <remarks>
/// This is useful for properties of type <see cref="DependencyObject"/> for which ValueInheritsDataContext is always interpreted as true.
/// This is useful for framework properties of type <see cref="DependencyObject"/> for which ValueInheritsDataContext is enabled by default
/// (cf. <see cref="Default"/>).
/// </remarks>
ValueDoesNotInheritDataContext = 1 << 3,

Expand Down Expand Up @@ -62,5 +59,12 @@ public enum FrameworkPropertyMetadataOptions
/// </summary>
AffectsRender = 1 << 8,

/// <summary>
/// The default options set when creating a <see cref="FrameworkPropertyMetadata"/>.
/// </summary>
/// <remarks>
/// By default all DP declared by the framework will inherit the DataContext while DP declared by application won't.
/// </remarks>
Default = ValueInheritsDataContext,
}
}
Expand Up @@ -67,5 +67,12 @@ public static bool HasLogicalChild(this FrameworkPropertyMetadataOptions options
public static bool HasWeakStorage(this FrameworkPropertyMetadataOptions options)
=> (options & FrameworkPropertyMetadataOptions.WeakStorage) != 0;

/// <summary>
/// Defines the default flags for a FrameworkPropertyMetadata if not explicitly opt-out (cf. <see cref="FrameworkPropertyMetadataOptions.Default"/>).
/// </summary>
internal static FrameworkPropertyMetadataOptions WithDefault(this FrameworkPropertyMetadataOptions options)
=> (options & (FrameworkPropertyMetadataOptions.ValueDoesNotInheritDataContext | FrameworkPropertyMetadataOptions.ValueInheritsDataContext)) == default
? options | FrameworkPropertyMetadataOptions.ValueInheritsDataContext // == FrameworkPropertyMetadataOptions.Default
: options;
}
}

0 comments on commit 4863929

Please sign in to comment.