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.

Sign up for GitHub

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

Jump to bottom

Private EventHandlers throw an exception on XAML-defined custom control, inherited by other control #11467

Closed
maiia-kuzmishyna opened this issue Nov 18, 2022 · 5 comments · Fixed by #17075
Closed

Private EventHandlers throw an exception on XAML-defined custom control, inherited by other control #11467

maiia-kuzmishyna opened this issue Nov 18, 2022 · 5 comments · Fixed by #17075
Assignees
@jonathanpeppers
Labels
area-xaml XAML, CSS, Triggers, Behaviors fixed-in-8.0.0-rc.2.9373 Look for this fix in 8.0.0-rc.2.9373! p/1 Work that is critical for the release, but we could probably ship without partner/cat 😻 Client CAT Team platform/android 🤖 platform/iOS 🍎 platform/windows 🪟 s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone
.NET 8 GA

Comments

@maiia-kuzmishyna
Copy link

Description

When I define a custom control in XAML that subscribes to some event handlers and those event handlers are private (ParentControl), and then I have a different control inheriting ParentControl (ChildControl) -
Application crashes with exception:
"No method ParentButton_OnClicked with correct signature found on type ProtectedEventHandlers.ChildButton".
If I just use ParentButton instead of ChildButton, or make event handler protected - works without errors.

image

