From 70da9b635d3fa35cda7bb83efc2b852060cc5187 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Thu, 8 Jan 2026 21:52:21 +0000
Subject: [PATCH 1/5] Initial plan
From 82bf111a8e7a9c35cf56cbf16e24b75dca050b1d Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Thu, 8 Jan 2026 22:03:14 +0000
Subject: [PATCH 2/5] Add .github folder changes from PRs #33406 and #31487
Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
---
.github/agent-pr-session/pr-31487.md | 261 +++++++++++++++++++
.github/agent-pr-session/pr-33406.md | 247 ++++++++++++++++++
.github/agents/pr.md | 2 +-
.github/instructions/uitests.instructions.md | 48 ++--
.github/skills/try-fix/SKILL.md | 48 ++--
5 files changed, 551 insertions(+), 55 deletions(-)
create mode 100644 .github/agent-pr-session/pr-31487.md
create mode 100644 .github/agent-pr-session/pr-33406.md
diff --git a/.github/agent-pr-session/pr-31487.md b/.github/agent-pr-session/pr-31487.md
new file mode 100644
index 000000000000..b15821b1c3a2
--- /dev/null
+++ b/.github/agent-pr-session/pr-31487.md
@@ -0,0 +1,261 @@
+# PR Review: #31487 - [Android] Fixed duplicate title icon when setting TitleIconImageSource Multiple times
+
+**Date:** 2026-01-08 | **Issue:** [#31445](https://github.com/dotnet/maui/issues/31445) | **PR:** [#31487](https://github.com/dotnet/maui/pull/31487)
+
+## β
Final Recommendation: APPROVE
+
+| Phase | Status |
+|-------|--------|
+| Pre-Flight | β
COMPLETE |
+| π§ͺ Tests | β
COMPLETE |
+| π¦ Gate | β
PASSED |
+| π§ Fix | β
COMPLETE |
+| π Report | β
COMPLETE |
+
+---
+
+
+π Issue Summary
+
+On Android, calling `NavigationPage.SetTitleIconImageSource(page, "image.png")` more than once for the same page results in the icon being rendered multiple times in the navigation bar.
+
+**Steps to Reproduce:**
+1. Launch app on Android
+2. Tap "Set TitleIconImageSource" once: icon appears
+3. Tap it again: a second identical icon appears
+
+**Expected:** Single toolbar icon regardless of how many times SetTitleIconImageSource is called.
+
+**Actual:** Each repeated call adds an additional duplicate icon.
+
+**Platforms Affected:**
+- [ ] iOS
+- [x] Android
+- [ ] Windows
+- [ ] MacCatalyst
+
+**Version:** 9.0.100 SR10
+
+
+
+
+π Files Changed
+
+| File | Type | Changes |
+|------|------|---------|
+| `src/Controls/src/Core/Platform/Android/Extensions/ToolbarExtensions.cs` | Fix | +17/-6 |
+| `src/Controls/tests/TestCases.HostApp/Issues/Issue31445.cs` | Test | +38 |
+| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31445.cs` | Test | +23 |
+| `snapshots/android/Issue31445DuplicateTitleIconDoesNotAppear.png` | Snapshot | binary |
+| `snapshots/mac/Issue31445DuplicateTitleIconDoesNotAppear.png` | Snapshot | binary |
+| `snapshots/windows/Issue31445DuplicateTitleIconDoesNotAppear.png` | Snapshot | binary |
+| `snapshots/ios/Issue31445DuplicateTitleIconDoesNotAppear.png` | Snapshot | binary |
+
+
+
+
+π¬ PR Discussion Summary
+
+**Key Comments:**
+- Issue verified by LogishaSelvarajSF4525 on MAUI 9.0.0 & 9.0.100
+- PR triggered UI tests by jsuarezruiz
+- PureWeen requested rebase
+
+**Reviewer Feedback:**
+- Copilot review: Suggested testing with different image sources or rapid succession to validate fix better
+
+**Disagreements to Investigate:**
+| File:Line | Reviewer Says | Author Says | Status |
+|-----------|---------------|-------------|--------|
+| Issue31445.cs:31 | Test with different images or rapid calls | N/A | β οΈ INVESTIGATE |
+
+**Author Uncertainty:**
+- None noted
+
+
+
+
+π§ͺ Tests
+
+**Status**: β
COMPLETE
+
+- [x] PR includes UI tests
+- [x] Tests reproduce the issue
+- [x] Tests follow naming convention (`Issue31445`)
+
+**Test Files:**
+- HostApp: `src/Controls/tests/TestCases.HostApp/Issues/Issue31445.cs`
+- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31445.cs`
+
+**Test Behavior:**
+- Uses snapshot verification (`VerifyScreenshot()`)
+- Navigates to test page, taps button to trigger duplicate icon scenario
+- Verified to compile successfully
+
+
+
+
+π¦ Gate - Test Verification
+
+**Status**: β
PASSED
+
+- [x] Tests FAIL without fix (bug reproduced - duplicate icons appeared)
+- [x] Tests PASS with fix (single icon as expected)
+
+**Result:** PASSED β
+
+**Verification Details:**
+- Platform: Android (emulator-5554)
+- Without fix: Test FAILED (screenshot mismatch - duplicate icons)
+- With fix: Test PASSED (single icon verified)
+
+
+
+
+π§ Fix Candidates
+
+**Status**: β
COMPLETE
+
+| # | Source | Approach | Test Result | Files Changed | Model | Notes |
+|---|--------|----------|-------------|---------------|-------|-------|
+| 1 | try-fix | Check for existing icon view at position 0, reuse if exists, only create new if needed | β
PASS | `ToolbarExtensions.cs` (+7) | Opus 4.5 | Works! Independently arrived at same solution logic as PR |
+| 2 | try-fix | Dedupe defensively by scanning all toolbar children, keep first `ToolbarTitleIconImageView`, remove extras; then reuse/create | β
PASS | `ToolbarExtensions.cs` (+22/-5) | GPT 5.2 | More robust if child ordering changes or duplicates already exist |
+| 3 | try-fix | Use `FindViewWithTag` to uniquely identify/retrieve the MAUI title icon | β
PASS | `ToolbarExtensions.cs` (+20/-6) | Gemini 2.0 Flash | Explicit identification; avoids index assumptions and iteration; most robust against external view insertions |
+| PR | PR #31487 | Check for existing ToolbarTitleIconImageView before adding new one | β
PASS (Gate) | `ToolbarExtensions.cs` (+17/-6) | Author | Original PR - validated by Gate |
+
+**Exhausted:** Yes (3 passing alternatives found)
+
+**Selected Fix:** PR's fix - Itβs simplest and sufficient.
+- #3 (Tag) is the most "correct" for robustness but adds Tag management overhead.
+- #2 (Dedupe) is good for cleanup.
+- PR/#1 (Index 0) are standard for this codebase's patterns.
+
+**Comparison Notes:**
+- PR/try-fix #1 rely on `GetChildAt(0)` being the title icon view when present
+- try-fix #2 is more defensive: it collapses existing duplicates regardless of child index and then reuses/creates as needed
+- try-fix #3 uses explicit tagging: precise but introduces new state (Tag) to manage
+
+
+
+---
+
+**Next Step:** Propose Alternative Fix #2 (Dedupe & Scan) to Author for Discussion
+
+---
+
+## π¬ Draft Comment for Author
+
+Hi @PureWeen,
+
+Reviewing the fix in this PR, it works correctly for the reported issue and tests pass.
+
+I explored a couple of alternative approaches and found one that might offer slightly better robustness against edge cases, which I wanted to run by you:
+
+**Alternative: Dedupe & Scan**
+Instead of just checking index 0, we could scan all children of the toolbar to find any `ToolbarTitleIconImageView` instances.
+
+```csharp
+// Scan all children to find existing title icons
+ToolbarTitleIconImageView? titleIcon = null;
+for (int i = 0; i < nativeToolbar.ChildCount; i++)
+{
+ var child = nativeToolbar.GetChildAt(i);
+ if (child is ToolbarTitleIconImageView icon)
+ {
+ if (titleIcon == null)
+ titleIcon = icon; // Keep the first one found
+ else
+ nativeToolbar.RemoveView(icon); // Remove any extras (self-healing)
+ }
+}
+```
+
+**Why consider this?**
+1. **Robustness against Injection:** If another library inserts a view at index 0 (e.g., search bar), the current PR fix (checking only index 0) would fail to see the existing icon and create a duplicate.
+2. **Self-Healing:** If the toolbar is already in a bad state (multiple icons from previous bugs), this approach cleans them up.
+
+**Trade-off:**
+It involves a loop, so O(N) instead of O(1), but for a toolbar with very few items, this is negligible.
+
+Do you think the added robustness is worth the change, or should we stick to the simpler Index 0 check (current PR) which matches the existing removal logic?
+
+---
+
+## π Final Report
+
+### Summary
+
+PR #31487 correctly fixes the duplicate title icon issue on Android. The fix checks for an existing `ToolbarTitleIconImageView` at position 0 before creating a new one, preventing duplicate icons when `SetTitleIconImageSource` is called multiple times.
+
+### Root Cause
+
+The original `UpdateTitleIcon` method always created a new `ToolbarTitleIconImageView` and added it to position 0, without checking if one already existed. This caused duplicate icons when the method was called repeatedly.
+
+### Validation
+
+| Check | Result |
+|-------|--------|
+| Tests reproduce bug | β
Test fails without fix (duplicate icons) |
+| Tests pass with fix | β
Test passes with fix (single icon) |
+| Independent fix analysis | β
try-fix arrived at same solution |
+| Code quality | β
Clean, minimal change |
+
+### Regression Analysis
+
+
+π Git History Analysis
+
+**Original Implementation:** `e2f3aaa222` (Oct 2021) by Shane Neuville
+- Part of "[Android] ToolbarHandler and fixes for various page nesting scenarios (#2781)"
+- The bug has existed since the original implementation - it was never designed to handle repeated calls
+
+**Key Finding:** The original code had a check for removing an existing icon when source is null/empty:
+```csharp
+if (nativeToolbar.GetChildAt(0) is ToolbarTitleIconImageView existingImageView)
+ nativeToolbar.RemoveView(existingImageView);
+```
+But this check was **only in the removal path**, not in the creation path. The fix extends this pattern to also check before adding.
+
+**Related Toolbar Issues in This File:**
+| Commit | Issue | Description |
+|--------|-------|-------------|
+| `a93e88c3de` | #7823 | Fix toolbar item icon not removed when navigating |
+| `c04b7d79cc` | #19673 | Fixed android toolbar icon change |
+| `158ed8b4f1` | #28767 | Removing outdated menu items after activity switch |
+
+**Pattern:** Multiple fixes in this file address issues where Android toolbar state isn't properly cleaned up or reused. This PR follows the same pattern.
+
+
+
+
+π Platform Comparison
+
+| Platform | TitleIcon Implementation | Duplicate Prevention |
+|----------|-------------------------|---------------------|
+| **Android** | Creates `ToolbarTitleIconImageView`, adds to position 0 | β Was missing (now fixed by PR) |
+| **Windows** | Sets `TitleIconImageSource` property directly | β
Property-based, no duplicates possible |
+| **iOS** | Uses `NavigationRenderer` with property binding | β
Property-based approach |
+
+**Why Android was vulnerable:** Android uses a view-based approach (adding/removing child views) while other platforms use property-based approaches. View management requires explicit duplicate checks.
+
+
+
+
+β οΈ Risk Assessment
+
+**Regression Risk: LOW**
+
+1. **Minimal change** - Only modifies the creation logic, doesn't change removal
+2. **Consistent pattern** - Uses same `GetChildAt(0)` check that already existed for removal
+3. **Well-tested** - UI test verifies the specific scenario
+4. **No side effects** - Reusing existing view is safe; `SetImageDrawable` handles updates
+
+**Potential Edge Cases (from Copilot review suggestion):**
+- Setting different image sources rapidly β Should work fine, image is updated on existing view
+- Setting same source multiple times β Explicitly tested, works correctly
+
+
+
+### Recommendation
+
+**β
APPROVE** - The PR's approach is correct and validated by independent analysis. The fix is minimal, focused, and addresses the root cause.
diff --git a/.github/agent-pr-session/pr-33406.md b/.github/agent-pr-session/pr-33406.md
new file mode 100644
index 000000000000..e79aca2901f5
--- /dev/null
+++ b/.github/agent-pr-session/pr-33406.md
@@ -0,0 +1,247 @@
+# PR Review: #33406 - [iOS] Fixed Shell navigation on search handler suggestion selection
+
+**Date:** 2026-01-08 | **Issue:** [#33356](https://github.com/dotnet/maui/issues/33356) | **PR:** [#33406](https://github.com/dotnet/maui/pull/33406)
+
+**Related Prior Attempt:** [PR #33396](https://github.com/dotnet/maui/pull/33396) (closed - Copilot CLI attempt)
+
+## β³ Status: IN PROGRESS
+
+| Phase | Status |
+|-------|--------|
+| Pre-Flight | β
COMPLETE |
+| π§ͺ Tests | β³ PENDING |
+| π¦ Gate | β³ PENDING |
+| π§ Fix | β³ PENDING |
+| π Report | β³ PENDING |
+
+---
+
+
+π Issue Summary
+
+**Issue #33356**: [iOS] Clicking on search suggestions fails to navigate to detail page correctly
+
+**Bug Description**: Clicking on a search suggestion using NavigationBar/SearchBar/custom SearchHandler does not navigate to the detail page correctly on iOS 26.1 & 26.2 with MAUI 10.
+
+**Root Cause (from PR #33406)**: Navigation fails because `UISearchController` was dismissed (`Active = false`) BEFORE `ItemSelected` was called. This triggers a UIKit transition that deactivates the Shell navigation context and prevents the navigation from completing.
+
+**Reproduction App**: https://github.com/dotnet/maui-samples/tree/main/10.0/Fundamentals/Shell/Xaminals
+
+**Steps to Reproduce:**
+1. Open the Xaminals sample app
+2. Deploy to iPhone 17 Pro 26.2 simulator (Xcode 26.2)
+3. Put focus on the search box
+4. Type 'b' (note: search dropdown position is wrong - see Issue #32930)
+5. Click on 'Bengal' in search suggestions
+6. **Issue 1:** No navigation happens (expected: navigate to Bengal cat detail page)
+7. Click on 'Bengal' from the main list - this works correctly
+8. Click back button
+9. **Issue 2:** Navigates to an empty page (expected: navigate back to list)
+10. Click back button again - actually navigates back
+
+**Platforms Affected:**
+- [ ] Android
+- [x] iOS (26.1 & 26.2)
+- [ ] Windows
+- [ ] MacCatalyst
+
+**Regression Info:**
+- **Confirmed regression** starting in version 9.0.90
+- Labels: `t/bug`, `platform/ios`, `s/verified`, `s/triaged`, `i/regression`, `shell-search-handler`, `regressed-in-9.0.90`
+- Issue 2 (empty page on back navigation) specifically reproducible from 9.0.90
+
+**Validated by:** TamilarasanSF4853 (Syncfusion partner) - Confirmed reproducible in VS Code 1.107.1 with MAUI versions 9.0.0, 9.0.82, 9.0.90, 9.0.120, 10.0.0, and 10.0.20 on iOS.
+
+
+
+
+π Files Changed - PR #33406 (Community PR)
+
+| File | Type | Changes |
+|------|------|---------|
+| `src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs` | Fix | 2 lines (swap order) |
+| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.cs` | Test (HostApp) | +261 lines |
+| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs` | Test (NUnit) | +46 lines |
+
+**PR #33406 Fix** (simpler approach - just swap order):
+```diff
+ void OnSearchItemSelected(object? sender, object e)
+ {
+ if (_searchController is null)
+ return;
+
+- _searchController.Active = false;
+ (SearchHandler as ISearchHandlerController)?.ItemSelected(e);
++ _searchController.Active = false;
+ }
+```
+
+
+
+
+π Files Changed - PR #33396 (Prior Copilot Attempt - CLOSED)
+
+| File | Type | Changes |
+|------|------|---------|
+| `.github/agent-pr-session/pr-33396.md` | Session | +210 lines |
+| `src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs` | Fix | +17 lines |
+| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml` | Test (XAML) | +41 lines |
+| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml.cs` | Test (C#) | +138 lines |
+| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs` | Test (NUnit) | +70 lines |
+
+**PR #33396 Fix** (more defensive approach with BeginInvokeOnMainThread):
+```diff
+ void OnSearchItemSelected(object? sender, object e)
+ {
+ if (_searchController is null)
+ return;
+
++ // Store the search controller reference before any state changes
++ var searchController = _searchController;
++
++ // Call ItemSelected first to trigger navigation before dismissing the search UI.
++ // On iOS 26+, setting Active = false before navigation can cause the navigation
++ // to be lost due to the search controller dismissal animation.
+ (SearchHandler as ISearchHandlerController)?.ItemSelected(e);
+- _searchController.Active = false;
++
++ // Deactivate the search controller after navigation has been initiated.
++ // Using BeginInvokeOnMainThread ensures this happens after the current run loop,
++ // allowing the navigation to proceed without interference from the dismissal animation.
++ ViewController?.BeginInvokeOnMainThread(() =>
++ {
++ if (searchController is not null)
++ {
++ searchController.Active = false;
++ }
++ });
+ }
+```
+
+
+
+
+π¬ Discussion Summary
+
+**Key Comments from Issue #33356:**
+- TamilarasanSF4853 (Syncfusion): Validated issue across multiple MAUI versions (9.0.0 through 10.0.20)
+- Issue 2 (empty page on back) specifically regressed in 9.0.90
+- Issue 1 (no navigation on search suggestion tap) affects all tested versions on iOS
+
+**PR #33406 Review Comments:**
+- Copilot PR reviewer caught typo: "searchHander" should be "searchHandler" (5 duplicate comments, all resolved/outdated now)
+- Prior agent review by kubaflo marked it as β
APPROVE with comprehensive analysis
+- PureWeen requested `/rebase` (latest comment)
+
+**PR #33396 Review Comments:**
+- PureWeen asked to update state file to match PR number
+- Copilot had firewall issues accessing GitHub API
+
+**Disagreements to Investigate:**
+| File:Line | Reviewer Says | Author Says | Status |
+|-----------|---------------|-------------|--------|
+| N/A | N/A | N/A | No active disagreements |
+
+**Author Uncertainty:**
+- None noted in either PR
+
+
+
+
+βοΈ Comparison: PR #33406 vs PR #33396
+
+### Fix Approach Comparison
+
+| Aspect | PR #33406 (Community) | PR #33396 (Copilot) |
+|--------|----------------------|---------------------|
+| **Author** | SubhikshaSf4851 (Syncfusion) | Copilot |
+| **Status** | Open | Closed (draft) |
+| **Lines Changed** | 2 (swap order) | 17 (more defensive) |
+| **Fix Strategy** | Simply swap order of operations | Swap order + dispatch to next run loop |
+| **Test Style** | Code-only (no XAML) | XAML + code-behind |
+| **Test Count** | 1 test method | 2 test methods |
+
+### Which Fix is Better?
+
+**PR #33406 (simpler approach):**
+- β
Minimal change - just swaps two lines
+- β
Addresses root cause: ItemSelected called while navigation context is valid
+- β οΈ Dismissal happens synchronously after ItemSelected
+- β οΈ Could theoretically still interfere if dismissal animation is fast
+
+**PR #33396 (defensive approach):**
+- β
Uses BeginInvokeOnMainThread for explicit async deactivation
+- β
Stores reference to search controller before state changes
+- β
More detailed comments explaining the fix
+- β οΈ More code complexity
+- β οΈ Was closed/abandoned
+
+### Recommendation
+
+Both approaches should work. PR #33406 is simpler and has been reviewed/approved. The extra defensive measures in PR #33396 (BeginInvokeOnMainThread) may provide additional safety margin but add complexity.
+
+**Prior agent review on PR #33406** already verified:
+- Tests FAIL without fix (bug reproduced - timeout)
+- Tests PASS with fix (navigation successful)
+
+
+
+
+π§ͺ Tests
+
+**Status**: β³ PENDING (need to verify tests compile and reproduce issue)
+
+**PR #33406 Tests:**
+- HostApp: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.cs` (code-only, no XAML)
+- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs`
+- 1 test: `Issue33356NavigateShouldOccur` - Tests search handler navigation AND back navigation + collection view navigation
+
+**PR #33396 Tests (for reference):**
+- HostApp XAML: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml`
+- HostApp Code: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml.cs`
+- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs`
+- 2 tests: `SearchSuggestionTapNavigatesToDetailPage`, `BackNavigationFromDetailPageWorks`
+
+**Test Checklist:**
+- [ ] PR includes UI tests
+- [ ] Tests reproduce the issue
+- [ ] Tests follow naming convention (`Issue33356`)
+
+
+
+
+π¦ Gate - Test Verification
+
+**Status**: β³ PENDING
+
+- [ ] Tests FAIL without fix (bug reproduced)
+- [ ] Tests PASS with fix (fix validated)
+
+**Prior Agent Review Result (kubaflo on PR #33406):**
+```
+WITHOUT FIX: FAILED - System.TimeoutException: Timed out waiting for element "Issue33356CatNameLabel"
+WITH FIX: PASSED - All 1 tests passed in 21.73 seconds
+```
+
+**Result:** [PENDING - needs re-verification]
+
+
+
+
+π§ Fix Candidates
+
+**Status**: β³ PENDING
+
+| # | Source | Approach | Test Result | Files Changed | Notes |
+|---|--------|----------|-------------|---------------|-------|
+| PR | PR #33406 | Swap order: ItemSelected before Active=false | β³ PENDING (Gate) | `ShellPageRendererTracker.cs` (2 lines) | Current PR - simpler fix |
+| Alt | PR #33396 | Swap order + BeginInvokeOnMainThread | β
VERIFIED (prior test) | `ShellPageRendererTracker.cs` (17 lines) | Prior attempt - more defensive |
+
+**Exhausted:** No
+**Selected Fix:** [PENDING]
+
+
+
+---
+
+**Next Step:** Verify PR #33406 tests compile and Gate passes. Read `.github/agents/pr/post-gate.md` after Gate passes.
diff --git a/.github/agents/pr.md b/.github/agents/pr.md
index 36a8026e4d93..6f0cec11f12a 100644
--- a/.github/agents/pr.md
+++ b/.github/agents/pr.md
@@ -216,7 +216,7 @@ This file:
- Serves as your TODO list for all phases
- Tracks progress if interrupted
- Must exist before you start gathering context
-- Gets committed to `.github/agent-pr-session/` directory
+- **Always include when committing changes** (to `.github/agent-pr-session/`)
- **Phases 4-5 sections are added AFTER Gate passes** (see `pr/post-gate.md`)
**Then gather context and update the file as you go.**
diff --git a/.github/instructions/uitests.instructions.md b/.github/instructions/uitests.instructions.md
index ebab7a6aa524..c0778ec7bb33 100644
--- a/.github/instructions/uitests.instructions.md
+++ b/.github/instructions/uitests.instructions.md
@@ -167,47 +167,36 @@ grep -E "public const string [A-Za-z]+ = " src/Controls/tests/TestCases.Shared.T
### Default Behavior
-**DO NOT** use platform-specific conditional compilation directives (`#if ANDROID`, `#if IOS`, `#if WINDOWS`, `#if MACCATALYST`, etc.) unless there is a specific reason.
+Tests should run on all applicable platforms by default. The test infrastructure handles platform detection automatically.
-Tests in the `TestCases.Shared.Tests` project should run on all applicable platforms by default. The test infrastructure automatically handles platform detection.
+### No Inline #if Directives in Test Methods
-### When Platform Directives Are Acceptable
+**Do NOT use `#if ANDROID`, `#if IOS`, etc. directly in test methods.** Platform-specific behavior must be hidden behind extension methods for readability.
-Only use platform-specific directives when:
+**Note:** This rule is about **code cleanliness**, not platform scope. Using `#if ANDROID ... #else ...` still compiles for all platforms - the issue is that inline directives make test logic hard to read and maintain.
-1. **Platform-specific API is being tested** - The test validates behavior that only exists on one platform
-2. **Known platform limitation** - There is a documented bug or limitation that prevents the test from running on a specific platform
-3. **Different expected behavior** - The platforms are expected to behave differently for valid reasons
-
-### Examples
-
-**β
Correct - Runs on all platforms:**
```csharp
+// β BAD - inline #if in test method (hard to read)
[Test]
-[Category(UITestCategories.SafeAreaEdges)]
-public void SoftInputBehaviorTest()
+public void MyTest()
{
- // This test runs on all applicable platforms
- App.WaitForElement("ContentGrid");
- // Test code...
+#if ANDROID
+ App.TapCoordinates(100, 200);
+#else
+ App.Tap("MyElement");
+#endif
}
-```
-**β Incorrect - Unnecessarily limits to one platform:**
-```csharp
-#if ANDROID
+// β
GOOD - platform logic in extension method (clean test)
[Test]
-[Category(UITestCategories.SafeAreaEdges)]
-public void SoftInputBehaviorTest()
+public void MyTest()
{
- // This unnecessarily limits the test to Android only
- // Unless there's a specific reason, tests should run on all platforms
- App.WaitForElement("ContentGrid");
- // Test code...
+ App.TapElementCrossPlatform("MyElement");
}
-#endif
```
+Move platform-specific logic to extension methods to keep test code clean and readable.
+
## Running UI Tests Locally
**CRITICAL: ALWAYS use the BuildAndRunHostApp.ps1 script to run UI tests. NEVER run `dotnet test` or `dotnet build` commands manually.**
@@ -330,9 +319,8 @@ Verify the following checklist before committing UI tests:
- [ ] Ensure file names follow the `IssueXXXXX` pattern and match between projects
- [ ] Ensure test methods have descriptive names
- [ ] Verify test inherits from `_IssuesUITest`
-- [ ] Confirm only ONE `[Category]` attribute is used per test
-- [ ] Verify tests run on all applicable platforms (iOS, Android, Windows, MacCatalyst) unless platform-specific
-- [ ] Document any platform-specific limitations with clear comments
+- [ ] Confirm only ONE `[Category]` attribute per test
+- [ ] No inline `#if` directives in test code (use extension methods)
- [ ] Test passes locally on at least one platform
### Test State Management
diff --git a/.github/skills/try-fix/SKILL.md b/.github/skills/try-fix/SKILL.md
index 4b414c4fd695..c266f3e1dabc 100644
--- a/.github/skills/try-fix/SKILL.md
+++ b/.github/skills/try-fix/SKILL.md
@@ -171,16 +171,16 @@ Add a new row to the **Fix Candidates** table in the state file:
**For PASSING fixes:**
```markdown
-| # | Source | Approach | Test Result | Files Changed | Notes |
-|---|--------|----------|-------------|---------------|-------|
-| N | try-fix | [Your approach] | β
PASS | `file.cs` (+X) | Works! [any observations] |
+| # | Source | Approach | Test Result | Files Changed | Model | Notes |
+|---|--------|----------|-------------|---------------|-------|-------|
+| N | try-fix | [Your approach] | β
PASS | `file.cs` (+X) | [Model Name] | Works! [any observations] |
```
**For FAILING fixes (include failure analysis):**
```markdown
-| # | Source | Approach | Test Result | Files Changed | Notes |
-|---|--------|----------|-------------|---------------|-------|
-| N | try-fix | [Your approach] | β FAIL | `file.cs` (+X) | **Why failed:** [Analysis of flawed reasoning and what you learned] |
+| # | Source | Approach | Test Result | Files Changed | Model | Notes |
+|---|--------|----------|-------------|---------------|-------|-------|
+| N | try-fix | [Your approach] | β FAIL | `file.cs` (+X) | [Model Name] | **Why failed:** [Analysis of flawed reasoning and what you learned] |
```
### Step 10: Revert Everything
@@ -213,12 +213,12 @@ The state file should have this section:
```markdown
## Fix Candidates
-| # | Source | Approach | Test Result | Files Changed | Notes |
-|---|--------|----------|-------------|---------------|-------|
-| 1 | try-fix | Fix in TabbedPageManager | β FAIL | `TabbedPageManager.cs` (+5) | **Why failed:** Too late in lifecycle - by the time OnPageSelected fires, layout already measured with stale values |
-| 2 | try-fix | RequestApplyInsets only | β FAIL | `ToolbarExtensions.cs` (+2) | **Why failed:** Trigger alone insufficient - calculation logic still used cached values |
-| 3 | try-fix | Reset cache + RequestApplyInsets | β
PASS | `ToolbarExtensions.cs`, `InsetListener.cs` (+8) | Works! Similar to PR's approach |
-| PR | PR #XXXXX | [PR's approach] | β
PASS (Gate) | [files] | Original PR - validated by Gate |
+| # | Source | Approach | Test Result | Files Changed | Model | Notes |
+|---|--------|----------|-------------|---------------|-------|-------|
+| 1 | try-fix | Fix in TabbedPageManager | β FAIL | `TabbedPageManager.cs` (+5) | Claude 3.5 Sonnet | **Why failed:** Too late in lifecycle - by the time OnPageSelected fires, layout already measured with stale values |
+| 2 | try-fix | RequestApplyInsets only | β FAIL | `ToolbarExtensions.cs` (+2) | Claude 3.5 Sonnet | **Why failed:** Trigger alone insufficient - calculation logic still used cached values |
+| 3 | try-fix | Reset cache + RequestApplyInsets | β
PASS | `ToolbarExtensions.cs`, `InsetListener.cs` (+8) | Claude 3.5 Sonnet | Works! Similar to PR's approach |
+| PR | PR #XXXXX | [PR's approach] | β
PASS (Gate) | [files] | Author | Original PR - validated by Gate |
**Exhausted:** Yes
**Selected Fix:** #3 or PR - both work, compare for simplicity
@@ -271,9 +271,9 @@ This helps the next try-fix invocation avoid the same mistake.
```markdown
## Fix Candidates
-| # | Source | Approach | Test Result | Files Changed | Notes |
-|---|--------|----------|-------------|---------------|-------|
-| PR | PR #33359 | RequestApplyInsets + reset appBarHasContent | β
PASS (Gate) | 2 files | Original PR |
+| # | Source | Approach | Test Result | Files Changed | Model | Notes |
+|---|--------|----------|-------------|---------------|-------|-------|
+| PR | PR #33359 | RequestApplyInsets + reset appBarHasContent | β
PASS (Gate) | 2 files | Author | Original PR |
**Exhausted:** No
**Selected Fix:** [PENDING]
@@ -294,10 +294,10 @@ This helps the next try-fix invocation avoid the same mistake.
```markdown
## Fix Candidates
-| # | Source | Approach | Test Result | Files Changed | Notes |
-|---|--------|----------|-------------|---------------|-------|
-| 1 | try-fix | Fix in TabbedPageManager.OnPageSelected | β FAIL | `TabbedPageManager.cs` (+5) | **Why failed:** Too late in lifecycle - OnPageSelected fires after layout measured |
-| PR | PR #33359 | RequestApplyInsets + reset appBarHasContent | β
PASS (Gate) | 2 files | Original PR |
+| # | Source | Approach | Test Result | Files Changed | Model | Notes |
+|---|--------|----------|-------------|---------------|-------|-------|
+| 1 | try-fix | Fix in TabbedPageManager.OnPageSelected | β FAIL | `TabbedPageManager.cs` (+5) | Claude 3.5 Sonnet | **Why failed:** Too late in lifecycle - OnPageSelected fires after layout measured |
+| PR | PR #33359 | RequestApplyInsets + reset appBarHasContent | β
PASS (Gate) | 2 files | Author | Original PR |
**Exhausted:** No
**Selected Fix:** [PENDING]
@@ -317,11 +317,11 @@ This helps the next try-fix invocation avoid the same mistake.
```markdown
## Fix Candidates
-| # | Source | Approach | Test Result | Files Changed | Notes |
-|---|--------|----------|-------------|---------------|-------|
-| 1 | try-fix | Fix in TabbedPageManager.OnPageSelected | β FAIL | `TabbedPageManager.cs` (+5) | **Why failed:** Too late in lifecycle |
-| 2 | try-fix | RequestApplyInsets in UpdateIsVisible | β
PASS | `ToolbarExtensions.cs` (+2) | Works! Simpler than PR (1 file vs 2) |
-| PR | PR #33359 | RequestApplyInsets + reset appBarHasContent | β
PASS (Gate) | 2 files | Original PR |
+| # | Source | Approach | Test Result | Files Changed | Model | Notes |
+|---|--------|----------|-------------|---------------|-------|-------|
+| 1 | try-fix | Fix in TabbedPageManager.OnPageSelected | β FAIL | `TabbedPageManager.cs` (+5) | Claude 3.5 Sonnet | **Why failed:** Too late in lifecycle |
+| 2 | try-fix | RequestApplyInsets in UpdateIsVisible | β
PASS | `ToolbarExtensions.cs` (+2) | Claude 3.5 Sonnet | Works! Simpler than PR (1 file vs 2) |
+| PR | PR #33359 | RequestApplyInsets + reset appBarHasContent | β
PASS (Gate) | 2 files | Author | Original PR |
**Exhausted:** No
**Selected Fix:** [PENDING]
From ee6627cccc8b193be1a3930602bf700fec9cfba4 Mon Sep 17 00:00:00 2001
From: Shane Neuville
Date: Thu, 8 Jan 2026 17:54:54 -0600
Subject: [PATCH 3/5] Delete .github/agent-pr-session/pr-31487.md
---
.github/agent-pr-session/pr-31487.md | 261 ---------------------------
1 file changed, 261 deletions(-)
delete mode 100644 .github/agent-pr-session/pr-31487.md
diff --git a/.github/agent-pr-session/pr-31487.md b/.github/agent-pr-session/pr-31487.md
deleted file mode 100644
index b15821b1c3a2..000000000000
--- a/.github/agent-pr-session/pr-31487.md
+++ /dev/null
@@ -1,261 +0,0 @@
-# PR Review: #31487 - [Android] Fixed duplicate title icon when setting TitleIconImageSource Multiple times
-
-**Date:** 2026-01-08 | **Issue:** [#31445](https://github.com/dotnet/maui/issues/31445) | **PR:** [#31487](https://github.com/dotnet/maui/pull/31487)
-
-## β
Final Recommendation: APPROVE
-
-| Phase | Status |
-|-------|--------|
-| Pre-Flight | β
COMPLETE |
-| π§ͺ Tests | β
COMPLETE |
-| π¦ Gate | β
PASSED |
-| π§ Fix | β
COMPLETE |
-| π Report | β
COMPLETE |
-
----
-
-
-π Issue Summary
-
-On Android, calling `NavigationPage.SetTitleIconImageSource(page, "image.png")` more than once for the same page results in the icon being rendered multiple times in the navigation bar.
-
-**Steps to Reproduce:**
-1. Launch app on Android
-2. Tap "Set TitleIconImageSource" once: icon appears
-3. Tap it again: a second identical icon appears
-
-**Expected:** Single toolbar icon regardless of how many times SetTitleIconImageSource is called.
-
-**Actual:** Each repeated call adds an additional duplicate icon.
-
-**Platforms Affected:**
-- [ ] iOS
-- [x] Android
-- [ ] Windows
-- [ ] MacCatalyst
-
-**Version:** 9.0.100 SR10
-
-
-
-
-π Files Changed
-
-| File | Type | Changes |
-|------|------|---------|
-| `src/Controls/src/Core/Platform/Android/Extensions/ToolbarExtensions.cs` | Fix | +17/-6 |
-| `src/Controls/tests/TestCases.HostApp/Issues/Issue31445.cs` | Test | +38 |
-| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31445.cs` | Test | +23 |
-| `snapshots/android/Issue31445DuplicateTitleIconDoesNotAppear.png` | Snapshot | binary |
-| `snapshots/mac/Issue31445DuplicateTitleIconDoesNotAppear.png` | Snapshot | binary |
-| `snapshots/windows/Issue31445DuplicateTitleIconDoesNotAppear.png` | Snapshot | binary |
-| `snapshots/ios/Issue31445DuplicateTitleIconDoesNotAppear.png` | Snapshot | binary |
-
-
-
-
-π¬ PR Discussion Summary
-
-**Key Comments:**
-- Issue verified by LogishaSelvarajSF4525 on MAUI 9.0.0 & 9.0.100
-- PR triggered UI tests by jsuarezruiz
-- PureWeen requested rebase
-
-**Reviewer Feedback:**
-- Copilot review: Suggested testing with different image sources or rapid succession to validate fix better
-
-**Disagreements to Investigate:**
-| File:Line | Reviewer Says | Author Says | Status |
-|-----------|---------------|-------------|--------|
-| Issue31445.cs:31 | Test with different images or rapid calls | N/A | β οΈ INVESTIGATE |
-
-**Author Uncertainty:**
-- None noted
-
-
-
-
-π§ͺ Tests
-
-**Status**: β
COMPLETE
-
-- [x] PR includes UI tests
-- [x] Tests reproduce the issue
-- [x] Tests follow naming convention (`Issue31445`)
-
-**Test Files:**
-- HostApp: `src/Controls/tests/TestCases.HostApp/Issues/Issue31445.cs`
-- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31445.cs`
-
-**Test Behavior:**
-- Uses snapshot verification (`VerifyScreenshot()`)
-- Navigates to test page, taps button to trigger duplicate icon scenario
-- Verified to compile successfully
-
-
-
-
-π¦ Gate - Test Verification
-
-**Status**: β
PASSED
-
-- [x] Tests FAIL without fix (bug reproduced - duplicate icons appeared)
-- [x] Tests PASS with fix (single icon as expected)
-
-**Result:** PASSED β
-
-**Verification Details:**
-- Platform: Android (emulator-5554)
-- Without fix: Test FAILED (screenshot mismatch - duplicate icons)
-- With fix: Test PASSED (single icon verified)
-
-
-
-
-π§ Fix Candidates
-
-**Status**: β
COMPLETE
-
-| # | Source | Approach | Test Result | Files Changed | Model | Notes |
-|---|--------|----------|-------------|---------------|-------|-------|
-| 1 | try-fix | Check for existing icon view at position 0, reuse if exists, only create new if needed | β
PASS | `ToolbarExtensions.cs` (+7) | Opus 4.5 | Works! Independently arrived at same solution logic as PR |
-| 2 | try-fix | Dedupe defensively by scanning all toolbar children, keep first `ToolbarTitleIconImageView`, remove extras; then reuse/create | β
PASS | `ToolbarExtensions.cs` (+22/-5) | GPT 5.2 | More robust if child ordering changes or duplicates already exist |
-| 3 | try-fix | Use `FindViewWithTag` to uniquely identify/retrieve the MAUI title icon | β
PASS | `ToolbarExtensions.cs` (+20/-6) | Gemini 2.0 Flash | Explicit identification; avoids index assumptions and iteration; most robust against external view insertions |
-| PR | PR #31487 | Check for existing ToolbarTitleIconImageView before adding new one | β
PASS (Gate) | `ToolbarExtensions.cs` (+17/-6) | Author | Original PR - validated by Gate |
-
-**Exhausted:** Yes (3 passing alternatives found)
-
-**Selected Fix:** PR's fix - Itβs simplest and sufficient.
-- #3 (Tag) is the most "correct" for robustness but adds Tag management overhead.
-- #2 (Dedupe) is good for cleanup.
-- PR/#1 (Index 0) are standard for this codebase's patterns.
-
-**Comparison Notes:**
-- PR/try-fix #1 rely on `GetChildAt(0)` being the title icon view when present
-- try-fix #2 is more defensive: it collapses existing duplicates regardless of child index and then reuses/creates as needed
-- try-fix #3 uses explicit tagging: precise but introduces new state (Tag) to manage
-
-
-
----
-
-**Next Step:** Propose Alternative Fix #2 (Dedupe & Scan) to Author for Discussion
-
----
-
-## π¬ Draft Comment for Author
-
-Hi @PureWeen,
-
-Reviewing the fix in this PR, it works correctly for the reported issue and tests pass.
-
-I explored a couple of alternative approaches and found one that might offer slightly better robustness against edge cases, which I wanted to run by you:
-
-**Alternative: Dedupe & Scan**
-Instead of just checking index 0, we could scan all children of the toolbar to find any `ToolbarTitleIconImageView` instances.
-
-```csharp
-// Scan all children to find existing title icons
-ToolbarTitleIconImageView? titleIcon = null;
-for (int i = 0; i < nativeToolbar.ChildCount; i++)
-{
- var child = nativeToolbar.GetChildAt(i);
- if (child is ToolbarTitleIconImageView icon)
- {
- if (titleIcon == null)
- titleIcon = icon; // Keep the first one found
- else
- nativeToolbar.RemoveView(icon); // Remove any extras (self-healing)
- }
-}
-```
-
-**Why consider this?**
-1. **Robustness against Injection:** If another library inserts a view at index 0 (e.g., search bar), the current PR fix (checking only index 0) would fail to see the existing icon and create a duplicate.
-2. **Self-Healing:** If the toolbar is already in a bad state (multiple icons from previous bugs), this approach cleans them up.
-
-**Trade-off:**
-It involves a loop, so O(N) instead of O(1), but for a toolbar with very few items, this is negligible.
-
-Do you think the added robustness is worth the change, or should we stick to the simpler Index 0 check (current PR) which matches the existing removal logic?
-
----
-
-## π Final Report
-
-### Summary
-
-PR #31487 correctly fixes the duplicate title icon issue on Android. The fix checks for an existing `ToolbarTitleIconImageView` at position 0 before creating a new one, preventing duplicate icons when `SetTitleIconImageSource` is called multiple times.
-
-### Root Cause
-
-The original `UpdateTitleIcon` method always created a new `ToolbarTitleIconImageView` and added it to position 0, without checking if one already existed. This caused duplicate icons when the method was called repeatedly.
-
-### Validation
-
-| Check | Result |
-|-------|--------|
-| Tests reproduce bug | β
Test fails without fix (duplicate icons) |
-| Tests pass with fix | β
Test passes with fix (single icon) |
-| Independent fix analysis | β
try-fix arrived at same solution |
-| Code quality | β
Clean, minimal change |
-
-### Regression Analysis
-
-
-π Git History Analysis
-
-**Original Implementation:** `e2f3aaa222` (Oct 2021) by Shane Neuville
-- Part of "[Android] ToolbarHandler and fixes for various page nesting scenarios (#2781)"
-- The bug has existed since the original implementation - it was never designed to handle repeated calls
-
-**Key Finding:** The original code had a check for removing an existing icon when source is null/empty:
-```csharp
-if (nativeToolbar.GetChildAt(0) is ToolbarTitleIconImageView existingImageView)
- nativeToolbar.RemoveView(existingImageView);
-```
-But this check was **only in the removal path**, not in the creation path. The fix extends this pattern to also check before adding.
-
-**Related Toolbar Issues in This File:**
-| Commit | Issue | Description |
-|--------|-------|-------------|
-| `a93e88c3de` | #7823 | Fix toolbar item icon not removed when navigating |
-| `c04b7d79cc` | #19673 | Fixed android toolbar icon change |
-| `158ed8b4f1` | #28767 | Removing outdated menu items after activity switch |
-
-**Pattern:** Multiple fixes in this file address issues where Android toolbar state isn't properly cleaned up or reused. This PR follows the same pattern.
-
-
-
-
-π Platform Comparison
-
-| Platform | TitleIcon Implementation | Duplicate Prevention |
-|----------|-------------------------|---------------------|
-| **Android** | Creates `ToolbarTitleIconImageView`, adds to position 0 | β Was missing (now fixed by PR) |
-| **Windows** | Sets `TitleIconImageSource` property directly | β
Property-based, no duplicates possible |
-| **iOS** | Uses `NavigationRenderer` with property binding | β
Property-based approach |
-
-**Why Android was vulnerable:** Android uses a view-based approach (adding/removing child views) while other platforms use property-based approaches. View management requires explicit duplicate checks.
-
-
-
-
-β οΈ Risk Assessment
-
-**Regression Risk: LOW**
-
-1. **Minimal change** - Only modifies the creation logic, doesn't change removal
-2. **Consistent pattern** - Uses same `GetChildAt(0)` check that already existed for removal
-3. **Well-tested** - UI test verifies the specific scenario
-4. **No side effects** - Reusing existing view is safe; `SetImageDrawable` handles updates
-
-**Potential Edge Cases (from Copilot review suggestion):**
-- Setting different image sources rapidly β Should work fine, image is updated on existing view
-- Setting same source multiple times β Explicitly tested, works correctly
-
-
-
-### Recommendation
-
-**β
APPROVE** - The PR's approach is correct and validated by independent analysis. The fix is minimal, focused, and addresses the root cause.
From dcb5427d291bc4fdd0f489f063ee9de152705b93 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Fri, 9 Jan 2026 00:03:52 +0000
Subject: [PATCH 4/5] Remove agent session state file pr-33406.md
Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
---
.github/agent-pr-session/pr-33406.md | 247 ---------------------------
1 file changed, 247 deletions(-)
delete mode 100644 .github/agent-pr-session/pr-33406.md
diff --git a/.github/agent-pr-session/pr-33406.md b/.github/agent-pr-session/pr-33406.md
deleted file mode 100644
index e79aca2901f5..000000000000
--- a/.github/agent-pr-session/pr-33406.md
+++ /dev/null
@@ -1,247 +0,0 @@
-# PR Review: #33406 - [iOS] Fixed Shell navigation on search handler suggestion selection
-
-**Date:** 2026-01-08 | **Issue:** [#33356](https://github.com/dotnet/maui/issues/33356) | **PR:** [#33406](https://github.com/dotnet/maui/pull/33406)
-
-**Related Prior Attempt:** [PR #33396](https://github.com/dotnet/maui/pull/33396) (closed - Copilot CLI attempt)
-
-## β³ Status: IN PROGRESS
-
-| Phase | Status |
-|-------|--------|
-| Pre-Flight | β
COMPLETE |
-| π§ͺ Tests | β³ PENDING |
-| π¦ Gate | β³ PENDING |
-| π§ Fix | β³ PENDING |
-| π Report | β³ PENDING |
-
----
-
-
-π Issue Summary
-
-**Issue #33356**: [iOS] Clicking on search suggestions fails to navigate to detail page correctly
-
-**Bug Description**: Clicking on a search suggestion using NavigationBar/SearchBar/custom SearchHandler does not navigate to the detail page correctly on iOS 26.1 & 26.2 with MAUI 10.
-
-**Root Cause (from PR #33406)**: Navigation fails because `UISearchController` was dismissed (`Active = false`) BEFORE `ItemSelected` was called. This triggers a UIKit transition that deactivates the Shell navigation context and prevents the navigation from completing.
-
-**Reproduction App**: https://github.com/dotnet/maui-samples/tree/main/10.0/Fundamentals/Shell/Xaminals
-
-**Steps to Reproduce:**
-1. Open the Xaminals sample app
-2. Deploy to iPhone 17 Pro 26.2 simulator (Xcode 26.2)
-3. Put focus on the search box
-4. Type 'b' (note: search dropdown position is wrong - see Issue #32930)
-5. Click on 'Bengal' in search suggestions
-6. **Issue 1:** No navigation happens (expected: navigate to Bengal cat detail page)
-7. Click on 'Bengal' from the main list - this works correctly
-8. Click back button
-9. **Issue 2:** Navigates to an empty page (expected: navigate back to list)
-10. Click back button again - actually navigates back
-
-**Platforms Affected:**
-- [ ] Android
-- [x] iOS (26.1 & 26.2)
-- [ ] Windows
-- [ ] MacCatalyst
-
-**Regression Info:**
-- **Confirmed regression** starting in version 9.0.90
-- Labels: `t/bug`, `platform/ios`, `s/verified`, `s/triaged`, `i/regression`, `shell-search-handler`, `regressed-in-9.0.90`
-- Issue 2 (empty page on back navigation) specifically reproducible from 9.0.90
-
-**Validated by:** TamilarasanSF4853 (Syncfusion partner) - Confirmed reproducible in VS Code 1.107.1 with MAUI versions 9.0.0, 9.0.82, 9.0.90, 9.0.120, 10.0.0, and 10.0.20 on iOS.
-
-
-
-
-π Files Changed - PR #33406 (Community PR)
-
-| File | Type | Changes |
-|------|------|---------|
-| `src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs` | Fix | 2 lines (swap order) |
-| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.cs` | Test (HostApp) | +261 lines |
-| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs` | Test (NUnit) | +46 lines |
-
-**PR #33406 Fix** (simpler approach - just swap order):
-```diff
- void OnSearchItemSelected(object? sender, object e)
- {
- if (_searchController is null)
- return;
-
-- _searchController.Active = false;
- (SearchHandler as ISearchHandlerController)?.ItemSelected(e);
-+ _searchController.Active = false;
- }
-```
-
-
-
-
-π Files Changed - PR #33396 (Prior Copilot Attempt - CLOSED)
-
-| File | Type | Changes |
-|------|------|---------|
-| `.github/agent-pr-session/pr-33396.md` | Session | +210 lines |
-| `src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs` | Fix | +17 lines |
-| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml` | Test (XAML) | +41 lines |
-| `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml.cs` | Test (C#) | +138 lines |
-| `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs` | Test (NUnit) | +70 lines |
-
-**PR #33396 Fix** (more defensive approach with BeginInvokeOnMainThread):
-```diff
- void OnSearchItemSelected(object? sender, object e)
- {
- if (_searchController is null)
- return;
-
-+ // Store the search controller reference before any state changes
-+ var searchController = _searchController;
-+
-+ // Call ItemSelected first to trigger navigation before dismissing the search UI.
-+ // On iOS 26+, setting Active = false before navigation can cause the navigation
-+ // to be lost due to the search controller dismissal animation.
- (SearchHandler as ISearchHandlerController)?.ItemSelected(e);
-- _searchController.Active = false;
-+
-+ // Deactivate the search controller after navigation has been initiated.
-+ // Using BeginInvokeOnMainThread ensures this happens after the current run loop,
-+ // allowing the navigation to proceed without interference from the dismissal animation.
-+ ViewController?.BeginInvokeOnMainThread(() =>
-+ {
-+ if (searchController is not null)
-+ {
-+ searchController.Active = false;
-+ }
-+ });
- }
-```
-
-
-
-
-π¬ Discussion Summary
-
-**Key Comments from Issue #33356:**
-- TamilarasanSF4853 (Syncfusion): Validated issue across multiple MAUI versions (9.0.0 through 10.0.20)
-- Issue 2 (empty page on back) specifically regressed in 9.0.90
-- Issue 1 (no navigation on search suggestion tap) affects all tested versions on iOS
-
-**PR #33406 Review Comments:**
-- Copilot PR reviewer caught typo: "searchHander" should be "searchHandler" (5 duplicate comments, all resolved/outdated now)
-- Prior agent review by kubaflo marked it as β
APPROVE with comprehensive analysis
-- PureWeen requested `/rebase` (latest comment)
-
-**PR #33396 Review Comments:**
-- PureWeen asked to update state file to match PR number
-- Copilot had firewall issues accessing GitHub API
-
-**Disagreements to Investigate:**
-| File:Line | Reviewer Says | Author Says | Status |
-|-----------|---------------|-------------|--------|
-| N/A | N/A | N/A | No active disagreements |
-
-**Author Uncertainty:**
-- None noted in either PR
-
-
-
-
-βοΈ Comparison: PR #33406 vs PR #33396
-
-### Fix Approach Comparison
-
-| Aspect | PR #33406 (Community) | PR #33396 (Copilot) |
-|--------|----------------------|---------------------|
-| **Author** | SubhikshaSf4851 (Syncfusion) | Copilot |
-| **Status** | Open | Closed (draft) |
-| **Lines Changed** | 2 (swap order) | 17 (more defensive) |
-| **Fix Strategy** | Simply swap order of operations | Swap order + dispatch to next run loop |
-| **Test Style** | Code-only (no XAML) | XAML + code-behind |
-| **Test Count** | 1 test method | 2 test methods |
-
-### Which Fix is Better?
-
-**PR #33406 (simpler approach):**
-- β
Minimal change - just swaps two lines
-- β
Addresses root cause: ItemSelected called while navigation context is valid
-- β οΈ Dismissal happens synchronously after ItemSelected
-- β οΈ Could theoretically still interfere if dismissal animation is fast
-
-**PR #33396 (defensive approach):**
-- β
Uses BeginInvokeOnMainThread for explicit async deactivation
-- β
Stores reference to search controller before state changes
-- β
More detailed comments explaining the fix
-- β οΈ More code complexity
-- β οΈ Was closed/abandoned
-
-### Recommendation
-
-Both approaches should work. PR #33406 is simpler and has been reviewed/approved. The extra defensive measures in PR #33396 (BeginInvokeOnMainThread) may provide additional safety margin but add complexity.
-
-**Prior agent review on PR #33406** already verified:
-- Tests FAIL without fix (bug reproduced - timeout)
-- Tests PASS with fix (navigation successful)
-
-
-
-
-π§ͺ Tests
-
-**Status**: β³ PENDING (need to verify tests compile and reproduce issue)
-
-**PR #33406 Tests:**
-- HostApp: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.cs` (code-only, no XAML)
-- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs`
-- 1 test: `Issue33356NavigateShouldOccur` - Tests search handler navigation AND back navigation + collection view navigation
-
-**PR #33396 Tests (for reference):**
-- HostApp XAML: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml`
-- HostApp Code: `src/Controls/tests/TestCases.HostApp/Issues/Issue33356.xaml.cs`
-- NUnit: `src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33356.cs`
-- 2 tests: `SearchSuggestionTapNavigatesToDetailPage`, `BackNavigationFromDetailPageWorks`
-
-**Test Checklist:**
-- [ ] PR includes UI tests
-- [ ] Tests reproduce the issue
-- [ ] Tests follow naming convention (`Issue33356`)
-
-
-
-
-π¦ Gate - Test Verification
-
-**Status**: β³ PENDING
-
-- [ ] Tests FAIL without fix (bug reproduced)
-- [ ] Tests PASS with fix (fix validated)
-
-**Prior Agent Review Result (kubaflo on PR #33406):**
-```
-WITHOUT FIX: FAILED - System.TimeoutException: Timed out waiting for element "Issue33356CatNameLabel"
-WITH FIX: PASSED - All 1 tests passed in 21.73 seconds
-```
-
-**Result:** [PENDING - needs re-verification]
-
-
-
-
-π§ Fix Candidates
-
-**Status**: β³ PENDING
-
-| # | Source | Approach | Test Result | Files Changed | Notes |
-|---|--------|----------|-------------|---------------|-------|
-| PR | PR #33406 | Swap order: ItemSelected before Active=false | β³ PENDING (Gate) | `ShellPageRendererTracker.cs` (2 lines) | Current PR - simpler fix |
-| Alt | PR #33396 | Swap order + BeginInvokeOnMainThread | β
VERIFIED (prior test) | `ShellPageRendererTracker.cs` (17 lines) | Prior attempt - more defensive |
-
-**Exhausted:** No
-**Selected Fix:** [PENDING]
-
-
-
----
-
-**Next Step:** Verify PR #33406 tests compile and Gate passes. Read `.github/agents/pr/post-gate.md` after Gate passes.
From 0f1a423a7189ed9584b078a4f98c0f2188236358 Mon Sep 17 00:00:00 2001
From: Shane Neuville
Date: Thu, 8 Jan 2026 18:55:25 -0600
Subject: [PATCH 5/5] Clarify #if directive rule applies to test method bodies
only
---
.github/instructions/uitests.instructions.md | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/.github/instructions/uitests.instructions.md b/.github/instructions/uitests.instructions.md
index c0778ec7bb33..247792eecc55 100644
--- a/.github/instructions/uitests.instructions.md
+++ b/.github/instructions/uitests.instructions.md
@@ -171,9 +171,9 @@ Tests should run on all applicable platforms by default. The test infrastructure
### No Inline #if Directives in Test Methods
-**Do NOT use `#if ANDROID`, `#if IOS`, etc. directly in test methods.** Platform-specific behavior must be hidden behind extension methods for readability.
+**Do NOT use `#if ANDROID`, `#if IOS`, etc. inside test method bodies.** Platform-specific behavior must be hidden behind extension methods for readability.
-**Note:** This rule is about **code cleanliness**, not platform scope. Using `#if ANDROID ... #else ...` still compiles for all platforms - the issue is that inline directives make test logic hard to read and maintain.
+**Note:** File-level `#if` (to exclude entire files) or wrapping entire test methods is acceptable. This rule targets inline conditionals within test logic that make code hard to read and maintain.
```csharp
// β BAD - inline #if in test method (hard to read)