diff --git a/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Android.cs b/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Android.cs index 5fceff130f22..f9942946064f 100644 --- a/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Android.cs +++ b/src/Controls/src/Core/Handlers/Shell/ShellItemHandler.Android.cs @@ -27,15 +27,15 @@ public partial class ShellItemHandler : ElementHandler, I internal ViewPager2? _viewPager; internal BottomNavigationView? _bottomNavigationView; internal TabbedViewManager? _tabbedViewManager; - ShellItemTabbedViewAdapter? _shellItemAdapter; - ShellSectionFragmentAdapter? _adapter; - ShellItemPageChangeCallback? _pageChangeCallback; + internal ShellSectionFragmentAdapter? _adapter; + internal ShellItemTabbedViewAdapter? _shellItemAdapter; + internal ShellItemPageChangeCallback? _pageChangeCallback; IShellContext? _shellContext; Fragment? _parentFragment; // The wrapper fragment that hosts this handler IShellBottomNavViewAppearanceTracker? _appearanceTracker; - Shell? _registeredShell; // Cached at AddAppearanceObserver time for reliable RemoveAppearanceObserver - ShellSection? _shellSection; - Page? _displayedPage; + internal Shell? _registeredShell; // Cached at AddAppearanceObserver time for reliable RemoveAppearanceObserver + internal ShellSection? _shellSection; + internal Page? _displayedPage; bool _preserveFragmentResources; // During SwitchToShellItem, preserve fragment-level resources bool _switchingShellItem; // During SwitchToShellItem, suppress mapper-triggered SwitchToSection bool _pendingAdapterUpdate; // After adapter rebuild, suppress next smooth scroll to avoid VP2 overshoot @@ -44,7 +44,7 @@ public partial class ShellItemHandler : ElementHandler, I internal Toolbar? _shellToolbar; // Virtual Toolbar view internal AToolbar? _toolbar; // Native platform toolbar internal IShellToolbarTracker? _toolbarTracker; - IShellToolbarAppearanceTracker? _toolbarAppearanceTracker; + internal IShellToolbarAppearanceTracker? _toolbarAppearanceTracker; internal AppBarLayout? _appBarLayout; /// @@ -299,7 +299,9 @@ internal void SwitchToSection(ShellSection newSection, bool animate) // Track the current section _shellSection = newSection; - // Track displayed page changes + // Remove before adding to guard against duplicate observers from re-entrant calls + // (SwitchToShellItem triggers SwitchToSection twice for the same section). + ((IShellSectionController)newSection).RemoveDisplayedPageObserver(this); ((IShellSectionController)newSection).AddDisplayedPageObserver(this, UpdateDisplayedPage); } diff --git a/src/Controls/src/Core/Handlers/Shell/ShellItemWrapperFragment.Android.cs b/src/Controls/src/Core/Handlers/Shell/ShellItemWrapperFragment.Android.cs index a99bec7cea32..83761aedfb9c 100644 --- a/src/Controls/src/Core/Handlers/Shell/ShellItemWrapperFragment.Android.cs +++ b/src/Controls/src/Core/Handlers/Shell/ShellItemWrapperFragment.Android.cs @@ -108,6 +108,114 @@ public override void OnViewCreated(AView view, Bundle? savedInstanceState) } } + public override void OnDestroyView() + { + // Null out view references so we don't hold on to detached views + // when the fragment is placed on the back stack (view destroyed, fragment alive). + // Also null _adapter so SetupViewPagerAdapter re-creates it and assigns it + // to the new ViewPager2 on the next OnViewCreated (adapter reset path). + if (_handler is not null) + { + // Unregister appearance observer before view is gone. + // RegisterAppearanceObserver() runs again on OnViewCreated via bare List.Add — + // without removal here, each back-stack return stacks a duplicate registration. + if (_handler._registeredShell is not null) + { + ((IShellController)_handler._registeredShell).RemoveAppearanceObserver(_handler); + _handler._registeredShell = null; + } + + // Unregister displayed-page observer before view is gone. + // SwitchToSection() only removes the observer when _shellSection != newSection — + // on same-section view recreation, no removal happens, causing AddDisplayedPageObserver + // to stack a duplicate entry on each back-stack return. + if (_handler._shellSection is not null) + { + ((IShellSectionController)_handler._shellSection).RemoveDisplayedPageObserver(_handler); + _handler._shellSection = null; + } + + // Reset _displayedPage so UpdateDisplayedPage re-runs fully on back-stack return + // instead of early-returning due to the same page reference, leaving the toolbar unconfigured. + _handler._displayedPage = null; + + // Null _bottomNavigationView so appearance callbacks don't update stale views + // while the fragment sits on the back stack. + _handler._bottomNavigationView = null; + + // Tear down TabbedViewManager before view recreation. + // SetupTabbedViewManager overwrites _tabbedViewManager/_shellItemAdapter silently — + // without teardown here, old BNV listeners and event subscriptions leak. + // Note: SetElement(null) already calls RemoveTabs() internally — no need to call it explicitly. + if (_handler._tabbedViewManager is not null) + { + _handler._tabbedViewManager.SetElement(null); + _handler._tabbedViewManager = null; + } + _handler._shellItemAdapter = null; + + // Tear down toolbar to prevent duplicate toolbars and tracker leaks on view recreation. + // SetupToolbar() runs again on OnViewCreated, so old trackers must be disposed + // and old toolbar removed from _appBarLayout before that happens. + _handler._toolbarAppearanceTracker?.Dispose(); + _handler._toolbarAppearanceTracker = null; + + _handler._toolbarTracker?.Dispose(); + _handler._toolbarTracker = null; + + if (_handler._toolbar?.Parent is ViewGroup toolbarParent) + { + toolbarParent.RemoveView(_handler._toolbar); + } + + _handler._toolbar = null; + + // Null _shellToolbar — mirrors DisconnectHandler's cleanup. + // SetupToolbar() creates a new one on OnViewCreated. + _handler._shellToolbar = null; + + // Null _appBarLayout — set in SetupToolbar() from OnViewCreated. + // Leaving it non-null holds a detached view reference after view destruction. + _handler._appBarLayout = null; + + // Unregister page-change callback before nulling _viewPager. + // SetupViewPagerAdapter guards on `if (_pageChangeCallback is null)` — + // if not nulled here, the new ViewPager2 never receives the callback + // and toolbar/top-tab sync silently breaks on back-stack return. + if (_handler._pageChangeCallback is not null && _handler._viewPager is not null) + { + _handler._viewPager.UnregisterOnPageChangeCallback(_handler._pageChangeCallback); + } + _handler._pageChangeCallback = null; + + // Clear adapter before nulling _viewPager to detach FragmentStateAdapter. + // FragmentStateAdapter unregisters lifecycle/adapter observers only when cleared, + // not when the view is destroyed — mirrors DisconnectHandler's adapter/viewpager cleanup. + if (_handler._viewPager is not null) + { + _handler._viewPager.Adapter = null; + } + _handler._viewPager = null; + _handler._adapter = null; + } + + // Remove window insets listener before nulling _rootLayout. + // Dispose guards on `_rootLayout is not null` — if nulled here first, + // Dispose skips the removal and the listener is never unregistered. + if (_rootLayout is not null) + { + MauiWindowInsetListener.RemoveViewWithLocalListener(_rootLayout); + } + + _rootLayout = null; + + // _backPressedCallback is auto-removed by ViewLifecycleOwner when the view is destroyed. + // Null here for symmetry — OnViewCreated recreates it. + _backPressedCallback = null; + + base.OnDestroyView(); + } + protected override void Dispose(bool disposing) { if (disposing) diff --git a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt index bca414541a13..453c1d009d63 100644 --- a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt @@ -196,6 +196,7 @@ override Microsoft.Maui.Controls.Handlers.ShellContentNavigationFragment.OnCreat override Microsoft.Maui.Controls.Handlers.ShellContentNavigationFragment.OnDestroyView() -> void override Microsoft.Maui.Controls.Handlers.ShellItemWrapperFragment.Dispose(bool disposing) -> void override Microsoft.Maui.Controls.Handlers.ShellItemWrapperFragment.OnCreateView(Android.Views.LayoutInflater! inflater, Android.Views.ViewGroup? container, Android.OS.Bundle? savedInstanceState) -> Android.Views.View! +override Microsoft.Maui.Controls.Handlers.ShellItemWrapperFragment.OnDestroyView() -> void override Microsoft.Maui.Controls.Handlers.ShellItemWrapperFragment.OnViewCreated(Android.Views.View! view, Android.OS.Bundle? savedInstanceState) -> void override Microsoft.Maui.Controls.Handlers.ShellSectionWrapperFragment.Dispose(bool disposing) -> void override Microsoft.Maui.Controls.Handlers.ShellSectionWrapperFragment.OnCreate(Android.OS.Bundle? savedInstanceState) -> void diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue36108.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue36108.cs new file mode 100644 index 000000000000..7548d1f72552 --- /dev/null +++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue36108.cs @@ -0,0 +1,99 @@ +using Microsoft.Maui.Controls; + +namespace Maui.Controls.Sample.Issues; + +// Regression guard for #36108 — Android Shell handler OnDestroyView defensive cleanup. +// Tests that after switching between FlyoutItems: +// 1. Toolbar title is correctly configured (UpdateDisplayedPage ran — not early-returned on stale _displayedPage) +// 2. Tab switching still updates the toolbar (ViewPager2 page callback registered correctly) +[Issue(IssueTracker.Github, 36108, "Android Shell handler — OnDestroyView defensive cleanup regression guard", PlatformAffected.Android)] +public class Issue36108 : TestShell +{ + public const string FlyoutItemATitle = "Section A"; + public const string FlyoutItemBTitle = "Section B"; + public const string Tab1Title = "Tab 1"; + public const string Tab2Title = "Tab 2"; + public const string Tab1LabelId = "Tab1Label"; + public const string Tab2LabelId = "Tab2Label"; + public const string SectionBLabelId = "SectionBLabel"; + + protected override void Init() + { + var tab1Page = new ContentPage + { + Title = Tab1Title, + Content = new Label + { + Text = "Tab 1 Content", + AutomationId = Tab1LabelId, + HorizontalOptions = LayoutOptions.Center, + VerticalOptions = LayoutOptions.Center + } + }; + + var tab2Page = new ContentPage + { + Title = Tab2Title, + Content = new Label + { + Text = "Tab 2 Content", + AutomationId = Tab2LabelId, + HorizontalOptions = LayoutOptions.Center, + VerticalOptions = LayoutOptions.Center + } + }; + + var sectionBPage = new ContentPage + { + Title = FlyoutItemBTitle, + Content = new Label + { + Text = "Section B Content", + AutomationId = SectionBLabelId, + HorizontalOptions = LayoutOptions.Center, + VerticalOptions = LayoutOptions.Center + } + }; + + // FlyoutItem A — has two bottom tabs + var flyoutItemA = new FlyoutItem + { + Title = FlyoutItemATitle, + Route = "SectionA", + Items = + { + new Tab + { + Title = Tab1Title, + Route = "Tab1", + AutomationId = Tab1Title, + Items = { new ShellContent { Content = tab1Page } } + }, + new Tab + { + Title = Tab2Title, + Route = "Tab2", + AutomationId = Tab2Title, + Items = { new ShellContent { Content = tab2Page } } + } + } + }; + + // FlyoutItem B — simple page + var flyoutItemB = new FlyoutItem + { + Title = FlyoutItemBTitle, + Route = "SectionB", + Items = + { + new Tab + { + Items = { new ShellContent { Content = sectionBPage } } + } + } + }; + + Items.Add(flyoutItemA); + Items.Add(flyoutItemB); + } +} diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36108.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36108.cs new file mode 100644 index 000000000000..72d319a6099b --- /dev/null +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36108.cs @@ -0,0 +1,72 @@ +using NUnit.Framework; +using UITest.Appium; +using UITest.Core; + +namespace Microsoft.Maui.TestCases.Tests.Issues; + +// Regression guard for https://github.com/dotnet/maui/issues/36108 +// Verifies that after navigating between FlyoutItems (A → B → A), the Shell tab bar +// and page content are still correctly rendered — guarding against the stale +// _displayedPage guard in UpdateDisplayedPage skipping toolbar/tab configuration. +public class Issue36108 : _IssuesUITest +{ + public Issue36108(TestDevice device) : base(device) + { + } + + public override string Issue => "Android Shell handler — OnDestroyView defensive cleanup regression guard"; + + // Navigate Section A → Section B → Section A and verify Tab 1 content is shown. + // If _displayedPage is not reset during ShellItem switch, UpdateDisplayedPage + // early-returns on the stale same-page reference and the tab bar is misconfigured. + [Test, Order(0)] + [Category(UITestCategories.Shell)] + public void TabContentVisibleAfterFlyoutItemRoundTrip() + { + App.WaitForElement("Tab1Label"); + + // Open flyout and navigate to Section B + App.TapShellFlyoutIcon(); + App.WaitForElement("Section B"); + App.Tap("Section B"); + App.WaitForElement("SectionBLabel"); + + // Open flyout and navigate back to Section A + App.TapShellFlyoutIcon(); + App.WaitForElement("Section A"); + App.Tap("Section A"); + + // Tab 1 content must be visible — UpdateDisplayedPage must not early-return + App.WaitForElement("Tab1Label"); + + // Defensive: verify tab bar is functional after round-trip. + // If UpdateTabBarVisibility was skipped (due to stale _displayedPage early-return), + // the bottom nav tabs may be hidden or misconfigured — Tab 2 would be unreachable. + App.WaitForElement("Tab 2"); + } + + // After navigating A → B → A, verify that tapping Tab 2 correctly shows Tab 2 content. + // This guards against ViewPager2 page-change callbacks being dropped when the + // ShellItem view is recreated after returning from Section B. + [Test, Order(1)] + [Category(UITestCategories.Shell)] + public void TabSwitchingWorksAfterFlyoutItemRoundTrip() + { + App.WaitForElement("Tab1Label"); + + // Round-trip A → B → A + App.TapShellFlyoutIcon(); + App.WaitForElement("Section B"); + App.Tap("Section B"); + App.WaitForElement("SectionBLabel"); + + App.TapShellFlyoutIcon(); + App.WaitForElement("Section A"); + App.Tap("Section A"); + App.WaitForElement("Tab1Label"); + + // Switch to Tab 2 — must work correctly after the round-trip + App.Tap("Tab 2"); + App.WaitForElement("Tab2Label"); + } +} diff --git a/src/Core/src/Platform/Android/Navigation/StackNavigationManager.cs b/src/Core/src/Platform/Android/Navigation/StackNavigationManager.cs index 9416c2796036..b963d15907cd 100644 --- a/src/Core/src/Platform/Android/Navigation/StackNavigationManager.cs +++ b/src/Core/src/Platform/Android/Navigation/StackNavigationManager.cs @@ -379,9 +379,13 @@ public virtual void Disconnect() _fragmentManager = null; } - // Extended to accept an optional FragmentContainerView for Shell sections that - // create their own container externally. The default (null) preserves backward - // compatibility — callers using the original Connect(IView) signature still work. + // Used by NavigationPage: connects the navigation view without an external + // FragmentContainerView (NavigationPage owns and manages its own container internally). + // To intercept all Connect calls in a subclass, override the two-arg overload below. + public virtual void Connect(IView navigationView) => Connect(navigationView, null); + + // Shell provides an external FragmentContainerView; NavigationPage passes null, + // falling back to navigationView.Handler.PlatformView as the container. public virtual void Connect(IView navigationView, FragmentContainerView? fragmentContainerView = null) { VirtualView = navigationView; diff --git a/src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt b/src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt index dd33b659a98d..7d7817ec6561 100644 --- a/src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt +++ b/src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt @@ -337,5 +337,4 @@ override Microsoft.Maui.Platform.MauiDrawerLayout.OnInterceptTouchEvent(Android. virtual Microsoft.Maui.Platform.MauiDrawerLayout.Disconnect() -> void virtual Microsoft.Maui.Platform.MauiDrawerLayout.LayoutAsFlyout() -> void virtual Microsoft.Maui.Platform.MauiDrawerLayout.LayoutSideBySide() -> void -*REMOVED*virtual Microsoft.Maui.Platform.StackNavigationManager.Connect(Microsoft.Maui.IView! navigationView) -> void virtual Microsoft.Maui.Platform.StackNavigationManager.Connect(Microsoft.Maui.IView! navigationView, AndroidX.Fragment.App.FragmentContainerView? fragmentContainerView = null) -> void