dotnet / dotnet/maui

[Android][Test gap] Issue36108 regression test doesn't exercise ShellItemWrapperFragment.OnDestroyView _displayedPage reset

Open
#36,264 0 comments 1 reaction 0 assignees View on GitHub
area-controls-shell platform/android s/triaged
Dominant language
C#
Stars
23.3k
Forks
2k
Avg merge
1d 15h
Merged PRs (30d)
290

Description

## Context

Follow-up from PR #36133 (fixes #36108). That PR made two changes:

1. **Fix 1** — restored the `StackNavigationManager.Connect(IView)` 1-arg binary-compat overload (the binary break). ✅ Correct.
2. **Fix 2** — added `_handler._displayedPage = null;` to `ShellItemWrapperFragment.OnDestroyView` (`src/Controls/src/Core/Handlers/Shell/ShellItemWrapperFragment.Android.cs`) so that on back-stack view recreation, `SwitchToSection → AddDisplayedPageObserver → UpdateDisplayedPage` re-runs fully instead of early-returning on the stale `_displayedPage == page` guard (leaving the toolbar/back-button unconfigured). ✅ The code fix is correct.

To guard Fix 2, PR #36133 also added a UI test `Issue36108` (`TestCases.HostApp/Issues/Issue36108.cs` + `TestCases.Shared.Tests/Tests/Issues/Issue36108.cs`) that navigates **FlyoutItem A → B → A**.

## The gap

The added test **does not exercise the `OnDestroyView` code path that Fix 2 changes**, so it would pass with or without the fix (it is not a valid regression guard for the specific bug).

Tracing the flyout-item navigation:

- `ShellHandler.SwitchToItem` (`ShellHandler.Android.cs`, ~L176–181): after the first item, subsequent `ShellItem` switches return early via `SwitchToShellItem()` — *"No fragment transaction — the wrapper fragment stays in navigationlayout_content."*
- The wrapper fragment is `.Replace()`d **once** into `navigationlayout_content` with **no `AddToBackStack`** (~L224–227), so `ShellItemWrapperFragment.OnDestroyView` never fires during A → B → A.
- `SwitchToShellItem` **independently** nulls `_displayedPage` at `ShellItemHandler.Android.cs:368` (and `DisconnectHandler` nulls it at ~L828), regardless of the `OnDestroyView` change.

Net: the flyout round-trip is handled entirely by `SwitchToShellItem`, which already resets `_displayedPage`. Reverting the new `OnDestroyView` line would **not** make `Issue36108` fail.

## What's needed

A regression test that actually triggers `ShellItemWrapperFragment.OnDestroyView` (fragment **view** destroyed while the fragment instance stays alive) and asserts the toolbar / back-button is correctly reconfigured on view recreation. Candidate triggers to investigate:

- Push/detail navigation that returns via the Android back stack, or
- A configuration change (rotation) / process-death-restore of the hosting fragment.

## Acceptance criteria

- [ ] Identify the concrete user scenario that invokes `ShellItemWrapperFragment.OnDestroyView` followed by `OnViewCreated` with the same displayed page.
- [ ] Add a test reproducing that scenario and asserting the toolbar/back-button state refreshes.
- [ ] **Verify the test FAILS when the `_handler._displayedPage = null;` line in `OnDestroyView` is reverted, and PASSES with it** (true regression guard).
- [ ] (Optional) Keep or adapt the existing `Issue36108` flyout round-trip test as coverage for the `SwitchToShellItem` reconfiguration path, clearly labeled as such.

## Notes

- The `_displayedPage` code fix in #36133 is correct and can merge as-is; this issue only tracks strengthening the test coverage so a future regression in `OnDestroyView` is caught.
- Reference: `UpdateDisplayedPage` early-return guard at `ShellItemHandler.Android.cs:597` (`if (_displayedPage == page) return;`); `AddDisplayedPageObserver` invokes the callback synchronously (`ShellSection.cs`).

Contributor guide

Open the contributing guide

Research direction

Start by tracing ShellHandler.Android.cs, ShellItemHandler.Android.cs, and ShellItemWrapperFragment.Android.cs to identify a lifecycle path that invokes OnDestroyView followed by OnViewCreated with the same displayed page. Review the existing Issue36108 tests and investigate back-stack navigation or configuration changes. Done means a UI test verifies toolbar/back-button refresh and fails when the OnDestroyView reset is reverted.

Written by the indexing model from the issue text.

Assessment

Tech stack
android, csharp
Domain
mobile, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.