Steps to Reproduce

  1. Create a blank MAUI App project
  2. Define a control in XAML that inherits f.e. Button: ParentButton
  3. Subscribe to Clicked event in XAML of ParentButton
  4. Define a control (XAML or C#) ChildButton that inherits from ParentButton
  5. Use ChildButton f.e. in MainPage
  6. Deploy the app

Link to public reproduction project repository

https://github.com/maiia-kuzmishyna/MAUI-ProtectedEventHandlers

Version with bug

6.0.400

Last version that worked well

Unknown/Other

Affected platforms

iOS, Android, Windows

Affected platform versions

iOS 15,16; Windows 11; Android 31

Did you find any workaround?

Make Event Handler protected instead of private.

Relevant log output

No response
@maiia-kuzmishyna maiia-kuzmishyna added the t/bug Something isn't working label Nov 18, 2022
@jsuarezruiz jsuarezruiz added the area-xaml XAML, CSS, Triggers, Behaviors label Nov 18, 2022
@rachelkang rachelkang added this to the Backlog milestone Nov 18, 2022
@ghost
Copy link

ghost commented Nov 18, 2022

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@rachelkang
Copy link
Member

cc @mattleibow

@mattleibow
Copy link
Member

This seems interesting @StephaneDelcroix... It appears the derived type is looking to attach the event from the derived type to the base, but the event handler is in the base...

Not sure if that makes sense, but the repro shows an exception for me in the latest net7.0 code too.

@mikeparker104 mikeparker104 added the partner/cat 😻 Client CAT Team label Nov 23, 2022
@samhouts samhouts added the p/1 Work that is critical for the release, but we could probably ship without label Dec 17, 2022
@samhouts samhouts modified the milestones: Backlog, .NET 8 Planning Jan 26, 2023
@samhouts samhouts modified the milestones: .NET 8 Planning, .NET 8 May 25, 2023
@XamlTest XamlTest added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Jun 6, 2023
@XamlTest
Copy link
Collaborator

XamlTest commented Jun 6, 2023

Verified this on Visual Studio Enterprise 17.7.0 Preview 1.0. Repro on Windows 11.

Windows: Application crashes with exception
image

Android 13.0-API33: Application works well.
image

iOS 16.4: Application deploy successfully, but there is no response when click "Parent Button" button.
image

@StephaneDelcroix
Copy link
Contributor

This seems interesting @StephaneDelcroix... It appears the derived type is looking to attach the event from the derived type to the base, but the event handler is in the base...

Not sure if that makes sense, but the repro shows an exception for me in the latest net7.0 code too.

If I understand this correctly, you would get the same result from c#. if you want to access methods from derived types, private isn't the keyword you're after.

@samhouts samhouts modified the milestones: .NET 8, .NET 8 GA Jul 12, 2023
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this issue Aug 29, 2023
@jonathanpeppers
[xaml] locate event handlers from base types
77b0f2d 
Fixes: dotnet#11467
Context: https://github.com/maiia-kuzmishyna/MAUI-ProtectedEventHandlers

Reviewing the customer sample, it appears to be a valid case. Consider a
`ChildButton` defined in XAML:

    <local:ParentButton
        xmlns="http://xamarin.com/schemas/2014/forms"
        xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
        xmlns:local="clr-namespace:foo"
        x:Class="foo.ChildButton">
    </local:ParentButton>

Where `ParentButton` is also defined in XAML:

    <Button
        xmlns="http://xamarin.com/schemas/2014/forms"
        xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
        x:Class="foo.ParentButton"
        Clicked="ParentButton_OnClicked">
    </Button>

Where `ParentButton_OnClicked` is of course private:

    private void ParentButton_OnClicked(object sender, EventArgs e) { }

Throws an exception at runtime with:

    No method ParentButton_OnClicked with correct signature found on type foo.ChildButton

What was also odd, it appears the problem just "goes away" in Release
mode, meaning it works under XamlC. This kind of points to a bug with
non-compiled XAML.

It appears the problem was the code:

    foreach (var mi in rootElement.GetType().GetRuntimeMethods())
    {
        //...
        if (mi.Name == (string)value)
        {
            addMethod.Invoke(element, new[] { mi.CreateDelegate(eventInfo.EventHandlerType, mi.IsStatic ? null : rootElement) });
            return true;
        }
        //...
    }

In this example, `rootElement` is of type `ChildButton` while the method
is on type `ParentButton`. As mentioned on:

https://stackoverflow.com/a/2267299

You need to access `Type.BaseType` to find private methods on base types.

I changed this to instead:

* Iterate over `rootElement.GetType()` and its base types.

* Call `Type.GetMethod()` using the method name, passing the appropriate
  `BindingFlags`.

This appears to make the new test pass, solving the issue.
@jonathanpeppers jonathanpeppers mentioned this issue Aug 29, 2023
Merged
@jonathanpeppers jonathanpeppers self-assigned this Aug 29, 2023
rmarinho added a commit that referenced this issue Sep 4, 2023
@rmarinho
[xaml] locate event handlers from base types (#17075)
c2626ba 
Fixes #11467
Context: https://github.com/maiia-kuzmishyna/MAUI-ProtectedEventHandlers

Reviewing the customer sample, it appears to be a valid case. Consider a
`ChildButton` defined in XAML:

    <local:ParentButton
        xmlns="http://xamarin.com/schemas/2014/forms"
        xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
        xmlns:local="clr-namespace:foo"
        x:Class="foo.ChildButton">
    </local:ParentButton>

Where `ParentButton` is also defined in XAML:

    <Button
        xmlns="http://xamarin.com/schemas/2014/forms"
        xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
        x:Class="foo.ParentButton"
        Clicked="ParentButton_OnClicked">
    </Button>

Where `ParentButton_OnClicked` is of course private:

    private void ParentButton_OnClicked(object sender, EventArgs e) { }

Throws an exception at runtime with:

No method ParentButton_OnClicked with correct signature found on type
foo.ChildButton

What was also odd, it appears the problem just "goes away" in Release
mode, meaning it works under XamlC. This kind of points to a bug with
non-compiled XAML.

It appears the problem was the code:

    foreach (var mi in rootElement.GetType().GetRuntimeMethods())
    {
        //...
        if (mi.Name == (string)value)
        {
addMethod.Invoke(element, new[] {
mi.CreateDelegate(eventInfo.EventHandlerType, mi.IsStatic ? null :
rootElement) });
            return true;
        }
        //...
    }

In this example, `rootElement` is of type `ChildButton` while the method
is on type `ParentButton`. As mentioned on:

https://stackoverflow.com/a/2267299

You need to access `Type.BaseType` to find private methods on base
types.

I changed this to instead:

* Iterate over `rootElement.GetType()` and its base types.

* Call `Type.GetMethod()` using the method name, passing the appropriate
`BindingFlags`.

This appears to make the new test pass, solving the issue.
tj-devel709 pushed a commit to tj-devel709/maui that referenced this issue Sep 8, 2023
@jonathanpeppers @tj-devel709
[xaml] locate event handlers from base types
fb10198 
Fixes: dotnet#11467
Context: https://github.com/maiia-kuzmishyna/MAUI-ProtectedEventHandlers

Reviewing the customer sample, it appears to be a valid case. Consider a
`ChildButton` defined in XAML:

    <local:ParentButton
        xmlns="http://xamarin.com/schemas/2014/forms"
        xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
        xmlns:local="clr-namespace:foo"
        x:Class="foo.ChildButton">
    </local:ParentButton>

Where `ParentButton` is also defined in XAML:

    <Button
        xmlns="http://xamarin.com/schemas/2014/forms"
        xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
        x:Class="foo.ParentButton"
        Clicked="ParentButton_OnClicked">
    </Button>

Where `ParentButton_OnClicked` is of course private:

    private void ParentButton_OnClicked(object sender, EventArgs e) { }

Throws an exception at runtime with:

    No method ParentButton_OnClicked with correct signature found on type foo.ChildButton

What was also odd, it appears the problem just "goes away" in Release
mode, meaning it works under XamlC. This kind of points to a bug with
non-compiled XAML.

It appears the problem was the code:

    foreach (var mi in rootElement.GetType().GetRuntimeMethods())
    {
        //...
        if (mi.Name == (string)value)
        {
            addMethod.Invoke(element, new[] { mi.CreateDelegate(eventInfo.EventHandlerType, mi.IsStatic ? null : rootElement) });
            return true;
        }
        //...
    }

In this example, `rootElement` is of type `ChildButton` while the method
is on type `ParentButton`. As mentioned on:

https://stackoverflow.com/a/2267299

You need to access `Type.BaseType` to find private methods on base types.

I changed this to instead:

* Iterate over `rootElement.GetType()` and its base types.

* Call `Type.GetMethod()` using the method name, passing the appropriate
  `BindingFlags`.

This appears to make the new test pass, solving the issue.
@dotnet dotnet locked as resolved and limited conversation to collaborators Oct 4, 2023
@samhouts samhouts added the fixed-in-8.0.0-rc.2.9373 Look for this fix in 8.0.0-rc.2.9373! label Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@jonathanpeppers jonathanpeppers

Labels
area-xaml XAML, CSS, Triggers, Behaviors fixed-in-8.0.0-rc.2.9373 Look for this fix in 8.0.0-rc.2.9373! p/1 Work that is critical for the release, but we could probably ship without partner/cat 😻 Client CAT Team platform/android 🤖 platform/iOS 🍎 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
Milestone
.NET 8 GA
Development

Successfully merging a pull request may close this issue.

[xaml] locate event handlers from base types jonathanpeppers/maui
9 participants
@StephaneDelcroix @jonathanpeppers @mattleibow @jsuarezruiz @samhouts @mikeparker104 @rachelkang @XamlTest @maiia-kuzmishyna