[Android] Fixed duplicate title icon when setting TitleIconImageSource Multiple times

.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 |
+
+---
+
+<details>
+<summary><strong>📋 Issue Summary</strong></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
+
+</details>
+
+<details>
+<summary><strong>📁 Files Changed</strong></summary>
+
+| 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 |
+
+</details>
+
+<details>
+<summary><strong>💬 PR Discussion Summary</strong></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
+
+</details>
+
+<details>
+<summary><strong>🧪 Tests</strong></summary>
+
+**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
+
+</details>
+
+<details>
+<summary><strong>🚦 Gate - Test Verification</strong></summary>
+
+**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)
+
+</details>
+
+<details>
+<summary><strong>🔧 Fix Candidates</strong></summary>
+
+**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
+
+</details>
+
+---
+
+**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
+
+<details>
+<summary><strong>📜 Git History Analysis</strong></summary>
+
+**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.
+
+</details>
+
+<details>
+<summary><strong>🔄 Platform Comparison</strong></summary>
+
+| 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.
+
+</details>
+
+<details>
+<summary><strong>⚠️ Risk Assessment</strong></summary>
+
+**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
+
+</details>
+
+### Recommendation
+
+**✅ APPROVE** - The PR's approach is correct and validated by independent analysis. The fix is minimal, focused, and addresses the root cause.

src/Controls/src/Core/Platform/Android/Extensions/ToolbarExtensions.cs

@@ -56,17 +56,38 @@ public static void UpdateTitleIcon(this AToolbar nativeToolbar, Toolbar toolbar)
 
 			ImageSource source = toolbar.TitleIcon;
 
-			if (source == null || source.IsEmpty)
+			ToolbarTitleIconImageView? iconView = null;
+			for (int childIndex = 0; childIndex < nativeToolbar.ChildCount; childIndex++)
 			{
-				if (nativeToolbar.GetChildAt(0) is ToolbarTitleIconImageView existingImageView)
-					nativeToolbar.RemoveView(existingImageView);
+				var child = nativeToolbar.GetChildAt(childIndex);
+				if (child is ToolbarTitleIconImageView icon)
+				{
+					if (iconView is null)
+					{
+						iconView = icon; // Keep the first one found
+					}
+					else
+					{
+						nativeToolbar.RemoveView(icon); // Remove any extras (self-healing)
+					}
+				}
+			}
 
+			if (source is null || source.IsEmpty)
+			{
+				if (iconView is not null)
+				{
+					nativeToolbar.RemoveView(iconView);
+				}
 				return;
 			}
 
-			var iconView = new ToolbarTitleIconImageView(nativeToolbar.Context);
-			nativeToolbar.AddView(iconView, 0);
-			iconView.SetImageResource(global::Android.Resource.Color.Transparent);
+			if (iconView is null)
+			{
+				iconView = new ToolbarTitleIconImageView(nativeToolbar.Context);
+				nativeToolbar.AddView(iconView, 0);
+				iconView.SetImageResource(global::Android.Resource.Color.Transparent);
+			}
 
 			source.LoadImage(toolbar.Handler.MauiContext, (result) =>
 			{

src/Controls/tests/TestCases.HostApp/Issues/Issue31445.cs

@@ -0,0 +1,38 @@
+namespace Controls.TestCases.HostApp.Issues;
+
+[Issue(IssueTracker.Github, "31445", "Duplicate Title icon should not appear", PlatformAffected.Android)]
+
+public class Issue31445 : NavigationPage
+{
+	public Issue31445() : base(new Issue31445Page())
+	{
+	}
+}
+
+public class Issue31445Page : ContentPage
+{
+	public Issue31445Page()
+	{
+		NavigationPage.SetTitleIconImageSource(this, "dotnet_bot.png");
+
+		var label = new Label()
+		{
+			Text = "Test passes if only one title icon is set after button click",
+			VerticalOptions = LayoutOptions.Start,
+			AutomationId = "label"
+		};
+
+		var button = new Button()
+		{
+			Text = "Click here",
+			VerticalOptions = LayoutOptions.Start,
+			AutomationId = "Issue31445Button"
+		};
+		button.Clicked += (s, e) => { NavigationPage.SetTitleIconImageSource(this, "dotnet_bot.png"); };
+
+		Content = new StackLayout
+		{
+			Children = { label, button }
+		};
+	}
+}

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31445.cs

@@ -0,0 +1,23 @@
+using NUnit.Framework;
+using UITest.Appium;
+using UITest.Core;
+
+namespace Microsoft.Maui.TestCases.Tests.Issues;
+
+public class Issue31445 : _IssuesUITest
+{
+	public Issue31445(TestDevice testDevice) : base(testDevice)
+	{
+	}
+
+	public override string Issue => "Duplicate Title icon should not appear";
+
+	[Test]
+	[Category(UITestCategories.Navigation)]
+	public void Issue31445DuplicateTitleIconDoesNotAppear()
+	{
+		App.WaitForElement("Issue31445Button");
+		App.Tap("Issue31445Button");
+		VerifyScreenshot();
+	}
+}