CommunityToolkit / WindowsCommunityToolkit

The Windows Community Toolkit is a collection of helpers, extensions, and custom controls. It simplifies and demonstrates common developer tasks building .NET apps with UWP and the Windows App SDK / WinUI 3 for Windows 10 and Windows 11. The toolkit is part of the .NET Foundation.
https://docs.microsoft.com/windows/communitytoolkit/
Other
5.82k stars 1.37k forks source link

FocusTracker crashes with NullReferenceException when `IsActive="True"` #4015

Open Marv51 opened 3 years ago

Marv51 commented 3 years ago

FocusTracker crashes with NullReferenceException if the IsActive="True" property is set in XAML.

I'm on WinUI 3.0 (desktop, Reunion 0.5.6) with Community Toolkit 7.0.1

Stacktrace points to ClearContent.

I think this is occurring because the property is set before the template is finished applying. If I set IsActive=true at a later point, for example from a button click event it works as intended.

For me this issue reproduces easily and reliably, let me know if more info is needed.

Full Exception:

System.NullReferenceException: 'Object reference not set to an instance of an object.'

This exception was originally thrown at this call stack:
    CommunityToolkit.WinUI.DeveloperTools.FocusTracker.ClearContent()
    CommunityToolkit.WinUI.DeveloperTools.FocusTracker.FocusOnControl(Microsoft.UI.Xaml.FrameworkElement)
    CommunityToolkit.WinUI.DeveloperTools.FocusTracker.Start()
    CommunityToolkit.WinUI.DeveloperTools.FocusTracker.OnIsActiveChanged(Microsoft.UI.Xaml.DependencyObject, Microsoft.UI.Xaml.DependencyPropertyChangedEventArgs)
    ABI.Microsoft.UI.Xaml.PropertyChangedCallback.Do_Abi_Invoke.AnonymousMethod__0(Microsoft.UI.Xaml.PropertyChangedCallback)
    WinRT.ComWrappersSupport.MarshalDelegateInvoke<T>(System.IntPtr, System.Action<T>)
    ABI.Microsoft.UI.Xaml.PropertyChangedCallback.Do_Abi_Invoke(System.IntPtr, System.IntPtr, System.IntPtr)
    System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
    WinRT.ExceptionHelpers.ThrowExceptionForHR(int)
    ABI.Microsoft.UI.Xaml.IDependencyObject.Microsoft.UI.Xaml.IDependencyObject.SetValue(Microsoft.UI.Xaml.DependencyProperty, object)
    ...
    [Call Stack Truncated]

XAML:

xmlns:developerTools="using:CommunityToolkit.WinUI.DeveloperTools"
<developerTools:FocusTracker IsActive="True" />
ghost commented 3 years ago

Hello Marv51, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

Kyaa-dost commented 3 years ago

Thanks for looking into this @Marv51! Do you want to fix this issue and create a PR?

michael-hawker commented 3 years ago

@azchohfi this works from our UWP Sample App in the store, can you test from the WinUI 3 version? Wondering if this is exposing a timing issue or something in WinUI 3 vs. UWP? Or maybe we missed something we needed to change?

I guess worst case we just add a guard around this code (which we probably should do anyway?):

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/529624cf1af8f034bed9c4f9927d0e0d62f1a6db/Microsoft.Toolkit.Uwp.DeveloperTools/FocusTracker/FocusTracker.cs#L112-L115

Marv51 commented 3 years ago

@Kyaa-dost I think somebody with a already set up dev-environment can add those null checks in a couple minutes. So I prefer if somebody else could do this?

Kyaa-dost commented 3 years ago

@Kyaa-dost I think somebody with a already set up dev-environment can add those null checks in a couple minutes. So I prefer if somebody else could do this?

@Marv51 Yes that would be another alternative. Thanks for checking!