Skip to content

Commit

Permalink
[X] Process ContentProperty before Add()
Browse files Browse the repository at this point in the history
We added IEnumerable interface, and Add() methods, to Layouts in MAUI.
As the XAML inflaters were trying to Add() before looking for a
ContentProperty,
 1/ ContentProperty on Layout (like Grid) is completely ignored (but
layouts works nonetheless, because of belt and suspenders).
 2/ overriding the ContentProperty is ignored (as found in #6944)

This PR changes the order of Add() and ContentProperty processing, and
restore the expected behavior, without breaking any other unit test.

- fixes #6944
  • Loading branch information
StephaneDelcroix committed May 10, 2022
1 parent 2a7acdc commit f1d55ed
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 24 deletions.
19 changes: 10 additions & 9 deletions src/Controls/src/Build.Tasks/SetPropertiesVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,15 @@ public void Visit(ElementNode node, INode parentNode)
Context.IL.Append(parentVar.LoadAs(Module.GetTypeDefinition(("Microsoft.Maui.Controls", "Microsoft.Maui.Controls", "ResourceDictionary")), Module));
Context.IL.Append(AddToResourceDictionary(parentVar, node, node, Context));
}
else if ((contentProperty = GetContentProperty(parentVar.VariableType)) != null)
{
var name = new XmlName(node.NamespaceURI, contentProperty);
if (skips.Contains(name))
return;
if (parentNode is IElementNode && ((IElementNode)parentNode).SkipProperties.Contains(propertyName))
return;
Context.IL.Append(SetPropertyValue(Context.Variables[(IElementNode)parentNode], name, node, Context, node));
}
// Collection element, implicit content, or implicit collection element.
else if (parentVar.VariableType.ImplementsInterface(Module.ImportReference(("mscorlib", "System.Collections", "IEnumerable")))
&& parentVar.VariableType.GetMethods(md => md.Name == "Add" && md.Parameters.Count == 1, Module).Any())
Expand All @@ -160,15 +169,7 @@ public void Visit(ElementNode node, INode parentNode)
if (adderRef.ReturnType.FullName != "System.Void")
Context.IL.Emit(Pop);
}
else if ((contentProperty = GetContentProperty(parentVar.VariableType)) != null)
{
var name = new XmlName(node.NamespaceURI, contentProperty);
if (skips.Contains(name))
return;
if (parentNode is IElementNode && ((IElementNode)parentNode).SkipProperties.Contains(propertyName))
return;
Context.IL.Append(SetPropertyValue(Context.Variables[(IElementNode)parentNode], name, node, Context, node));
}

else
throw new BuildException(BuildExceptionCode.ContentPropertyAttributeMissing, node, null, ((IElementNode)parentNode).XmlType.Name);
}
Expand Down
35 changes: 20 additions & 15 deletions src/Controls/src/Xaml/ApplyPropertiesVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,14 @@ public void Visit(ElementNode node, INode parentNode)
{
if (!Values.TryGetValue(parentNode, out var source) && Context.ExceptionHandler != null)
return;

ProvideValue(ref value, node, source, XmlName.Empty);
string contentProperty;
Exception xpe = null;

string xKey = null;
if (xpe == null && node.Properties.ContainsKey(XmlName.xKey))
if ( xpe == null
&& node.Properties.ContainsKey(XmlName.xKey))
{
if ((node.Properties[XmlName.xKey] is ValueNode valueNode))
xKey = valueNode.Value as string;
Expand All @@ -137,11 +139,26 @@ public void Visit(ElementNode node, INode parentNode)
}

//ResourceDictionary
if (xpe == null && TryAddToResourceDictionary(source as ResourceDictionary, value, xKey, node, out xpe))
if ( xpe == null
&& TryAddToResourceDictionary(source as ResourceDictionary, value, xKey, node, out xpe))
return;

//ContentProperty
if ( xpe == null
&& (contentProperty = GetContentPropertyName(Context.Types[parentElement])) != null)
{
var name = new XmlName(node.NamespaceURI, contentProperty);
if (Skips.Contains(name))
return;
if (parentElement.SkipProperties.Contains(propertyName))
return;

SetPropertyValue(source, name, value, Context.RootElement, node, Context, node);
return;
}

// Collection element, implicit content, or implicit collection element.
if (xpe == null
if ( xpe == null
&& typeof(IEnumerable).IsAssignableFrom(Context.Types[parentElement])
&& Context.Types[parentElement].GetRuntimeMethods().Any(mi => mi.Name == "Add" && mi.GetParameters().Length == 1))
{
Expand All @@ -158,17 +175,7 @@ public void Visit(ElementNode node, INode parentNode)

return;
}
if (xpe == null && (contentProperty = GetContentPropertyName(Context.Types[parentElement])) != null)
{
var name = new XmlName(node.NamespaceURI, contentProperty);
if (Skips.Contains(name))
return;
if (parentElement.SkipProperties.Contains(propertyName))
return;

SetPropertyValue(source, name, value, Context.RootElement, node, Context, node);
return;
}
xpe = xpe ?? new XamlParseException($"Can not set the content of {((IElementNode)parentNode).XmlType.Name} as it doesn't have a ContentPropertyAttribute", node);
if (Context.ExceptionHandler != null)
Context.ExceptionHandler(xpe);
Expand Down Expand Up @@ -214,8 +221,6 @@ public void Visit(ElementNode node, INode parentNode)
}
}



public void Visit(RootNode node, INode parentNode)
{
}
Expand Down
9 changes: 9 additions & 0 deletions src/Controls/tests/Xaml.UnitTests/Issues/Maui6944.xaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0" encoding="UTF-8" ?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:local="clr-namespace:Microsoft.Maui.Controls.Xaml.UnitTests"
x:Class="Microsoft.Maui.Controls.Xaml.UnitTests.Maui6944">
<local:Maui6944Layout x:Name="layout">
<Label x:Name="label"/>
</local:Maui6944Layout>
</ContentPage>
52 changes: 52 additions & 0 deletions src/Controls/tests/Xaml.UnitTests/Issues/Maui6944.xaml.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
using Microsoft.Maui.ApplicationModel;
using Microsoft.Maui.Controls.Core.UnitTests;
using Microsoft.Maui.Devices;
using NUnit.Framework;

namespace Microsoft.Maui.Controls.Xaml.UnitTests
{
public partial class Maui6944 : ContentPage
{
public Maui6944() => InitializeComponent();
public Maui6944(bool useCompiledXaml)
{
//this stub will be replaced at compile time
}

[TestFixture]
class Test
{
[SetUp] public void Setup() => AppInfo.SetCurrent(new MockAppInfo());
[TearDown] public void TearDown() => AppInfo.SetCurrent(null);

[Test]
public void ContentPropertyAttributeOnLayoutSubclass([Values(false, true)] bool useCompiledXaml)
{
var page = new Maui6944(useCompiledXaml);
Assert.That(page.layout, Is.Not.Null);
Assert.That(page.layout, Is.TypeOf<Maui6944Layout>());
Assert.That(page.layout.ChildContent, Is.EqualTo(page.label));
}
}
}

public class Maui6944Base : Grid
{
}

[ContentProperty("ChildContent")]
public class Maui6944Layout : Maui6944Base
{
public static readonly BindableProperty ChildContentProperty =
BindableProperty.Create(
nameof(ChildContent),
typeof(View), typeof(Maui6944Layout),
defaultValue: null);

public View ChildContent {
get => (View)GetValue(ChildContentProperty);
set => SetValue(ChildContentProperty, value);
}

}
}

0 comments on commit f1d55ed

Please sign in to comment.