From f24ab044f78edf98fb9bd69c2b624adbc5da1c1e Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 15 Jun 2016 00:26:00 +0200 Subject: [PATCH 1/4] Added failing binding tests. --- .../Data/BindingTests.cs | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests.cs index ca65b763f5..c5a1744810 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests.cs @@ -1,11 +1,13 @@ // Copyright (c) The Avalonia Project. All rights reserved. // Licensed under the MIT license. See licence.md file in the project root for full license information. -using Moq; +using System; +using System.Collections.Generic; using Avalonia.Controls; using Avalonia.Data; using Avalonia.Markup.Data; using Avalonia.Markup.Xaml.Data; +using Moq; using ReactiveUI; using Xunit; @@ -112,6 +114,24 @@ namespace Avalonia.Markup.Xaml.UnitTests.Data Assert.Equal("baz", source.Foo); } + [Fact] + public void Broken_Binding_Should_Use_Default_Value_Instead_Of_Inherited_Value() + { + var parent = new InheritanceTest() + { + Baz = 9, + DataContext = "data", + }; + + var child = new InheritanceTest(); + var bazBinding = new Binding("Missing"); + parent.Child = child; + + child.Bind(InheritanceTest.BazProperty, bazBinding); + + Assert.Equal(6, child.Baz); + } + [Fact] public void DataContext_Binding_Should_Use_Parent_DataContext() { @@ -159,6 +179,26 @@ namespace Avalonia.Markup.Xaml.UnitTests.Data Assert.Equal("foo", child.DataContext); } + [Fact] + public void DataContext_Binding_Should_Produce_Correct_Results() + { + var root = new Decorator + { + DataContext = new { Foo = "bar" }, + }; + + var child = new Control(); + var dataContextBinding = new Binding("Foo"); + var values = new List(); + + child.GetObservable(Border.DataContextProperty).Subscribe(x => values.Add(x)); + child.Bind(ContentControl.DataContextProperty, dataContextBinding); + + root.Child = child; + + Assert.Equal(new[] { null, "bar" }, values); + } + [Fact] public void Should_Use_DefaultValueConverter_When_No_Converter_Specified() { @@ -337,5 +377,17 @@ namespace Avalonia.Markup.Xaml.UnitTests.Data Bind(BarProperty, this.GetObservable(FooProperty)); } } + + private class InheritanceTest : Decorator + { + public static readonly StyledProperty BazProperty = + AvaloniaProperty.Register("Baz", defaultValue: 6, inherits: true); + + public int Baz + { + get { return GetValue(BazProperty); } + set { SetValue(BazProperty, value); } + } + } } } From 7018b3c36d33052d7b95b7471789c625fa34c9d6 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 15 Jun 2016 22:54:14 +0200 Subject: [PATCH 2/4] Removed test. Decided that we shouldn't do this. --- .../Data/BindingTests.cs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests.cs b/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests.cs index c5a1744810..1174b4ceb5 100644 --- a/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests.cs +++ b/tests/Avalonia.Markup.Xaml.UnitTests/Data/BindingTests.cs @@ -114,24 +114,6 @@ namespace Avalonia.Markup.Xaml.UnitTests.Data Assert.Equal("baz", source.Foo); } - [Fact] - public void Broken_Binding_Should_Use_Default_Value_Instead_Of_Inherited_Value() - { - var parent = new InheritanceTest() - { - Baz = 9, - DataContext = "data", - }; - - var child = new InheritanceTest(); - var bazBinding = new Binding("Missing"); - parent.Child = child; - - child.Bind(InheritanceTest.BazProperty, bazBinding); - - Assert.Equal(6, child.Baz); - } - [Fact] public void DataContext_Binding_Should_Use_Parent_DataContext() { From 0b28e10f212e7429593c3485309d2222eb6140e5 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 15 Jun 2016 22:56:38 +0200 Subject: [PATCH 3/4] Make broken DataContext bindings produce null. This prevents incorrect DataContexts cascading down to children when the DataContext binding is invalid, e.g. when things are being set up. --- .../Avalonia.Markup.Xaml/Data/Binding.cs | 15 +++++++++- .../MarkupExtensions/BindingExtension.cs | 2 +- .../Avalonia.Markup/Data/ExpressionSubject.cs | 28 ++++++++++++++++--- .../Data/ExpressionSubjectTests.cs | 8 +++--- 4 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/Markup/Avalonia.Markup.Xaml/Data/Binding.cs b/src/Markup/Avalonia.Markup.Xaml/Data/Binding.cs index 4b812e12f0..ec60695374 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Data/Binding.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Data/Binding.cs @@ -20,6 +20,7 @@ namespace Avalonia.Markup.Xaml.Data /// public Binding() { + FallbackValue = AvaloniaProperty.UnsetValue; } /// @@ -27,6 +28,7 @@ namespace Avalonia.Markup.Xaml.Data /// /// The binding path. public Binding(string path) + : this() { Path = path; } @@ -122,12 +124,23 @@ namespace Avalonia.Markup.Xaml.Data throw new NotSupportedException(); } + var fallback = FallbackValue; + + // If we're binding to DataContext and our fallback is UnsetValue then override + // the fallback value to null, as broken bindings to DataContext must reset the + // DataContext in order to not propagate incorrect DataContexts to child controls. + // See Avalonia.Markup.Xaml.UnitTests.Data.DataContext_Binding_Should_Produce_Correct_Results. + if (targetProperty == Control.DataContextProperty && fallback == AvaloniaProperty.UnsetValue) + { + fallback = null; + } + var subject = new ExpressionSubject( observer, targetProperty?.PropertyType ?? typeof(object), + fallback, Converter ?? DefaultValueConverter.Instance, ConverterParameter, - FallbackValue, Priority); return new InstancedBinding(subject, Mode, Priority); diff --git a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/BindingExtension.cs b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/BindingExtension.cs index a58a8614ee..70d3f7d161 100644 --- a/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/BindingExtension.cs +++ b/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions/BindingExtension.cs @@ -36,7 +36,7 @@ namespace Avalonia.Markup.Xaml.MarkupExtensions public IValueConverter Converter { get; set; } public object ConverterParameter { get; set; } public string ElementName { get; set; } - public object FallbackValue { get; set; } + public object FallbackValue { get; set; } = AvaloniaProperty.UnsetValue; public BindingMode Mode { get; set; } public string Path { get; set; } public BindingPriority Priority { get; set; } = BindingPriority.LocalValue; diff --git a/src/Markup/Avalonia.Markup/Data/ExpressionSubject.cs b/src/Markup/Avalonia.Markup/Data/ExpressionSubject.cs index 05bf818aad..0a3be26c18 100644 --- a/src/Markup/Avalonia.Markup/Data/ExpressionSubject.cs +++ b/src/Markup/Avalonia.Markup/Data/ExpressionSubject.cs @@ -41,16 +41,36 @@ namespace Avalonia.Markup.Data /// /// A parameter to pass to . /// + /// The binding priority. + public ExpressionSubject( + ExpressionObserver inner, + Type targetType, + IValueConverter converter, + object converterParameter = null, + BindingPriority priority = BindingPriority.LocalValue) + : this(inner, targetType, AvaloniaProperty.UnsetValue, converter, converterParameter, priority) + { + } + + /// + /// Initializes a new instance of the class. + /// + /// The . + /// The type to convert the value to. /// /// The value to use when the binding is unable to produce a value. /// + /// The value converter to use. + /// + /// A parameter to pass to . + /// /// The binding priority. public ExpressionSubject( ExpressionObserver inner, - Type targetType, + Type targetType, + object fallbackValue, IValueConverter converter, object converterParameter = null, - object fallbackValue = null, BindingPriority priority = BindingPriority.LocalValue) { Contract.Requires(inner != null); @@ -117,7 +137,7 @@ namespace Avalonia.Markup.Data _inner.Expression, error.Exception.Message); - if (_fallbackValue != null) + if (_fallbackValue != AvaloniaProperty.UnsetValue) { if (TypeUtilities.TryConvert( type, @@ -162,7 +182,7 @@ namespace Avalonia.Markup.Data ConverterParameter, CultureInfo.CurrentUICulture); - if (_fallbackValue != null && + if (_fallbackValue != AvaloniaProperty.UnsetValue && (converted == AvaloniaProperty.UnsetValue || converted is BindingError)) { diff --git a/tests/Avalonia.Markup.UnitTests/Data/ExpressionSubjectTests.cs b/tests/Avalonia.Markup.UnitTests/Data/ExpressionSubjectTests.cs index 9ee32149e0..8b763e7fb9 100644 --- a/tests/Avalonia.Markup.UnitTests/Data/ExpressionSubjectTests.cs +++ b/tests/Avalonia.Markup.UnitTests/Data/ExpressionSubjectTests.cs @@ -123,8 +123,8 @@ namespace Avalonia.Markup.UnitTests.Data var target = new ExpressionSubject( new ExpressionObserver(data, "DoubleValue"), typeof(string), - DefaultValueConverter.Instance, - fallbackValue: "9.8"); + "9.8", + DefaultValueConverter.Instance); target.OnNext("foo"); @@ -162,7 +162,7 @@ namespace Avalonia.Markup.UnitTests.Data new ExpressionObserver(data, "DoubleValue"), typeof(string), converter.Object, - "foo"); + converterParameter: "foo"); target.Subscribe(_ => { }); @@ -178,7 +178,7 @@ namespace Avalonia.Markup.UnitTests.Data new ExpressionObserver(data, "DoubleValue"), typeof(string), converter.Object, - "foo"); + converterParameter: "foo"); target.OnNext("bar"); From e252a35509b2e20e0fa19522ee1b97d4d90ff7ae Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 15 Jun 2016 22:57:24 +0200 Subject: [PATCH 4/4] Don't delay bind to DataContext. DataContext bindings are special in that they return null instead of UnsetValue when broken, and it's important that they do that when things are being set up. --- .../Avalonia.Markup.Xaml/Context/PropertyAccessor.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Markup/Avalonia.Markup.Xaml/Context/PropertyAccessor.cs b/src/Markup/Avalonia.Markup.Xaml/Context/PropertyAccessor.cs index 20d9b07daf..e295292ba0 100644 --- a/src/Markup/Avalonia.Markup.Xaml/Context/PropertyAccessor.cs +++ b/src/Markup/Avalonia.Markup.Xaml/Context/PropertyAccessor.cs @@ -136,7 +136,14 @@ namespace Avalonia.Markup.Xaml.Context if (control != null) { - DelayedBinding.Add(control, property, binding); + if (property != Control.DataContextProperty) + { + DelayedBinding.Add(control, property, binding); + } + else + { + control.Bind(property, binding); + } } else {