Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AmbiguousMatchException when BindableProperty is hidden in a derived class and is then set in XAML #13962

Closed
dilyantraykov opened this issue Mar 15, 2023 · 4 comments · Fixed by #15873
Assignees
Labels
area-xaml XAML, CSS, Triggers, Behaviors fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 platform/android 🤖 platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working

Comments

@dilyantraykov
Copy link

dilyantraykov commented Mar 15, 2023

Description

Consider the following custom control which hides a BindableProperty:

public class CustomCheckBox : CheckBox
{
    public static new readonly BindableProperty IsCheckedProperty =
        BindableProperty.Create(nameof(IsChecked), typeof(bool?), typeof(CustomCheckBox), false, BindingMode.TwoWay);

    public new bool? IsChecked
    {
        get { return (bool?)this.GetValue(IsCheckedProperty); }
        set { this.SetValue(IsCheckedProperty, value); }
    }
}

Notice that the type of the new property (bool?) differs from that of the default property (bool).

When trying to set the new property in XAML, an AmbiguousMatchException is thrown with the following message:
"Multiple properties with name 'CustomCheckBox.IsChecked' found."

Setting the property in code-behind works as expected.

The exception seems to stem from the TrySetValue method of the ApplyPropertiesVisitor which uses the GetRuntimeProperty method. Actually, calling the method in the following manner is also sufficient to reproduce the exception:

this.checkBox.GetType().GetRuntimeProperty("IsChecked");

Possible fix would be to use the GetRuntimeProperties method instead and then check the DeclaringType property to retrieve only a single property.

var properties = this.checkBox.GetType().GetRuntimeProperties();
var property = properties.FirstOrDefault(x => x.Name == "IsChecked" && x.DeclaringType == this.checkBox.GetType());

Note: Removing the "new" keyword does not resolve the exception.

Steps to Reproduce

  1. Open the project from the referenced reproduction repository.
  2. Try running the project.

Expected: The project runs successfully and the CheckBox is displayed.
Actual: The application crashes with an AmbiguousMatchException.

Link to public reproduction project repository

https://github.com/telerik/ms-samples/tree/main/Maui/HiddenBindableProperty

Version with bug

7.0 (current)

Last version that worked well

Unknown/Other

Affected platforms

Android, Windows

Affected platform versions

Windows SDK 10.0.19041.0, Android 33

Did you find any workaround?

No response

Relevant log output

No response

@dilyantraykov dilyantraykov added the t/bug Something isn't working label Mar 15, 2023
@jsuarezruiz jsuarezruiz added the area-xaml XAML, CSS, Triggers, Behaviors label Mar 15, 2023
@jsuarezruiz
Copy link
Contributor

@StephaneDelcroix Thoughts?

@jsuarezruiz jsuarezruiz added this to the Backlog milestone Mar 15, 2023
@ghost
Copy link

ghost commented Mar 15, 2023

We've added this issue to our backlog, and we will work to address it as time and resources allow. If you have any additional information or questions about this issue, please leave a comment. For additional info about issue management, please read our Triage Process.

@StephaneDelcroix
Copy link
Contributor

@StephaneDelcroix Thoughts?

looks valid. SetValue doesn't set the c# property though, but the BP. But it still looks for the c# prop to get the type and attributes, and the ambiguous match comes from there

@StephaneDelcroix StephaneDelcroix self-assigned this Mar 16, 2023
@Zhanglirong-Winnie Zhanglirong-Winnie added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Jun 20, 2023
@Zhanglirong-Winnie
Copy link

Verified this issue with Visual Studio Enterprise 17.7.0 Preview 2.0. Can repro on android and windows platform with sample project.
https://github.com/telerik/ms-samples/tree/main/Maui/HiddenBindableProperty
Screenshot 2023-06-20 170838

StephaneDelcroix added a commit that referenced this issue Jun 27, 2023
with XamlC disabled, setting an overriden property throws a
AmbiguousMatchException. This fixes the mismatch between the 2
inflaters.

- fixes #13962
@samhouts samhouts modified the milestones: Backlog, Under Consideration Jul 26, 2023
github-actions bot pushed a commit that referenced this issue Aug 15, 2023
with XamlC disabled, setting an overriden property throws a
AmbiguousMatchException. This fixes the mismatch between the 2
inflaters.

- fixes #13962
jfversluis pushed a commit that referenced this issue Aug 15, 2023
with XamlC disabled, setting an overriden property throws a
AmbiguousMatchException. This fixes the mismatch between the 2
inflaters.

- fixes #13962
rmarinho pushed a commit that referenced this issue Aug 19, 2023
with XamlC disabled, setting an overriden property throws a
AmbiguousMatchException. This fixes the mismatch between the 2
inflaters.

- fixes #13962
@samhouts samhouts added the fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 label Sep 12, 2023
@samhouts samhouts modified the milestones: Under Consideration, .NET 8 + Servicing Sep 19, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-xaml XAML, CSS, Triggers, Behaviors fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 platform/android 🤖 platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants