From 99f06887e4d6ef0a8faf22f011cbc0c821e9671d Mon Sep 17 00:00:00 2001 From: Mike Corsaro Date: Fri, 1 Nov 2024 13:21:07 -0700 Subject: [PATCH 1/6] Create work-around for navigation crash on windows --- .../Windows/StackNavigationManager.cs | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/Core/src/Platform/Windows/StackNavigationManager.cs b/src/Core/src/Platform/Windows/StackNavigationManager.cs index 79ab61095b73..252a49bd192f 100644 --- a/src/Core/src/Platform/Windows/StackNavigationManager.cs +++ b/src/Core/src/Platform/Windows/StackNavigationManager.cs @@ -1,9 +1,6 @@ using System; using System.Collections.Generic; -using System.Linq; -using System.Text; using System.Threading; -using System.Threading.Tasks; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; using Microsoft.UI.Xaml.Media.Animation; @@ -156,31 +153,34 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e) return; var nv = NavigationView; - ContentPresenter? presenter; + ContentControl? presenter; if (page.Content == null) { - presenter = new ContentPresenter() + presenter = new ContentControl() { HorizontalAlignment = UI.Xaml.HorizontalAlignment.Stretch, VerticalAlignment = UI.Xaml.VerticalAlignment.Stretch }; + presenter.Unloaded += Presenter_Unloaded; page.Content = presenter; } else { - presenter = page.Content as ContentPresenter; + presenter = page.Content as ContentControl; } // At this point if the Content isn't a ContentPresenter the user has replaced - // the conent so we just let them take control + // the content so we just let them take control if (presenter == null || _currentPage == null) return; + var platformPage = _currentPage.ToPlatform(MauiContext); + try { - presenter.Content = _currentPage.ToPlatform(MauiContext); + presenter.Content = platformPage; } catch (Exception) { @@ -208,6 +208,21 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e) fe.OnLoaded(FirePendingNavigationFinished); } + /// + /// There's some bug in our code, or the lifecycle of ContentControl, that is causing the content to + /// never be removed from the parent... + /// + /// + /// + private void Presenter_Unloaded(object sender, RoutedEventArgs e) + { + if (sender is ContentControl control) + { + control.Content = null; + control.Unloaded -= Presenter_Unloaded; + } + } + void FireNavigationFinished() { _pendingNavigationFinished = null; From 2b6eca0ba48415bdc746b071298dc6b9c7da6045 Mon Sep 17 00:00:00 2001 From: Mike Corsaro Date: Thu, 7 Nov 2024 12:24:26 -0800 Subject: [PATCH 2/6] Add small test --- .../Elements/Shell/ShellTests.Windows.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.Windows.cs b/src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.Windows.cs index 3a5b98c09518..4bbcc4c27276 100644 --- a/src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.Windows.cs +++ b/src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.Windows.cs @@ -541,6 +541,31 @@ await CreateHandlerAndAddToWindow(shell, async (handler) => }); } + [Fact(DisplayName = "Navigate back and forth doesn't crash")] + public async Task NavigateBackAndForthDoesntCrash() + { + SetupBuilder(); + + var shell = await CreateShellAsync(shell => + { + shell.CurrentItem = new ContentPage(); + }); + + await CreateHandlerAndAddToWindow(shell, async (handler) => + { + var secondPage = new ContentPage(); + + for (int i = 0; i < 5; i++) + { + await shell.Navigation.PushAsync(secondPage, false); + await Task.Delay(100); + await shell.Navigation.PopToRootAsync(false); + await Task.Delay(100); + } + Assert.NotNull(shell.Handler); + }); + } + [Fact(DisplayName = "Shell Toolbar With Only MenuBarItems Is Visible")] public async Task ShellToolbarWithOnlyMenuBarItemsIsVisible() { From 591bc9cf9195492e33df58c1da34947212ac4e1e Mon Sep 17 00:00:00 2001 From: Mike Corsaro Date: Fri, 8 Nov 2024 11:23:58 -0800 Subject: [PATCH 3/6] Add logic to clear content ASAP if unloaded event didn't yet fire --- .../src/Platform/Windows/StackNavigationManager.cs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Core/src/Platform/Windows/StackNavigationManager.cs b/src/Core/src/Platform/Windows/StackNavigationManager.cs index 252a49bd192f..2901c31c7d92 100644 --- a/src/Core/src/Platform/Windows/StackNavigationManager.cs +++ b/src/Core/src/Platform/Windows/StackNavigationManager.cs @@ -14,6 +14,7 @@ public class StackNavigationManager IMauiContext _mauiContext; Frame? _navigationFrame; Action? _pendingNavigationFinished; + ContentControl? _previousContent; bool _connected; protected NavigationRootManager WindowManager => _mauiContext.GetNavigationRootManager(); @@ -162,8 +163,15 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e) HorizontalAlignment = UI.Xaml.HorizontalAlignment.Stretch, VerticalAlignment = UI.Xaml.VerticalAlignment.Stretch }; - presenter.Unloaded += Presenter_Unloaded; + // It's possible that the unloaded event didn't happen yet, so force clear the content + if (_previousContent is not null) + { + _previousContent.Content = null; + _previousContent = null; + } + + presenter.Unloaded += Presenter_Unloaded; page.Content = presenter; } else @@ -190,6 +198,8 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e) _pendingNavigationFinished = () => { + _previousContent = presenter; + if (presenter?.Content is not FrameworkElement pc) { FireNavigationFinished(); From cef6bef8c7aa6927229ed730d6d324677d5b834f Mon Sep 17 00:00:00 2001 From: Mike Corsaro Date: Fri, 8 Nov 2024 14:03:44 -0800 Subject: [PATCH 4/6] Fix test issues --- .../Windows/StackNavigationManager.cs | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/src/Core/src/Platform/Windows/StackNavigationManager.cs b/src/Core/src/Platform/Windows/StackNavigationManager.cs index 2901c31c7d92..367384e78751 100644 --- a/src/Core/src/Platform/Windows/StackNavigationManager.cs +++ b/src/Core/src/Platform/Windows/StackNavigationManager.cs @@ -164,14 +164,14 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e) VerticalAlignment = UI.Xaml.VerticalAlignment.Stretch }; - // It's possible that the unloaded event didn't happen yet, so force clear the content + // There's some bug in our code, or the lifecycle of ContentControl, that is causing the content to + // never be removed from the parent... if (_previousContent is not null) { _previousContent.Content = null; _previousContent = null; } - presenter.Unloaded += Presenter_Unloaded; page.Content = presenter; } else @@ -218,21 +218,6 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e) fe.OnLoaded(FirePendingNavigationFinished); } - /// - /// There's some bug in our code, or the lifecycle of ContentControl, that is causing the content to - /// never be removed from the parent... - /// - /// - /// - private void Presenter_Unloaded(object sender, RoutedEventArgs e) - { - if (sender is ContentControl control) - { - control.Content = null; - control.Unloaded -= Presenter_Unloaded; - } - } - void FireNavigationFinished() { _pendingNavigationFinished = null; From c5f9996384d0a9256b7f1b637868cbafda53f0bf Mon Sep 17 00:00:00 2001 From: Mike Corsaro Date: Fri, 8 Nov 2024 15:51:23 -0800 Subject: [PATCH 5/6] Switch back to content presenter to fix tests --- src/Core/src/Platform/Windows/StackNavigationManager.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Core/src/Platform/Windows/StackNavigationManager.cs b/src/Core/src/Platform/Windows/StackNavigationManager.cs index 367384e78751..8fe98589b398 100644 --- a/src/Core/src/Platform/Windows/StackNavigationManager.cs +++ b/src/Core/src/Platform/Windows/StackNavigationManager.cs @@ -14,7 +14,7 @@ public class StackNavigationManager IMauiContext _mauiContext; Frame? _navigationFrame; Action? _pendingNavigationFinished; - ContentControl? _previousContent; + ContentPresenter? _previousContent; bool _connected; protected NavigationRootManager WindowManager => _mauiContext.GetNavigationRootManager(); @@ -154,11 +154,11 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e) return; var nv = NavigationView; - ContentControl? presenter; + ContentPresenter? presenter; if (page.Content == null) { - presenter = new ContentControl() + presenter = new ContentPresenter() { HorizontalAlignment = UI.Xaml.HorizontalAlignment.Stretch, VerticalAlignment = UI.Xaml.VerticalAlignment.Stretch @@ -176,7 +176,7 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e) } else { - presenter = page.Content as ContentControl; + presenter = page.Content as ContentPresenter; } // At this point if the Content isn't a ContentPresenter the user has replaced From 1a39910172c42dece3346f28aa62710d9c18915f Mon Sep 17 00:00:00 2001 From: Mike Corsaro Date: Mon, 18 Nov 2024 11:49:17 -0800 Subject: [PATCH 6/6] Update StackNavigationManager.cs --- src/Core/src/Platform/Windows/StackNavigationManager.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Core/src/Platform/Windows/StackNavigationManager.cs b/src/Core/src/Platform/Windows/StackNavigationManager.cs index 8fe98589b398..3ff11c3fbf77 100644 --- a/src/Core/src/Platform/Windows/StackNavigationManager.cs +++ b/src/Core/src/Platform/Windows/StackNavigationManager.cs @@ -56,6 +56,12 @@ public virtual void Disconnect(IStackNavigation navigationView, Frame navigation FirePendingNavigationFinished(); _navigationFrame = null; NavigationView = null; + + if (_previousContent is not null) + { + _previousContent.Content = null; + _previousContent = null; + } } public virtual void NavigateTo(NavigationRequest args)