UserControl inside ContentDialog receives unexpected Unloaded/Loaded events on showing ContentDialog
Describe the bug
Showing a ContentDialog containing a UserControl in it's Content property will lead to unexpected calls of Loaded and Unloaded event handlers in the displayed UserControl:
- Call to ContentDialog.ShowAsync
- UserControl.Loaded -> expected call
- UserControl.Unloaded -> unexpected call
- UserControl.Unloaded -> unexpected call
- Close ContenDialog
- UserControl.Unloaded -> expected call
This behavior leads to unexpected results if there needs to be cleanup logic inside the hosted UserControl's Unloaded event handler as this is already executed while opening the ContentDialog.
Steps to reproduce the bug
Please see attached repro solution: ContentDialog_Events_Repro.zip
Expected behavior
- Call to ContentDialog.ShowAsync
- UserControl.Loaded -> expected call
- Close ContenDialog
- UserControl.Unloaded -> expected call
Screenshots
No response
NuGet package version
WinUI 3 - Windows App SDK 1.4 (stable): 1.4.230913002
Windows version
Windows 11 (22H2): Build 22621
Additional context
No response
This issue still persists with the latest stable version of Windows App SDK (1.4.230913002). Are there any plans on fixing this? This is a blocking issue for us as we rely on best practices for custom control development. We will free all template parts in our custom controls in the unloaded event...
The issue that occurs in the sample application is that the control gets added to the tree, removed and re-added and the removed and re-added. We have identified an issue where the "Unloaded" event gets fired for each of these removals, 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. In this scenario, the application can look at the IsLoaded property and if it is true, skip the Unloaded event processing. The final time that the element is removed from the tree, the "Unloaded" event will fire again and the element's IsLoaded property will be false.
Details:
First a bit of a side as to why this content is getting removed and re-added multiple times. When the content dialog is shown, it added underneath a popup element in the live tree along with its content (element is added). Then when the template is applied to the content dialog (to generate things like the common buttons and such), the content (your control) is removed from the tree and added to the content control under the template (element is removed and re added). And finally, afther the smoke layer is created (which happens later during template application), the popup (and its children) get 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-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 no 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 o 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 (one or more times) in the same tick. Here is what happens:
- Element is added. It is marked as needing a Loading event.
- Element is removed. An Unloaded event is raised asynchronously (so it doesn't execute right away).
- Element is re-added. It is marked as needing a Loading event.
- Element is removed. An Unloaded event is raised asynchronously (so it doesn't execute right away).
- Element is re-added. It is marked as needing a Loading event.
- 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.
- Layout completes and we raise the Loaded event (executes immediately) and the frame is complete.
- Asynchronous events are fired, including the Unloaded event raised in step 2 AND the unloaded event raised in step 4.
- So now we have unloaded events firing 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.
We are going to close this issue with the work around. However, there a number of other open issues caused by this sequence of events that may not have a workaround and we hope to fix and resolve those in 2.0.
Reactivated because there is an extra Unloaded event in WinAppSDK 1.6 when showing a ContentDialog. A fix is in progress for a 1.6 servicing release.
meet the same bug here, hope got fixed quickly.
Closing as fixed in 1.6.1.
Hi @duncanmacmichael & @codendone, great to see progress in this area! Unfortunately, we're still experiencing the issue. I've updated the repro to version 1.6.240923002:
Thanks @s-beltz! @codendone Should we reopen?
Please this needs a fix. It messes with shadows in ContentDialogs