VB -> C#: Event handler assignments are placed above InitializeComponent

[HeinziAT]

Repro instructions

1. Create a new VB WPF Application (.NET Framework) project.
2. Fill MainWindow.xaml as follows:

<Window x:Class="MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        Title="MainWindow" Height="450" Width="800">
    <Button x:Name="btn" />
</Window>

3. Fill MainWindow.xaml.vb as follows:

Class MainWindow
    Private Sub MainWindow_Loaded(sender As Object, e As RoutedEventArgs) Handles Me.Loaded
        MsgBox("Window loaded")
    End Sub

    Private Sub btn_Click(sender As Object, e As RoutedEventArgs) Handles btn.Click
        MsgBox("Button clicked")
    End Sub
End Class

4. Run the program and note that everythings works as expected: The first message box is shown immediately, and the second one upon clicking the (full-size) button.
5. Convert the project to C# using the Visual Studio Extension.
6. Fix the namespace in the XAML file (depends on your project name, in my case it was x:Class="MainWindow" -> x:Class="WpfApp1.MainWindow").
7. Run the program.

Erroneous output

Neither the window's nor the button's event handler gets called, because (a) the XAML file is left unmodified and (b) the Handles clauses from the VB code are gone without replacement. This is what MainWindow.xaml.cs looks like:

using System.Windows;
using Microsoft.VisualBasic;

namespace WpfApp1
{
    public partial class MainWindow
    {
        public MainWindow()
        {
            InitializeComponent();
        }
        private void MainWindow_Loaded(object sender, RoutedEventArgs e)
        {
            Interaction.MsgBox("Window loaded");
        }

        private void btn_Click(object sender, RoutedEventArgs e)
        {
            Interaction.MsgBox("Button clicked");
        }
    }
}

Expected output

There are multiple ways to fix this:

Solution 1: Add the event handler names to the XAML file. This would be the most idiomatic solution, but requires parsing and modifying the XAML file, which is probably a lot of work.

<Window x:Class="WpfApp1.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        Title="MainWindow" Height="450" Width="800"
        Loaded="MainWindow_Loaded">
    <Button x:Name="btn" Click="btn_Click" />
</Window>

Solution 2: Add the event handlers in code in the constructor.

        public MainWindow()
        {
            InitializeComponent();

            this.Loaded += MainWindow_Loaded;
            btn.Click += btn_Click;
        }

Solution 3: Inform the user that there is still work that needs to be done:

        // TODO: Attach the event handler to its event. VB version was: "Handles Me.Loaded"
        private void MainWindow_Loaded(object sender, RoutedEventArgs e)
        {
            Interaction.MsgBox("Window loaded");
        }

        // TODO: Attach the event handler to its event. VB version was: "Handles btn.Click"
        private void btn_Click(object sender, RoutedEventArgs e)
        {
            Interaction.MsgBox("Button clicked");
        }

Details

• Product in use: VS extension
• Version in use: 9.0.4.0

[GrahamTheCoder]

Thanks for the full analysis of the situation. I agree that changing the xaml would be the neatest solution, but as you say, probably lots of work.

Event initialization

Like solution 2 - the assignment to the field should have an event handler hookup appended, either in InitializeComponent or the constructor depending on the situation. I'll try to look into why this hasn't happened

Events on fields

The monstrosity of a property shown below replaces a field so that when it's assigned, the event handler is hooked up (as happens in VB). But that only happens when there is an assignment to the field other than the initialization. So it's probably ok/good that this part is skipped in the example given.

CodeConverter/Tests/CSharp/MemberTests/EventMemberTests.cs

Lines 632 to 654 in da720fa

	internal virtual System.Windows.Forms.Button Button1
	{
	[MethodImpl(MethodImplOptions.Synchronized)]
	get
	{
	return _Button1;
	}
	[MethodImpl(MethodImplOptions.Synchronized)]
	set
	{
	if (_Button1 != null)
	{
	_Button1.Click -= MultiClickHandler;
	}
	_Button1 = value;
	if (_Button1 != null)
	{
	_Button1.Click += MultiClickHandler;
	}
	}
	}

[HeinziAT]

The monstrosity of a property shown below replaces a field so that when it's assigned, the event handler is hooked up (as happens in VB). But that only happens when there is an assignment to the field other than the initialization. So it's probably ok/good that this part is skipped in the example given.

I agree. It's theoretically possible to re-assign the auto-generated WPF UI control fields (such as btn), since they are not readonly (probably so that they can to be initialized in InitializeComponent rather than in the constructor), but I can't think of a sane use case for doing that.

[GrahamTheCoder]

I've found that if people use the WPF designer to add events, they get added in the xaml even in VB, so people will only face this issue if manually adding Handles statements. So yeah if it's slightly rarer that's even less reason to dabble with mutating xaml.

It looks like the first part of the reason the event hookups aren't added is to do with partial classes that WPF generates in the obj folder to represent the xaml. This gets selected as the best place to add the initialization. I'll see if I can stop that happening.
(The files have the .g.i.vb and .g.vb extensions if you're interested)

[HeinziAT]

The "Handles" version of the event handlers gets generated automatically if you choose the UI element and the event from the drop down menus in the code window (like you did in VB6, and like you still do in VBA - I guess old habits die hard):

[GrahamTheCoder]

Ah I see yeah, that's a pity. I tried it from the Properties window in the designer which adds it the xaml, but yeah I'm sure lots of people will use that dropdown too.

So I think the overall fix here is to have a fallback which puts these events in the constructor when none of the other locations are viable (e.g. Because in a temp file, or in a field initializer might trigger this issue too).

[GrahamTheCoder]

I've put in PR #968 which hopefully covers the main issues here, but haven't managed to run full regression tests yet

[HeinziAT]

Thanks. Sorry for not testing this earlier, it took a while until I had another WPF project to convert.

The current version (9.1.3.0) yields the following C# output:

    public partial class MainWindow
    {
        public MainWindow()
        {
            this.Loaded += MainWindow_Loaded;
            btn.Click += btn_Click;
        }
        private void MainWindow_Loaded(object sender, RoutedEventArgs e)
        {
            Interaction.MsgBox("Window loaded");
        }

        private void btn_Click(object sender, RoutedEventArgs e)
        {
            Interaction.MsgBox("Button clicked");
        }
    }

If the VB file already contains a constructor calling InitializeComponent(), the following C# code is generated instead:

        public MainWindow()
        {
            this.Loaded += MainWindow_Loaded;
            btn.Click += btn_Click;
            this.InitializeComponent();
        }

Both are incorrect and yield a NullReferenceException (because btn is null): Since InitializeComponent() creates the UI elements, the correct output would be:

        public MainWindow()
        {
            this.InitializeComponent();

            this.Loaded += MainWindow_Loaded;
            btn.Click += btn_Click;
        }

[GrahamTheCoder]

Thanks, I've reopened this and will try to take another look when I get time