SourceName In MultiTriggers in WPF Themes

There is a bug in the WPF code that can cause a null pointer exception.  The null pointer exception occurs on line 5924 of System.Windows.StyleHelper.cs :

object evaluationValue = evaluationNode.GetValue( conditions[i].Property );

The problem has to do when you are defining a MultiTrigger on an element in a theme.  This bug manifested itself when on our project we were refreshing a collection that was bound to a TreeViewer and the TreeViewItem style had a MultiTrigger in it.  The style in question comes from the WPFThemes project and was the ExpressionDark Theme (the problem occurs in about 7 of the other themes in the project). The one Condition on the MultiTrigger in the theme has a SourceName defined for it.  The SourceName was not needed because it was the element right inside the root element anyway.  This bug was especially subtle because it would only manifest itself after other conditions would have happened that would cause the MultiTrigger to fire.

 I suspect, but have not verified, that the precondition for causing this bug is having a currently selected TreeViewItem that is removed upon refreshing the control and a newly generated item in the refresh was being set as the selected item in its stead.  The trigger was checking the condition but the new item hadn't fully been generated so when it was looking for the child element it didn't yet exist.  This makes sense in context  of the code in the StyleHelper class.  The code that returns the element using the GetChild method defined on line 6388.  The comment inside the method says:


 // Notice that if we are requesting a childIndex that hasn't been
 // instantiated yet we return null. This could happen when we are 
 // invalidating the dependents for a property on a TemplateNode and
 // the dependent properties are meant to be on template nodes that
 // haven't been instantiated yet.


This method was returning null because


if (styledChildren == null || childIndex > styledChildren.Count)
was evaluating to true because styledChilderen was null.

The null pointer exception was occurring because line 5924


object evaluationValue = evaluationNode.GetValue( conditions[i].Property );



was assuming evaluationNode as not null.

It was assuming it could do this because of the Debug.Assert on line 5903:


 Debug.Assert(evaluationNode != null, 
                    "Couldn't find the node corresponding to the ID and name given in the trigger.  This should have been caught somewhere upstream, like StyleHelper.SealTemplate()." );


This brings up a point about using Debug.Asserts in code.  People often use them (I see it on my project often) when they don't want to perform the null check overhead or other conditions that they feel should never happen.  The idea is that you should catch any of these cases in testing.  Obviously this is a case that was over looked in Microsoft's testing.  The method that set the value of evaluationNode before the Assert has a perfectly legitimate reason for returning null (see the comments earlier) then its not reasonable to use the Assert here.  More on a future post about using Asserts versus null checks.

The work around for this bug was to remove the SourceName from the MultiTrigger.  This does not change the style in a meaningful way and removes the null pointer exception