Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unloaded event is triggered for the control when the control is set as content for the ContentDailog #8342

Open
saiganesh-sakthivel opened this issue Mar 30, 2023 · 6 comments
Labels
bug Something isn't working team-Controls Issue for the Controls team

Comments

@saiganesh-sakthivel
Copy link

Describe the bug

The TextBox control is set as content for the contentDailog which is called to show inside the button click. When we call the content dailog through the button click, the unloaded event is called for the TextBox control. It should not called when the text box is display in the view.

Steps to reproduce the bug

  1. Run the Sample.
    SfCalendarDatePickerBug.zip
  2. TextBox in the mainpage will call loaded event properly without calling the unloaded evet. inside the loaded and unloaded event, we have set the text for the text box for better understandable.
  3. Click the button to display the content dailog with the TextBox as content.
  4. On that time, the unloaded event triggered, so the text in the content is set as unloaded.
    Expected Behavior
  5. It should not trigger the unloaded event.

Expected behavior

It should trigger the loaded event only.

Screenshots

No response

NuGet package version

WinUI 3 - Windows App SDK 1.2.5: 1.2.230313.1

Windows version

Windows 11 (22H2): Build 22621

Additional context

No response

@saiganesh-sakthivel saiganesh-sakthivel added the bug Something isn't working label Mar 30, 2023
@saiganesh-sakthivel
Copy link
Author

Hi Team,

Any update regarding this?

@bpulliam bpulliam added the team-Controls Issue for the Controls team label Aug 9, 2023
@saiganesh-sakthivel
Copy link
Author

Hi @bpulliam ,

Any update regarding this bug?

@JJBrychell
Copy link

The issue that occurs in that the control gets added to the tree, removed and then re-added. We have identified an issue where the "Unloaded" event gets fired removal, but since it is an asynchronous event, the application doesn't get notified until the element is already back in the tree.

Unfortunately, we have not found a way to correct this issue without potentially breaking applications that may have inadvertently relied upon this behavior (including some internal Xaml code). Because of this, we will try to target a fix for the 2.0 release, but until then you will need to work around the issue and fortunately there appears to be such a workaround. What is happening is that the Unloaded event is getting fired asynchronously when the control is removed from the tree. But then it gets re-added and, by the time the event is actually processed by the application, the element is back in tree. To get around this issue, the application can look at the IsLoaded property on the element and only remove the handler if the value is false. Since the element is, in fact, in the tree, another Unloaded event will come when it is removed a second time.

Details:

First a bit of a side as to why this TextBox is getting removed and re-added. When content dialog is shown, it ends up being added to the tree (causing anything in it to be added to the tree), but then after the smoke layer is created (which happens during template application), it gets removed and re-added to ensure the correct z-order between the dialog content and the smoke layer. So from the perspective of a control, it gets added-removed-added, causing this issue.

Now to the events. Because of the nature of these tree related events, they are raised at different times and in different manners in order to assure that things get manipulated predictably (which they are, it just seems not correctly). For example, Loading and Loaded are raised synchronously since an application might want to modify the tree in them and want to limit the amount of stale UI we render. But Unloaded, is asynchronous since is primarily just resource cleanup and there is no need to delay a render frame for it. So basically, what happens is:

When an element is added to the tree, is marked as needing a Loading event. But we want some "order" to how we raise these (other than the order an application might add them) so we do not raise it immediately.

  • During the tree walk for layout, we see if any element needs a Loading event and if so we synchronously raise it. This gives us a very predictable order. But, the element is not actually loaded yet since we haven't expanded any templates, applied any resources or any of the other things that occur during a layout pass. So at this point, we request that when the layout pass is complete, we raise a Loaded event.
  • At the end of the layout pass, we then iterate over all the elements needing a Loaded event and (if there are still listeners) will raise those events synchronously.
  • And finally, when the element is removed from the tree we asynchronously raise the Unloaded event.

The issue occurs if you add then remove, then re-add the element in the same tick. Here is what happens:

  1. Element is added. It is marked as needing a Loading event.
  2. Element is removed. An Unloaded event is raised asynchronously (so it doesn't execute right away).
  3. Element is re-added. It is marked as needing a Loading event.
  4. During layout, we see the element needs a Loading event and raise it synchronously (executes immediately) and indicate that we should raise a Loaded event.
  5. Layout completes and we raise the Loaded event (executes immediately) and the frame is complete.
  6. Asynchronous events are fired, including the Unloaded event raised in step 2.

So now we have a unloaded event fired while the element is in the tree, thus causing this issue.

The fix:

Although we are still discussing options, we think the fix is that we would ensure that we don't raise an Unloaded event:

  • Until we have confirmed the Loaded event has been raised (we are still discussing the Loading event).
  • And the element has not been added back into the tree.

It is the first condition that we found breaks some internal code and could potentially break some applications.

For now we are going to keep this issue open to track our work on hopefully getting a solution for 2.0.

Two other issue that are caused by the same series of events:
#8402 UserControl inside ContentDialog receives unexpected Unloaded/Loaded events on showing ContentDialog
#9343 Check Box Click Event Not Fired when added in a ContentDialog in WINUI

@saiganesh-sakthivel
Copy link
Author

@JJBrychell ,

Thank you for the update. We will wait for the fix.

@garrettpauls
Copy link

Thanks for the detailed reply @JJBrychell, I appreciate the in depth explanation!

For anyone who's searching, this bug causes Interaction.Behaviors/Microsoft.Xaml.Interactivity.Behavior to sometimes detach from an object and then not get re-attached while the object is still in use, particularly when used inside DataTemplate for controls that use data virtualization like AutoSuggestBox. I use this as a drop-in replacement for Behavior which works around it in the meantime (provided you don't use 3rd party behaviors):

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 } )
		{
			// 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()
	{
	}
}

@JJBrychell
Copy link

One other issue that has been pointed out based upon a duped bug is that if we add a collapsed element to the tree, we won't get the loading because we don't measure collapsed elements. But we do get the loaded event when we complete the next layout pass. Then when the element is made visible, we will get the loading event (even though it is already loaded) and don't get the loaded event (even though templates may have been expanded).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working team-Controls Issue for the Controls team
Projects
None yet
Development

No branches or pull requests

4 participants