[iOS] Fix Shell long-press back button not triggering navigation events

.github/agent-pr-session/pr-33380.md

@@ -0,0 +1,226 @@
+# PR Review: #33380 - [PR agent] Issue23892.ShellBackButtonShouldWorkOnLongPress - test fix
+
+**Date:** 2026-01-07 | **Issue:** [#33379](https://github.com/dotnet/maui/issues/33379) | **PR:** [#33380](https://github.com/dotnet/maui/pull/33380)
+
+## ✅ Final Recommendation: APPROVE
+
+| Phase | Status |
+|-------|--------|
+| Pre-Flight | ✅ COMPLETE |
+| 🧪 Tests | ✅ COMPLETE |
+| 🚦 Gate | ✅ PASSED |
+| 🔧 Fix | ✅ COMPLETE |
+| 📋 Report | ✅ COMPLETE |
+
+---
+
+<details>
+<summary><strong>📋 Issue Summary</strong></summary>
+
+**Issue #33379**: The UI test `Issue23892.ShellBackButtonShouldWorkOnLongPress` started failing after PR #32456 was merged.
+
+**Test Expectation**: `OnAppearing count: 2`
+**Test Actual**: `OnAppearing count: 1`
+
+**Original Issue #23892**: Using long-press navigation on the iOS back button in Shell does not update `Shell.Current.CurrentPage`. The `Navigated` and `Navigating` events don't fire.
+
+**Platforms Affected:**
+- [x] iOS
+- [ ] Android
+- [ ] Windows
+- [ ] MacCatalyst
+
+</details>
+
+<details>
+<summary><strong>🔍 Deep Regression Analysis - Full Timeline</strong></summary>
+
+## The Regression Chain
+
+This PR addresses a **double regression** - the same functionality was broken twice by subsequent PRs.
+
+### Timeline of Changes to `ShellSectionRenderer.cs`
+
+| Date | PR | Purpose | Key Change | Broke Long-Press? |
+|------|-----|---------|------------|-------------------|
+| Feb 2025 | #24003 | Fix #23892 (long-press back) | Added `_popRequested` flag + `DidPopItem` | ✅ Fixed it |
+| Jul 2025 | #29825 | Fix #29798/#30280 (tab blank issue) | **Removed** `_popRequested`, expanded `DidPopItem` with manual sync | ❌ **Broke it** |
+| Jan 2026 | #32456 | Fix #32425 (navigation hang) | Added null checks, changed `ElementForViewController` | ❌ Maintained broken state |
+
+### PR #24003 - The Original Fix (Feb 2025)
+
+**Problem solved**: Long-press back button didn't trigger navigation events.
+
+**Solution**: Added `_popRequested` flag to distinguish:
+- **User-initiated navigation** (long-press): Call `SendPop()` → triggers `GoToAsync("..")` → fires `OnAppearing`
+- **Programmatic navigation** (code): Skip `SendPop()` to avoid double-navigation
+
+**Key code added**:
+```csharp
+bool _popRequested;
+
+bool DidPopItem(UINavigationBar _, UINavigationItem __)
+    => _popRequested || SendPop();  // If not requested, call SendPop
+```
+
+### PR #29825 - The First Regression (Jul 2025)
+
+**Problem solved**: Tab becomes blank after specific navigation pattern (pop via tab tap, then navigate again, then back).
+
+**What went wrong**: The PR author expanded `DidPopItem` with manual stack synchronization logic (`_shellSection.SyncStackDownTo()`) and **removed the `_popRequested` flag entirely**.
+
+**Result**: `DidPopItem` now ALWAYS does manual sync, never calls `SendPop()` for user-initiated navigation. Long-press navigation stopped triggering `OnAppearing`.
+
+**Why the test didn't catch it**: Unclear - possibly the test wasn't run or was flaky at the time.
+
+### PR #32456 - Maintained the Broken State (Jan 2026)
+
+**Problem solved**: Navigation hangs after rapidly opening/closing pages (iOS 26 specific).
+
+**What it did**: Added null checks to prevent crashes in `DidPopItem` and changed `ElementForViewController` pattern matching.
+
+**Maintained the regression**: The PR kept the broken `DidPopItem` logic from #29825 (no `_popRequested` flag).
+
+**This triggered the test failure**: When #32456 merged to `inflight/candidate`, the existing `Issue23892` test started failing.
+
+</details>
+
+<details>
+<summary><strong>📁 Files Changed</strong></summary>
+
+| File | Type | Changes |
+|------|------|---------|
+| `src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRenderer.cs` | Fix | -20 lines (simplified) |
+| `src/Controls/src/Core/Shell/ShellSection.cs` | Fix | -44 lines (removed `SyncStackDownTo`) |
+
+**Net change:** -49 lines (code reduction)
+
+</details>
+
+<details>
+<summary><strong>💬 PR Discussion Summary</strong></summary>
+
+**Key Comments:**
+- Issue #33379 was filed by @sheiksyedm pointing to the test failure after #32456 merged
+- @kubaflo (author of both #32456 and #33380) created this fix
+
+**Reviewer Feedback:**
+- None yet
+
+**Disagreements to Investigate:**
+| File:Line | Reviewer Says | Author Says | Status |
+|-----------|---------------|-------------|--------|
+| (none) | | | |
+
+**Author Uncertainty:**
+- None expressed
+
+</details>
+
+<details>
+<summary><strong>🧪 Tests</strong></summary>
+
+**Status**: ✅ COMPLETE
+
+- [x] PR includes UI tests (existing test from #24003)
+- [x] Tests reproduce the issue
+- [x] Tests follow naming convention (`IssueXXXXX`) ✅
+
+**Test Files:**
+- HostApp: `src/Controls/tests/TestCases.HostApp/Issues/Issue23892.cs`
+- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue23892.cs`
+
+</details>
+
+<details>
+<summary><strong>🚦 Gate - Test Verification</strong></summary>
+
+**Status**: ✅ PASSED
+
+- [x] Tests PASS with fix
+
+**Test Run:**
+```
+Platform:     iOS
+Test Filter:  FullyQualifiedName~Issue23892
+Result:       SUCCESS ✅
+```
+
+**Result:** PASSED ✅ - The `Issue23892.ShellBackButtonShouldWorkOnLongPress` test now passes with the PR's fix.
+
+</details>
+
+<details>
+<summary><strong>🔧 Fix Candidates</strong></summary>
+
+**Status**: ✅ COMPLETE
+
+| # | Source | Approach | Test Result | Files Changed | Notes |
+|---|--------|----------|-------------|---------------|-------|
+| 1 | try-fix | Simplified `DidPopItem`: Always call `SendPop()` when stacks are out of sync | ✅ PASS (Issue23892 + Issue29798 + Issue21119) | `ShellSectionRenderer.cs` (-17, +6) | **Simpler AND works!** |
+| PR | PR #33380 (original) | Restore `_popRequested` flag + preserve manual sync from #29825/#32456 | ✅ PASS (Gate) | `ShellSectionRenderer.cs` (+11) | Superseded by update |
+| PR | PR #33380 (updated) | **Adopted try-fix #1** - Stack sync detection, removed `SyncStackDownTo` | ✅ PASS (CI pending) | `ShellSectionRenderer.cs`, `ShellSection.cs` (-49 net) | **CURRENT - matches recommendation** |
+
+**Update (2026-01-08):** Developer @kubaflo adopted the simpler approach recommended in try-fix #1.
+
+**Exhausted:** Yes
+**Selected Fix:** PR #33380 (updated) - Now implements the recommended simpler approach
+
+</details>
+
+---
+
+## 📋 Final Report
+
+### Recommendation: ✅ APPROVE
+
+**Update (2026-01-08):** Developer @kubaflo has adopted the recommended simpler approach.
+
+### Changes Made by Developer
+
+The PR now implements exactly the simplified stack-sync detection approach:
+
+**ShellSectionRenderer.cs** - Simplified `DidPopItem`:
+```csharp
+bool DidPopItem(UINavigationBar _, UINavigationItem __)
+{
+    if (_shellSection?.Stack is null || NavigationBar?.Items is null)
+        return true;
+
+    // If stacks are in sync, nothing to do
+    if (_shellSection.Stack.Count == NavigationBar.Items.Length)
+        return true;
+
+    // Stacks out of sync = user-initiated navigation
+    return SendPop();
+}
+```
+
+**ShellSection.cs** - Removed `SyncStackDownTo` method (44 lines deleted)
+
+### Why This Approach Works
+
+| Scenario | What Happens |
+|----------|--------------|
+| **Tab tap pop** | Shell updates stack BEFORE `DidPopItem` → stacks ARE in sync → returns early (no `SendPop()`) |
+| **Long-press back** | iOS pops directly → Shell stack NOT updated → stacks out of sync → calls `SendPop()` |
+
+### Benefits of Updated PR
+
+| Aspect | Before (Original PR) | After (Updated PR) |
+|--------|---------------------|-------------------|
+| Lines changed | +11 | **-49 net** |
+| New fields | `_popRequested` bool | **None (stateless)** |
+| Complexity | State tracking | **Simple sync check** |
+| `SyncStackDownTo` | Preserved | **Removed** |
+
+### Conclusion
+
+The PR now:
+- ✅ Fixes Issue #33379 (long-press back navigation)
+- ✅ Uses the simpler stateless approach
+- ✅ Removes 49 lines of code
+- ✅ No new state tracking required
+- ⏳ Pending CI verification
+
+**Approve once CI passes.**u

src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRenderer.cs

@@ -108,35 +108,15 @@ public bool ShouldPopItem(UINavigationBar _, UINavigationItem __)
 		[Internals.Preserve(Conditional = true)]
 		bool DidPopItem(UINavigationBar _, UINavigationItem __)
 		{
-			// Check for null references
 			if (_shellSection?.Stack is null || NavigationBar?.Items is null)
 				return true;
 
-			// Check if stacks are in sync
+			// If stacks are in sync, nothing to do
 			if (_shellSection.Stack.Count == NavigationBar.Items.Length)
 				return true;
 
-			var pages = _shellSection.Stack.ToList();
-
-			// Ensure we have enough pages and navigation items
-			if (pages.Count == 0 || NavigationBar.Items.Length == 0)
-				return true;
-
-			// Bounds check: ensure we have a valid index for pages array
-			int targetIndex = NavigationBar.Items.Length - 1;
-			if (targetIndex < 0 || targetIndex >= pages.Count || pages[targetIndex] is null)
-				return true;
-
-			_shellSection.SyncStackDownTo(pages[targetIndex]);
-
-			for (int i = pages.Count - 1; i >= NavigationBar.Items.Length; i--)
-			{
-				var page = pages[i];
-				if (page != null)
-					DisposePage(page);
-			}
-
-			return true;
+			// Stacks out of sync = user-initiated navigation
+			return SendPop();
 		}
 
 		internal bool SendPop()
@@ -577,7 +557,7 @@ Element ElementForViewController(UIViewController viewController)
 
 			foreach (var child in ShellSection.Stack)
 			{
-				if (child?.Handler is IPlatformViewHandler { ViewController: var vc } && viewController == vc)
+				if (child?.Handler is IPlatformViewHandler handler && viewController == handler.ViewController)
 					return child;
 			}
 

src/Controls/src/Core/Shell/ShellSection.cs

@@ -110,44 +110,6 @@ void IShellSectionController.SendInsetChanged(Thickness inset, double tabThickne
 			_lastInset = inset;
 			_lastTabThickness = tabThickness;
 		}
-
-		internal void SyncStackDownTo(Page page)
-		{
-			if (_navStack.Count <= 1)
-			{
-				throw new Exception("Nav Stack consistency error");
-			}
-
-			var oldStack = _navStack;
-
-			int index = oldStack.IndexOf(page);
-			_navStack = new List<Page>();
-
-			// Rebuild the stack up to the page that was passed in
-			// Since this now represents the current accurate stack
-			for (int i = 0; i <= index; i++)
-			{
-				_navStack.Add(oldStack[i]);
-			}
-
-			// Send Disappearing for all pages that are no longer in the stack
-			// This will really only SendDisappearing on the top page
-			// but we just call it on all of them to be sure
-			for (int i = oldStack.Count - 1; i > index; i--)
-			{
-				oldStack[i].SendDisappearing();
-			}
-
-			UpdateDisplayedPage();
-
-			for (int i = index + 1; i < oldStack.Count; i++)
-			{
-				RemovePage(oldStack[i]);
-			}
-
-			(Parent?.Parent as IShellController)?.UpdateCurrentState(ShellNavigationSource.Pop);
-		}
-
 		async void IShellSectionController.SendPopping(Task poppingCompleted)
 		{
 			if (_navStack.Count <= 1)