From 5271f8006afc6384ec3beac4615b9aec2babe412 Mon Sep 17 00:00:00 2001 From: Bartosz Korczynski Date: Tue, 7 May 2024 09:41:16 +0100 Subject: [PATCH 1/2] Wrap type conversions in try-catch to prevent crashes due to unhandled exceptions --- .../Data/Core/TargetTypeConverter.cs | 36 +++++++++++++++--- .../TextBoxTests_DataValidation.cs | 37 +++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/src/Avalonia.Base/Data/Core/TargetTypeConverter.cs b/src/Avalonia.Base/Data/Core/TargetTypeConverter.cs index 2efc5b42bdc..57018bff55e 100644 --- a/src/Avalonia.Base/Data/Core/TargetTypeConverter.cs +++ b/src/Avalonia.Base/Data/Core/TargetTypeConverter.cs @@ -76,16 +76,32 @@ public override bool TryConvert(object? value, Type type, CultureInfo culture, o if (toTypeConverter.CanConvertFrom(from)) { - result = toTypeConverter.ConvertFrom(null, culture, value); - return true; + try + { + result = toTypeConverter.ConvertFrom(null, culture, value); + return true; + } + catch + { + result = null; + return false; + } } var fromTypeConverter = TypeDescriptor.GetConverter(from); if (fromTypeConverter.CanConvertTo(t)) { - result = fromTypeConverter.ConvertTo(null, culture, value, t); - return true; + try + { + result = fromTypeConverter.ConvertTo(null, culture, value, t); + return true; + } + catch + { + result = null; + return false; + } } // TODO: This requires reflection: we probably need to make compiled bindings emit @@ -95,8 +111,16 @@ public override bool TryConvert(object? value, Type type, CultureInfo culture, o t, OperatorType.Implicit | OperatorType.Explicit) is { } cast) { - result = cast.Invoke(null, new[] { value }); - return true; + try + { + result = cast.Invoke(null, new[] { value }); + return true; + } + catch + { + result = null; + return false; + } } #pragma warning restore IL2067 #pragma warning restore IL2026 diff --git a/tests/Avalonia.Controls.UnitTests/TextBoxTests_DataValidation.cs b/tests/Avalonia.Controls.UnitTests/TextBoxTests_DataValidation.cs index 295fc192d76..0eaa3092e98 100644 --- a/tests/Avalonia.Controls.UnitTests/TextBoxTests_DataValidation.cs +++ b/tests/Avalonia.Controls.UnitTests/TextBoxTests_DataValidation.cs @@ -6,8 +6,11 @@ using Avalonia.Controls.Presenters; using Avalonia.Controls.Templates; using Avalonia.Data; +using Avalonia.Data.Core; using Avalonia.Headless; using Avalonia.Markup.Data; +using Avalonia.Markup.Xaml.MarkupExtensions; +using Avalonia.Markup.Xaml.MarkupExtensions.CompiledBindings; using Avalonia.Platform; using Avalonia.UnitTests; using Moq; @@ -110,6 +113,40 @@ public void Setter_Exceptions_Should_Set_DataValidationErrors_HasErrors() } } + [Fact] + public void CompiledBindings_TypeConverter_Exceptions_Should_Set_DataValidationErrors_HasErrors() + { + var path = new CompiledBindingPathBuilder() + .Property( + new ClrPropertyInfo( + nameof(ExceptionTest.LessThan10), + target => ((ExceptionTest)target).LessThan10, + (target, value) => ((ExceptionTest)target).LessThan10 = (int)value, + typeof(int)), + PropertyInfoAccessorFactory.CreateInpcPropertyAccessor) + .Build(); + + using (UnitTestApplication.Start(Services)) + { + var target = new TextBox + { + DataContext = new ExceptionTest(), + [!TextBox.TextProperty] = new CompiledBindingExtension + { + Source = new ExceptionTest(), + Path = path, + Mode = BindingMode.TwoWay + }, + Template = CreateTemplate(), + }; + + target.ApplyTemplate(); + + target.Text = "a"; + Assert.True(DataValidationErrors.GetHasErrors(target)); + } + } + private static TestServices Services => TestServices.MockThreadingInterface.With( standardCursorFactory: Mock.Of(), textShaperImpl: new HeadlessTextShaperStub(), From e54d25fbfe1f48b1ebc90f7793328789cca16dba Mon Sep 17 00:00:00 2001 From: Bartosz Korczynski Date: Tue, 23 Apr 2024 18:51:55 +0100 Subject: [PATCH 2/2] Include Adorners in matrix calculations in VisualExtensions --- .../Controls/AdornerLayerBase.cs | 23 +++++++++++ src/Avalonia.Base/Controls/IAdornerLayer.cs | 9 ++++ src/Avalonia.Base/VisualExtensions.cs | 41 ++++++++++++++++++- .../Primitives/AdornerLayer.cs | 4 +- .../PopupPositioning/IPopupPositioner.cs | 29 +------------ .../AdornerLayerTests.cs | 34 +++++++++++++++ 6 files changed, 108 insertions(+), 32 deletions(-) create mode 100644 src/Avalonia.Base/Controls/AdornerLayerBase.cs create mode 100644 src/Avalonia.Base/Controls/IAdornerLayer.cs create mode 100644 tests/Avalonia.Controls.UnitTests/AdornerLayerTests.cs diff --git a/src/Avalonia.Base/Controls/AdornerLayerBase.cs b/src/Avalonia.Base/Controls/AdornerLayerBase.cs new file mode 100644 index 00000000000..b1e0fca75da --- /dev/null +++ b/src/Avalonia.Base/Controls/AdornerLayerBase.cs @@ -0,0 +1,23 @@ +using Avalonia.Metadata; + +namespace Avalonia.Controls; + +[PrivateApi] +public class AdornerLayerBase +{ + /// + /// Allows for getting and setting of the adorned element. + /// + public static readonly AttachedProperty AdornedElementProperty = + AvaloniaProperty.RegisterAttached("AdornedElement"); + + public static Visual? GetAdornedElement(Visual adorner) + { + return adorner.GetValue(AdornedElementProperty); + } + + public static void SetAdornedElement(Visual adorner, Visual? adorned) + { + adorner.SetValue(AdornedElementProperty, adorned); + } +} diff --git a/src/Avalonia.Base/Controls/IAdornerLayer.cs b/src/Avalonia.Base/Controls/IAdornerLayer.cs new file mode 100644 index 00000000000..d5bd462b26b --- /dev/null +++ b/src/Avalonia.Base/Controls/IAdornerLayer.cs @@ -0,0 +1,9 @@ +using Avalonia.Metadata; + +namespace Avalonia.Controls; + +[PrivateApi] +[NotClientImplementable] +public interface IAdornerLayer +{ +} diff --git a/src/Avalonia.Base/VisualExtensions.cs b/src/Avalonia.Base/VisualExtensions.cs index e8dc5465d69..f2a3d5e2276 100644 --- a/src/Avalonia.Base/VisualExtensions.cs +++ b/src/Avalonia.Base/VisualExtensions.cs @@ -1,4 +1,5 @@ using System; +using Avalonia.Controls; using Avalonia.VisualTree; namespace Avalonia @@ -52,8 +53,21 @@ public static PixelPoint PointToScreen(this Visual visual, Point point) if (common != null) { - var thisOffset = GetOffsetFrom(common, from); - var thatOffset = GetOffsetFrom(common, to); + Matrix thisOffset; + if (TryGetAdorner(from, common, out var adornedFrom, out var adornerLayerFrom)) + { + thisOffset = GetOffsetFrom(common, adornedFrom!) * GetOffsetFrom(adornerLayerFrom!, from); + } + else + thisOffset = GetOffsetFrom(common, from); + + Matrix thatOffset; + if (TryGetAdorner(to, common, out var adornedTo, out var adornerLayerTo)) + { + thatOffset = GetOffsetFrom(common, adornedTo!) * GetOffsetFrom(adornerLayerTo!, to); + } + else + thatOffset = GetOffsetFrom(common, to); if (!thatOffset.TryInvert(out var thatOffsetInverted)) { @@ -66,6 +80,29 @@ public static PixelPoint PointToScreen(this Visual visual, Point point) return null; } + private static bool TryGetAdorner(Visual target, Visual? stopAtVisual, out Visual? adorned, out Visual? adornerLayer) + { + var element = target; + while (element != null && element != stopAtVisual) + { + if (AdornerLayerBase.GetAdornedElement(element) is { } adornedElement) + { + adorned = adornedElement; + adornerLayer = element; + while (adornerLayer != null && adornerLayer is not IAdornerLayer) + { + adornerLayer = adornerLayer.VisualParent; + } + return adornerLayer != null; + } + element = element.VisualParent; + } + + adorned = null; + adornerLayer = null; + return false; + } + /// /// Translates a point relative to this visual to coordinates that are relative to the specified visual. /// diff --git a/src/Avalonia.Controls/Primitives/AdornerLayer.cs b/src/Avalonia.Controls/Primitives/AdornerLayer.cs index 412dd236ffd..96edbde9495 100644 --- a/src/Avalonia.Controls/Primitives/AdornerLayer.cs +++ b/src/Avalonia.Controls/Primitives/AdornerLayer.cs @@ -13,13 +13,13 @@ namespace Avalonia.Controls.Primitives /// /// TODO: Need to track position of adorned elements and move the adorner if they move. /// - public class AdornerLayer : Canvas + public class AdornerLayer : Canvas, IAdornerLayer { /// /// Allows for getting and setting of the adorned element. /// public static readonly AttachedProperty AdornedElementProperty = - AvaloniaProperty.RegisterAttached("AdornedElement"); + AdornerLayerBase.AdornedElementProperty.AddOwner(); /// /// Allows for controlling clipping of the adorner. diff --git a/src/Avalonia.Controls/Primitives/PopupPositioning/IPopupPositioner.cs b/src/Avalonia.Controls/Primitives/PopupPositioning/IPopupPositioner.cs index 31c1e6054a4..92d4b314b0d 100644 --- a/src/Avalonia.Controls/Primitives/PopupPositioning/IPopupPositioner.cs +++ b/src/Avalonia.Controls/Primitives/PopupPositioning/IPopupPositioner.cs @@ -468,15 +468,7 @@ static class PopupPositionerExtensions { if (target == null) throw new InvalidOperationException("Placement mode is not Pointer and PlacementTarget is null"); - Matrix? matrix; - if (TryGetAdorner(target, out var adorned, out var adornerLayer)) - { - matrix = adorned!.TransformToVisual(topLevel) * target.TransformToVisual(adornerLayer!); - } - else - { - matrix = target.TransformToVisual(topLevel); - } + Matrix? matrix = target.TransformToVisual(topLevel); if (matrix == null) { @@ -538,25 +530,6 @@ static class PopupPositionerExtensions } } } - - private static bool TryGetAdorner(Visual target, out Visual? adorned, out Visual? adornerLayer) - { - var element = target; - while (element != null) - { - if (AdornerLayer.GetAdornedElement(element) is { } adornedElement) - { - adorned = adornedElement; - adornerLayer = AdornerLayer.GetAdornerLayer(adorned); - return true; - } - element = element.VisualParent; - } - - adorned = null; - adornerLayer = null; - return false; - } } } diff --git a/tests/Avalonia.Controls.UnitTests/AdornerLayerTests.cs b/tests/Avalonia.Controls.UnitTests/AdornerLayerTests.cs new file mode 100644 index 00000000000..6ba891b87a9 --- /dev/null +++ b/tests/Avalonia.Controls.UnitTests/AdornerLayerTests.cs @@ -0,0 +1,34 @@ +using Avalonia.Controls.Primitives; +using Avalonia.UnitTests; +using Xunit; + +namespace Avalonia.Controls.UnitTests; + +public class AdornerLayerTests : ScopedTestBase +{ + [Fact] + public void Adorners_Include_Adorned_Elements_In_Transform_Visual() + { + var button = new Button() + { + Margin = new Thickness(100, 100) + }; + var root = new TestRoot() + { + Child = new VisualLayerManager() + { + Child = button + } + }; + var adorner = new Border(); + + var adornerLayer = AdornerLayer.GetAdornerLayer(button); + adornerLayer.Children.Add(adorner); + AdornerLayer.SetAdornedElement(adorner, button); + + root.LayoutManager.ExecuteInitialLayoutPass(); + + var translatedPoint = root.TranslatePoint(new Point(100, 100), adorner); + Assert.Equal(new Point(0, 0), translatedPoint); + } +}