microsoft / XamlBehaviors

This is the official home for UWP XAML Behaviors on GitHub.
MIT License
701 stars 112 forks source link

WinUI3 fires Unloaded event incorrectly, causing Behaviors to be detached and not re-attach to a UI element that is still in use #251

Open garrettpauls opened 4 months ago

garrettpauls commented 4 months ago

WinUI3 currently (as of WindowsAppSdk 1.5) has a bug where Unloaded can fire out of order with Loaded, causing OnDetaching to be called on a behavior without OnAttached being called, but the object is still in use. A detailed explanation of what's going on in WinUI3 can be found in the corresponding issue and a fix is expected in the WindowsAppSdk 2.0 timeframe (undecided).

The flow of events happens like this:

  1. AssociatedObject is loaded and the Behavior is attached. Behavior.OnAttached gets called.
  2. The AssociatedObject is removed from the control tree, causing Unloaded to be fired immediately (asynchronously), causing Behavior.OnDetaching to be called.
  3. In the same UI pass, AssociatedObject is added back to the control tree. The framework sees that AssociatedObject was removed from the tree and then added back, so it does not fire a Loaded event. (This is the bug in WinUI3, the asymmetric calling of Unloaded vs Loaded.) This means Behavior.OnAttached will not be called, and the behavior is no longer attached to AssociatedObject.
  4. The UI continues to use AssociatedObject as part of the UI, but the Behavior is no longer attached to it, causing an inconsistent UI experience.

The current workaround I've added is to implement a custom Behavior class to use instead of using the one built in to the framework, which checks AssociatedObject.IsLoaded and only detaches if the object is no longer loaded.

While I would appreciate this library updating to handle this case, I can also understand not wanting to accommodate a known bug in a technology when this library's code is using the events in alignment with their expected behavior. If you don't want to change this I understand, but wanted to raise the issue for the sake of visibility for anyone else who encounters it.

This Loaded/Unloaded issue crops up frequently in areas that use control virtualization, particularly things like ListView and AutoSuggestBox.

public abstract class Behavior<T> : DependencyObject, IBehavior
    where T : DependencyObject
{
    protected Behavior()
    {
        AssociatedObject = null;
    }

    DependencyObject IBehavior.AssociatedObject => AssociatedObject;

    public T AssociatedObject
    {
        get;
        private set;
    }

    public void Attach( DependencyObject? associatedObject )
    {
        if( associatedObject == null || ReferenceEquals( associatedObject, AssociatedObject ) || DesignMode.DesignModeEnabled )
        {
            // do nothing, object is already attached
        }
        else if( associatedObject is T typedObject )
        {
            AssociatedObject = typedObject;
            OnAttached();
        }
        else
        {
            throw new InvalidOperationException( $"AssociatedObject is expected to be type {typeof( T ).FullName} but was {associatedObject.GetType().FullName}" );
        }
    }

    public void Detach()
    {
        if( AssociatedObject is FrameworkElement { IsLoaded: true } element )
        {
            // This case happens when the control is removed from the visual tree and added back before the Unloaded event is fired.
            // Details on why this happens can be found here: https://github.com/microsoft/microsoft-ui-xaml/issues/8342#issuecomment-2031017667
            // This bug is expected to be fixed in the WindowsAppSdk 2.0 timeframe, at which point this workaround can be removed.
        }
        else
        {
            OnDetaching();
            AssociatedObject = default!;
        }
    }

    protected virtual void OnAttached()
    {
    }

    protected virtual void OnDetaching()
    {
    }
}