Skip to content

Commit

Permalink
fix: Swallow Style setter exceptions as in WinUI
Browse files Browse the repository at this point in the history
  • Loading branch information
Youssef1313 committed Jan 29, 2024
1 parent 420bf38 commit e34b703
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 8 deletions.
25 changes: 20 additions & 5 deletions src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_Style.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,28 +8,43 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Private.Infrastructure;
using Microsoft.UI.Xaml;
using Microsoft.UI.Xaml.Data;

namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml
{
// The attribute is required when running WinUI. See:
// https://github.com/microsoft/microsoft-ui-xaml/issues/4723#issuecomment-812753123
[Bindable]
public sealed partial class ThrowingElement : FrameworkElement
{
public ThrowingElement() => throw new Exception("Inner exception");
}

[TestClass]
public class Given_Style
{
#if !WINAPPSDK // Control template does not support lambda parameter
[TestMethod]
[RunsOnUIThread]
public void When_StyleFailsToApply()
{
var controlTemplate = (ControlTemplate)XamlReader.Load("""
<ControlTemplate xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:local="using:Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml">
<local:ThrowingElement />
</ControlTemplate>
""");

var style = new Style()
{
Setters =
{
new Setter(ContentControl.TemplateProperty, new ControlTemplate(() => throw new Exception("Inner exception")))
new Setter(ContentControl.TemplateProperty, controlTemplate)
}
};

var e = Assert.ThrowsException<Exception>(() => new ContentControl() { Style = style });
Assert.AreEqual("Inner exception", e.Message);
// This shouldn't throw.
_ = new ContentControl() { Style = style };
}
#endif
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1582,8 +1582,11 @@ public async Task When_Unbound_FullFlyout()
new Setter(FlyoutPresenter.BorderThicknessProperty, new Thickness(0)),

// remove limit from default style
new Setter(FlyoutPresenter.MaxWidthProperty, double.PositiveInfinity),
new Setter(FlyoutPresenter.MaxHeightProperty, double.PositiveInfinity),
// Note that NaN isn't allowed to be set for MaxWidth and MaxHeight. However,
// WinUI swallows failure HResults specifically when applying style setters.
// So, Uno catches the exception as well.
new Setter(FlyoutPresenter.MaxWidthProperty, double.NaN),
new Setter(FlyoutPresenter.MaxHeightProperty, double.NaN),

// full stretch
new Setter(FlyoutPresenter.HorizontalAlignmentProperty, HorizontalAlignment.Stretch),
Expand Down
25 changes: 24 additions & 1 deletion src/Uno.UI/UI/Xaml/Style/Style.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,30 @@ internal void ApplyTo(DependencyObject o, DependencyPropertyValuePrecedences pre
{
for (var i = 0; i < _flattenedSetters.Length; i++)
{
_flattenedSetters[i].ApplyTo(o);
try
{
_flattenedSetters[i].ApplyTo(o);
}
catch (Exception)
{
if (this.Log().IsEnabled(LogLevel.Warning))
{
this.Log().LogWarning($"An exception occurred while applying style setter.");
}

// Ideally, this should be an empty catch, keeping parity equivalent to WinUI's IGNOREHR in https://github.com/microsoft/microsoft-ui-xaml/blob/93742a178db8f625ba9299f62c21f656e0b195ad/dxaml/xcp/core/core/elements/framework.cpp#L790
// However, Later when default style is applied (using ImplicitStyle precedence), we don't observe that value in WinUI.
// As a workaround, we force-set the current value to Local precedence.
// Note: This workaround can be removed as long as When_Unbound_FullFlyout is passing.
// Maybe we are applying default styles later than we should?
// For now, this is very uncommon code path for already bad code that throws.
if (_flattenedSetters[i] is Setter { Property: { } dp })
{
var value = o.GetValue(dp);
this.Log().LogWarning($"Force-setting local value to '{value}'.");
o.SetValue(dp, value);
}
}
}
}

Expand Down

0 comments on commit e34b703

Please sign in to comment.