The Danger of Assigning Event Handlers in XAML

I recently discovered a nasty little “WPF gotcha.”  This issue makes me question whether one should ever hook events of an element in XAML.  Basically the problem arises if you hook an element’s event in XAML, set a property of the element which will cause the event to be raised (also in XAML), and in the event handling method reference another element which is declared after that element.  You will find that the reference to the other element is null, because the BAML deserialization process hasn’t gotten around to creating that other element yet.

Here’s a stupid example which shows the problem in action.  It allows you to pick a word from a ComboBox, and then displays that word in reverse on a TextBlock beneath the ComboBox.  The SelectionChanged event of the combo is hooked in XAML so that we can execute the code which reverses the text and display it on the TextBlock (in a real application this type of data manipulation should be done in a value converter).  Also note that the ComboBox’s SelectedIndex is set in XAML, which causes the SelectionChanged event to be raised during the BAML deserialization process.

EventHandlersInXAML (markup)

Here’s the OnComboSelectionChanged event handling method, which is referenced in the XAML seen before:

EventHandlersInXAML (handler)

To fix the problem, we can remove the event handler assignment from the XAML and replicate it in code, as seen below:

EventHandlersInXAML (ctor)

You would also have to execute the logic which transfers the reversed text to the TextBlock at some point as the Window loads, to handle the initial value of the ComboBox.

Other ways to fix the problem include: set the ComboBox’s SelectedIndex property after the call to InitializeComponent in the constructor, or declare the TextBlock above the ComboBox in XAML, etc.

Download the demo application here: EventHandlersInXAML (demo project)  Be sure to change the file extension from .DOC to .ZIP and then decompress it.

About these ads

13 Responses to The Danger of Assigning Event Handlers in XAML

  1. Josh,

    For us VB.NET types it gets worse. In addition to your findins we also have to deal with this condition.

    If we set up event handler for the SelectionChanged event and assign the handler using the Handles statement on the SelectedChanged event and reference any controls in the Window, UserControl, etc. we MUST run a check against the referenced control to check if its Nothing. If the control is Nothing, then exit sub because all the controls are not fully warmed up yet.

    So I find myself using AddHandler in the Loaded event for the SelectionChanged event.

    You can also get some strange behavior when a UserControl with the SelectionChanged event is in a TabItem that is declared in XAML.

    Get posting and find,

    Karl

  2. Josh,

    Forgot to mention. That demo code I sent today, actually has the VB.NET workaround for using the Handles Statement. Its in Window1.xaml.vb.

    When I put the custom control into my Code Camp presentation this evening, I moved my AddHandler to the loaded event so I didn’t have to look at hacky code.

    Cheers,

    Karl

  3. David says:

    Glad to see this blogged about. I run into this constantly. I typically do both 1) initialization code after InitializeComponent() and 2) checks for null controls within the event handler. In my experience, #1 can get very confusing with multiple property dependencies and binding mixed in.

    Thanks for the tip on the BAML deserialization event fire…always wondered where that extra call was coming from.

    -David

  4. You know, a more elegant solution to the problem would not be to remove the event bindings from the XAML code, but to actually adopt a full separation of concerns. For example, rather than explicitly setting the text of a control that has not yet been parsed by the XAML parser, you could simply modify the text property on your model object (appropriate in MVC), and then your text control would be notified of the change by virtue of proper data binding to a model.

    To me, the notion of yanking event handlers from XAML for the purpose you describe is just enabling tight coupling. With a true MVC approach, you don’t run into this problem.

  5. Josh Smith says:

    .NET Addict,

    I agree with you. The demo shown in this post is remarkably stupid and small, but that’s a necessary evil when concoting such demos. The point here is not to show how to “properly” design a UI in WPF, but just to show the “gotcha” I stumbled across.

    On a tangent, I don’t agree that making your designs MVC-friendly is always a good goal either. In some cases using MVC is just plain overkill.

    Thanks for the feedback,
    Josh

  6. arkhivania says:

    Josh, change properties inside SynchronizationContext.Current.Post delegate, and all will be fine.

  7. rei says:

    What do you think it’d take for Microsoft to fix this for an upcoming version?

    They could simply queue events until the end of InitializeComponent(), but this might mean that they have to identify which of the events should be fired afterwards, and which should be fired on the spot, ie, Loaded might need to be fired then and there, whereas things like SelectionChanged would be best done later. Not sure on that though; just a suspicion.

    Then there are the logistical problems of getting in such a fix, since they’d probably have to dig deep into DependencyObject and RoutedEvent to add a fix for this. Also, I can’t think of any ways it would break any existing products, but the .NET team (especially WinForms) is a bit notorious for using that as an excuse to not fix a lot of things.

    I don’t think this is a big deal for us WPF devs since we’re used to tinkering with code to do sanity checks, but this could be a real nuisance for some Silverlight devs. For them it’s probably more than just MVC aesthetics. And we all know that angry web devs means bad adoption.

  8. Josh Smith says:

    rei,

    I don’t see this as a bug which needs to be fixed by Microsoft. The same “problem” existed in WinForms, where InitComponent was executing lines of code instead of deserializing baml. It’s just something you need to be aware of, so that you don’t do it. There are plenty of ways to organize your code so that this “gotcha” does not affect you.

    Josh

  9. hypersw says:

    It’s not quite the “BAML decerialization process”, it’s a piece of explicit code in your Window1 class that attaches the event:

    void System.Windows.Markup.IComponentConnector.Connect(int connectionId, object target) {
    switch (connectionId)
    {
    case 1:
    this.combo = ((System.Windows.Controls.ComboBox)(target));
    this.combo.SelectionChanged += new System.Windows.Controls.SelectionChangedEventHandler(this.OnComboSelectionChanged);
    return;
    case 2:
    this.reversedTextBlk = ((System.Windows.Controls.TextBlock)(target));
    return;
    }
    this._contentLoaded = true;
    }

    Of course it could execute more flexibility in first assigning all the fields, then sinking the events.

    This example in the demo app (why is it a .doc anyway?) just demonstrates that procedural code is poorly suited for implementing data flow tasks. The selected item could just flow into the text box with a transform in the middle, which would take as much code as one {Binding} in markup and one class with one method in codebehind.

    For those events-that-really-should-be-handled-as-events, it’s not so bad.

  10. Josh Smith says:

    hypersw,

    The Connect method in the generated code-behind file is invoked during the BAML deserialization process.

    The demo app file is zipped to renamed to be a .DOC because WordPress does not allow me to upload .ZIP files.

    Thanks,
    Josh

  11. Louis Duran says:

    Holy Smokes Josh!

    I think you just pointed out a problem I was having that I couldn’t figure out the cause of. I handled it in code by trapping the event and through various checks I could determine if the OnSelectionChanged event was being done by the user or not. I now need to go back and examine that code and validate my assumption. Thanks for posting this…

  12. Laurent says:

    Hi Josh,

    Interesting post, though I don’t really see this as an issue. In the event handler, you should be testing the textbox to see if it’s null anyway, shouldn’t you? It’s good to be aware of the order of initialization of the elements, and the order in which the events get called. I think that changing it would be dangerous, because this behavior (the event handlers being called immediately) may be needed for other functionalities (I know you don’t say it should be changed, I agree with you).

    The problem is similar in ASP.NET (and as you mention, it is the same in WinForms too).

    Greetings,
    Laurent

  13. […] WPF – Scrolling a treeview and XAML event handler dangers. […]

Follow

Get every new post delivered to your Inbox.

Join 289 other followers

%d bloggers like this: