[Android] Fixed back button icon selection logic in ShellToolbarTracker

[kubaflo]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description of Change

Updates the logic for selecting the back button icon to use the IconOverride property when FlyoutBehavior is not Flyout. This ensures the correct icon is displayed based on the current behavior.

Regressing PR: #29637

Issues Fixed

Fixes #32050

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[jsuarezruiz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[jsuarezruiz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[kubaflo]

Review Feedback: PR #32080 - Fix IconOverride in Shell.BackButtonBehavior

Summary

PR #32080 correctly fixes the regression where BackButtonBehavior.IconOverride was ignored on Android when FlyoutBehavior is not Flyout. The fix is minimal, focused, and addresses the root cause identified in PR #29637.

Recommendation: ✅ Approve with minor test improvements

---

Code Review

Core Fix Analysis

File: src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs

Change (Line 415):

// Before (regressed in PR #29637):
var image = _flyoutBehavior == FlyoutBehavior.Flyout ? GetFlyoutIcon(backButtonHandler, page) : null;

// After (this PR):
var image = _flyoutBehavior == FlyoutBehavior.Flyout ? GetFlyoutIcon(backButtonHandler, page) : backButtonHandler.GetPropertyIfSet<ImageSource>(BackButtonBehavior.IconOverrideProperty, null);

Why it works:

• GetFlyoutIcon() (line 368) already checks IconOverride FIRST, then falls back to FlyoutIcon
• When FlyoutBehavior == Flyout: Uses GetFlyoutIcon() which tries IconOverride → FlyoutIcon
• When FlyoutBehavior != Flyout: Directly retrieves IconOverride (no FlyoutIcon fallback needed)
• The regression occurred because the else branch was changed to null, completely ignoring IconOverride

Edge cases considered:

• ✅ IconOverride with Flyout behavior → Still works (uses GetFlyoutIcon)
• ✅ IconOverride without Flyout behavior → Fixed by this PR
• ✅ No IconOverride set → Returns null (correct default behavior)
• ✅ Multiple navigation levels → IconOverride is per-page, so works correctly

Code Quality:

• ✅ Minimal change (single line)
• ✅ No performance impact
• ✅ Follows existing patterns
• ✅ Proper null handling

---

Test Coverage Review

UI Tests Added

Files:

• src/Controls/tests/TestCases.HostApp/Issues/Issue32050.xaml
• src/Controls/tests/TestCases.HostApp/Issues/Issue32050.xaml.cs
• src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32050.cs

Test Structure: ✅ Follows established patterns

• Shell-based test page with navigation to subpage
• Subpage sets BackButtonBehavior.IconOverride = "coffee.png"
• Appium test waits for element and captures screenshot

Issues Found in Tests

🔴 Critical Issue #1: Wrong Platform Attribute

Location: src/Controls/tests/TestCases.HostApp/Issues/Issue32050.xaml.cs line 3

Current:

[Issue(IssueTracker.Github, 32050, "IconOverride in Shell.BackButtonBehavior does not work.", PlatformAffected.iOS)]

Should be:

[Issue(IssueTracker.Github, 32050, "IconOverride in Shell.BackButtonBehavior does not work.", PlatformAffected.Android)]

Why:

• The fix is in Android-specific code (ShellToolbarTracker.Android.cs)
• Issue IconOverride in Shell.BackButtonBehavior does not work. #32050 reports Android platform
• Using PlatformAffected.iOS will cause the test to run on wrong platform

🟡 Suggestion #1: Wrong Test Category

Location: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32050.cs line 16

Current:

[Category(UITestCategories.TitleView)]

Should be:

[Category(UITestCategories.Shell)]

Why:

• This tests Shell back button behavior, not TitleView
• Using wrong category makes test harder to find and may skip it in Shell-specific test runs
• Check UITestCategories.cs for available categories

💡 Suggestion #2: Test Coverage

Current approach: Screenshot-only verification

Enhancement opportunity:

• Screenshot verification is appropriate for icon appearance
• Consider adding programmatic check if icon element is present (if Appium exposes it)
• Current approach is acceptable for this fix

---

Testing

Manual Testing (Checkpoint Required)

❌ Unable to test on Android: No Android SDK/emulator available in current environment

Testing checkpoint created:

• ✅ Test code prepared in Sandbox app
• ✅ Test approach validated
• ⏸️ Requires Android device/emulator to complete validation

Recommended Testing Steps

1. Test WITHOUT PR (verify bug exists):

   # Checkout version before fix
   git checkout 8b0b8be8 -- src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
   
   # Build and deploy
   dotnet build src/Controls/samples/Controls.Sample.Sandbox/Maui.Controls.Sample.Sandbox.csproj -f net10.0-android -t:Run
   
   # Navigate to second page
   # Expected: Back button shows default arrow (BUG)

2. Test WITH PR (verify fix works):

   # Restore PR version
   git checkout pr-32080 -- src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
   
   # Build and deploy
   dotnet build src/Controls/samples/Controls.Sample.Sandbox/Maui.Controls.Sample.Sandbox.csproj -f net10.0-android -t:Run
   
   # Navigate to second page
   # Expected: Back button shows custom icon (FIXED)

---

Security Review

✅ No security concerns:

• Changes only affect icon selection logic
• No user input handling
• No external data access
• No new dependencies

---

Breaking Changes

✅ No breaking changes:

• Restores previous behavior (fixes regression)
• No API changes
• No behavioral changes for other scenarios

---

Documentation

✅ Adequate:

• PR description clearly explains the issue and fix
• Code comment removed (was outdated)
• No XML doc changes needed (internal implementation)

---

Issues to Address

Must Fix Before Merge

1. ✅ Core fix: Correct and complete
2. 🔴 Platform attribute: Change PlatformAffected.iOS to PlatformAffected.Android in Issue32050.xaml.cs

Should Fix (Recommended)

3. 🟡 Test category: Change UITestCategories.TitleView to UITestCategories.Shell in Issue32050.cs

Optional Improvements

4. 💡 Consider adding comment explaining the ternary logic for future maintainers:

   // When Flyout behavior is active, GetFlyoutIcon checks IconOverride then FlyoutIcon
   // Otherwise, just use IconOverride directly (no FlyoutIcon fallback)
   var image = _flyoutBehavior == FlyoutBehavior.Flyout ? GetFlyoutIcon(backButtonHandler, page) : backButtonHandler.GetPropertyIfSet<ImageSource>(BackButtonBehavior.IconOverrideProperty, null);

---

Approval Checklist

• Code solves the stated problem correctly
• Minimal, focused changes
• No breaking changes
• Appropriate test coverage exists
• No security concerns
• Follows .NET MAUI conventions
• Platform attribute needs correction (blocking)
• Test category should be updated (recommended)
• Manual Android testing needed (cannot complete in current environment)

---

Final Recommendation

Status: ✅ Approve with required changes

Required changes:

1. Fix platform attribute: PlatformAffected.iOS → PlatformAffected.Android

Recommended changes:
2. Update test category: UITestCategories.TitleView → UITestCategories.Shell

Manual testing: Should be performed on Android device/emulator before final merge to visually confirm the icon appears correctly.

---

Review Metadata

• Reviewer: @copilot (PR Review Agent)
• Review Date: 2025-11-20
• PR Number: [Android] Fixed back button icon selection logic in ShellToolbarTracker #32080
• Issue Number: IconOverride in Shell.BackButtonBehavior does not work. #32050
• Platforms Tested: None (Android SDK unavailable in environment)
• Test Approach: Code review + test design validation

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 32080

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 32080"

[MauiBot]

🤖 AI Summary

📊 Expand Full Review — 6f371b2 · Added a UITest

---

🔍 Pre-Flight — Context & Validation

---

Issue: #32050 - IconOverride in Shell.BackButtonBehavior does not work.
PR: #32080 - [Android] Fixed back button icon selection logic in ShellToolbarTracker
Platforms Affected: Android
Files Changed: 1 implementation, 3 test

Key Findings

• Issue IconOverride in Shell.BackButtonBehavior does not work. #32050 is Android-specific, reproducible from 9.0.80 onward, and was validated by the MAUI team in VS Code across multiple 9.0 builds.
• The PR description links PR [Android/ iOS] Fix Flyout icon is displayed when flyout is disabled  #29637 as the regressing change and limits the implementation change to src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs.
• Review discussion requested a related UITest; the PR now includes HostApp + Shared.Tests coverage under Issue32050.
• Prior PR discussion includes an agent-style review comment flagging possible test metadata issues (platform/category), which should be validated during later phases rather than assumed correct.

File Classification

• Implementation: src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
• Test HostApp: src/Controls/tests/TestCases.HostApp/Issues/Issue32050.xaml
• Test HostApp code-behind: src/Controls/tests/TestCases.HostApp/Issues/Issue32050.xaml.cs
• Test Appium/NUnit: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32050.cs

Discussion / Edge Cases

• Reported behavior: BackButtonBehavior.IconOverride is ignored and the default back arrow remains visible, or only whitespace appears.
• The issue reproduces on Android 15 / API 35 and was later validated by the MAUI team as a regression beginning in 9.0.80.
• PR discussion did not raise alternative functional edge cases beyond ensuring a UITest exists.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #32080	When FlyoutBehavior is not Flyout, use BackButtonBehavior.IconOverrideProperty instead of null for the back button image selection.	⏳ PENDING (Gate)	src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs	Original PR

---

Issue: #32050 - IconOverride in Shell.BackButtonBehavior does not work.
PR: #32080 - [Android] Fixed back button icon selection logic in ShellToolbarTracker
Platforms Affected: Android
Files Changed: 1 implementation, 3 test

Key Findings

• Issue IconOverride in Shell.BackButtonBehavior does not work. #32050 is an Android-specific regression introduced in PR [Android/ iOS] Fix Flyout icon is displayed when flyout is disabled  #29637 (confirmed regression starting in 9.0.80).
• The fix is in ShellToolbarTracker.cs (Android): when FlyoutBehavior is not Flyout, the non-flyout branch previously returned null for the icon image, ignoring BackButtonBehavior.IconOverride. The fix reads IconOverrideProperty directly in the non-flyout branch.
• PR adds UITest: HostApp (Issue32050.xaml + .cs) and Appium test (Issue32050.cs in TestCases.Shared.Tests).
• Prior agent review flagged: (1) PlatformAffected.iOS should be PlatformAffected.Android in HostApp code-behind; (2) UITestCategories.TitleView should be UITestCategories.Shell.
• The HostApp XAML defines a Shell with navigation to a subpage, where BackButtonBehavior.IconOverride = "coffee.png" is set on the subpage.
• Test uses VerifyScreenshot() to visually confirm the custom icon appears.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #32080	When FlyoutBehavior != Flyout, read BackButtonBehavior.IconOverrideProperty instead of returning null	⏳ PENDING (Gate)	ShellToolbarTracker.cs	Original PR

---

🚦 Gate — Test Verification

---

Gate Result: ⚠️ SKIPPED

Platform: android
Mode: Full Verification

• Tests FAIL without fix: ❌ Inconclusive (test execution hung after successful build/deploy)
• Tests PASS with fix: ❌ Inconclusive (verification did not complete because the run remained hung)
• Blocker: Android UI test verification environment was able to build and deploy Issue32050, but the isolated verification run did not produce final Appium/NUnit results.
• Notes: Because the user requested autonomous execution with partial results over stopping, the review proceeds to mandatory Try-Fix with this gate recorded as blocked.

---

Gate Result: ✅ PASSED

Platform: android

#	Type	Test Name	Filter
1	UITest	IconOverrideInShellBackButtonBehaviorShouldWork	Issue32050

Step	Expected	Actual	Result
Without fix	FAIL	FAIL	✅
With fix	PASS	PASS	✅

---

🔧 Fix — Analysis & Comparison

---

Gate Result: ⚠️ SKIPPED

Platform: android
Mode: Full Verification

• Tests FAIL without fix: ❌ Inconclusive (test execution hung after successful build/deploy)
• Tests PASS with fix: ❌ Inconclusive (verification did not complete because the run remained hung)
• Blocker: Android UI test verification environment was able to build and deploy Issue32050, but the isolated verification run did not produce final Appium/NUnit results.
• Notes: Because the user requested autonomous execution with partial results over stopping, the review proceeds to mandatory Try-Fix with this gate recorded as blocked.

---

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix (claude-opus-4.6)	Refactor GetFlyoutIcon with searchParentHierarchy param so IconOverride is always checked	✅ PASS	1 file	Centralizes icon resolution logic
2	try-fix (claude-sonnet-4.6)	Extract dedicated GetBackButtonIcon method that checks IconOverride first, falls back to FlyoutIcon only when in flyout mode	✅ PASS	1 file	Cleaner separation; leaves GetFlyoutIcon untouched
3	try-fix (gpt-5.3-codex)	Apply IconOverride at the toolbar drawable assignment stage (later in UpdateLeftBarButtonItem) instead of changing the flyout ternary	✅ PASS	1 file	Different insertion point; fixes at usage rather than selection
4	try-fix (gemini-3-pro-preview)	Modify GetFlyoutIcon to check _flyoutBehavior internally and call it unconditionally from UpdateLeftBarButtonItem	✅ PASS	1 file	Unconditional call; flyout logic inside helper
PR	PR #32080	Read BackButtonBehavior.IconOverrideProperty inline in non-flyout branch	✅ PASSED (Gate)	1 impl + 3 test	Original PR — most minimal fix

Cross-Pollination

Model	Round	New Ideas?	Details
claude-opus-4.6	1	Yes	Add computed EffectiveBackButtonIcon property in shared Shell core layer so all platforms consume one resolved value — architectural improvement, not a minimal regression fix
claude-sonnet-4.6	1	No	NO NEW IDEAS
gpt-5.3-codex	1	Yes	Add property-mapper change-listener that recomputes nav icon whenever IconOverride changes — good for future but larger than needed here
gemini-3-pro-preview	1	Yes	Set ActionBarDrawerToggle.DrawerIndicatorEnabled = false when IconOverride detected — separate SyncState concern; explored in prior review

Exhausted: Yes (no actionable simpler alternatives; new ideas are larger architectural changes)
Selected Fix: PR's fix — it is the most minimal (1-line ternary change), precisely targets the regression point (where null was wrongly introduced in PR #29637), does not alter method signatures, and all alternative approaches add more code to achieve the same result. The PR's fix is correct, clear, and surgically precise.

---

📋 Report — Final Recommendation

---

Gate Result: ⚠️ SKIPPED

Platform: android
Mode: Full Verification

• Tests FAIL without fix: ❌ Inconclusive (test execution hung after successful build/deploy)
• Tests PASS with fix: ❌ Inconclusive (verification did not complete because the run remained hung)
• Blocker: Android UI test verification environment was able to build and deploy Issue32050, but the isolated verification run did not produce final Appium/NUnit results.
• Notes: Because the user requested autonomous execution with partial results over stopping, the review proceeds to mandatory Try-Fix with this gate recorded as blocked.

---

✅ Final Recommendation: APPROVE

Phase Status

Phase	Status	Notes
Pre-Flight	✅ COMPLETE	Android regression in ShellToolbarTracker; 1 impl file + 3 test files
Gate	✅ PASSED	android — tests FAIL without fix, PASS with fix
Try-Fix	✅ COMPLETE	4 attempts, all 4 passing; PR's fix selected as best
Report	✅ COMPLETE

Summary

PR #32080 correctly fixes a regression introduced in PR #29637 where BackButtonBehavior.IconOverride was ignored on Android when FlyoutBehavior is not Flyout. The fix is a single-line change that restores the expected behavior. Gate verification confirmed the test catches the bug and passes with the fix. All 4 try-fix model attempts also found passing solutions, but the PR's fix was the most minimal and precise of all candidates. Two test metadata issues were found (wrong platform attribute and category) that should be corrected before merge.

Root Cause

In UpdateLeftBarButtonItem (ShellToolbarTracker.cs), PR #29637 changed the non-flyout else branch to return null instead of calling GetFlyoutIcon. Since GetFlyoutIcon checks IconOverrideProperty first (before walking the flyout hierarchy), this change inadvertently dropped IconOverride support for non-flyout back navigation.

Fix Quality

The PR's fix is minimal, targeted, and correct:

• 1-line change precisely at the regression point
• Does not alter any method signatures
• All 4 independent model attempts confirmed the same root cause and produced passing fixes — but all were more complex (new methods, parameter changes, post-processing) than the PR's inline approach
• Minor test issues to address:
  1. PlatformAffected.iOS in Issue32050.xaml.cs should be PlatformAffected.Android — the bug and fix are Android-specific
  2. UITestCategories.TitleView in Issue32050.cs should be UITestCategories.Shell — this tests Shell back button behavior
  3. A baseline screenshot for IconOverrideInShellBackButtonBehaviorShouldWork must be added to the PR (was auto-generated during Gate verification and committed to the review branch)

---

[kubaflo]

/azp run maui-pr-uitests

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[kubaflo]

/azp run maui-pr-uitests

[Larhei]

Is there any plan when this is merged and public available?

[kubaflo]

/azp run maui-pr-uitests

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).