Skip to content

Commit

Permalink
[Windows] Fix crash when navigating pages (#25740)
Browse files Browse the repository at this point in the history
### Description of Change

Fixes a long-standing crash when navigating pages:

1. Manually clear the `ContentPresenter.Content` property.

When navigating pages it was discovered that the `Content` property of
the `ContentPresenter` sometimes was not being detached from the parent
`ContentPresenter` (the `ContentPresenter` itself does detach from the
Page). A workaround for this was to manually clear the `Content`
property.

**The actual cause of this bug:** I suspect that something causing the
lifecycle tracking to be messed up. I'm still not sure what this is, but
for now this will help our customers more than nothing.

## Workaround

The following can be used as a workaround. Place it in the `Loaded`
event for your `AppShell`:

```csharp
private void AppShell_Loaded(object? sender, EventArgs e)
{
    Loaded -= AppShell_Loaded;

#if WINDOWS
    if (Handler != null && Handler.PlatformView is ShellView shellView &&
        shellView.Content is MauiNavigationView nv &&
        nv.Content is Microsoft.UI.Xaml.Controls.Frame frame)
    {
        frame.Navigating += (s, e) =>
        {
            if (frame.Content is Microsoft.UI.Xaml.Controls.Page page)
            {
                page.Unloaded += PageUnloaded;

                void PageUnloaded(object sender, Microsoft.UI.Xaml.RoutedEventArgs e)
                {
                    page.Unloaded -= PageUnloaded;
                    if (page.Content is Microsoft.UI.Xaml.Controls.ContentPresenter presenter)
                    {
                        presenter.Content = null;
                    }
                };
            }
        };
    }
#endif
}
```

### Issues Fixed

Fixes #22790 #18441 #22131
  • Loading branch information
Foda authored Nov 19, 2024
2 parents 6b00a27 + 1a39910 commit fa519a2
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,31 @@ await CreateHandlerAndAddToWindow<ShellHandler>(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<ShellHandler>(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()
{
Expand Down
26 changes: 21 additions & 5 deletions src/Core/src/Platform/Windows/StackNavigationManager.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -17,6 +14,7 @@ public class StackNavigationManager
IMauiContext _mauiContext;
Frame? _navigationFrame;
Action? _pendingNavigationFinished;
ContentPresenter? _previousContent;
bool _connected;

protected NavigationRootManager WindowManager => _mauiContext.GetNavigationRootManager();
Expand Down Expand Up @@ -58,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)
Expand Down Expand Up @@ -166,6 +170,14 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e)
VerticalAlignment = UI.Xaml.VerticalAlignment.Stretch
};

// 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;
}

page.Content = presenter;
}
else
Expand All @@ -174,13 +186,15 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e)
}

// 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)
{
Expand All @@ -190,6 +204,8 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e)

_pendingNavigationFinished = () =>
{
_previousContent = presenter;

if (presenter?.Content is not FrameworkElement pc)
{
FireNavigationFinished();
Expand Down

0 comments on commit fa519a2

Please sign in to comment.