[PR-Agent] Fix OnBackButtonPressed not firing for Shell Navigation Bar #33531
Conversation
kubaflo
added
area-navigation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the Shell navigation bar back button (top-left arrow) does not trigger OnBackButtonPressed() on Android, while the system back button and Windows platform work correctly.
Changes:
- Added
SendBackButtonPressed()call inShellToolbarTracker.OnNavigateBack()to allow pages to intercept navigation bar back button clicks - Created UI test to verify
OnBackButtonPressedis called when tapping the Shell navigation bar back button - Test verifies the page can prevent navigation by returning
truefromOnBackButtonPressed()
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs |
Added SendBackButtonPressed() call before PopAsync() in OnNavigateBack() to allow pages to intercept navigation |
src/Controls/tests/TestCases.HostApp/Issues/Issue33523.cs |
Test page with Shell navigation that overrides OnBackButtonPressed() and prevents navigation |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33523.cs |
NUnit test verifying OnBackButtonPressed is called when tapping the navigation bar back button |
.github/agent-pr-session/pr-33523.md |
Documentation of the PR review process and test verification |
|
|
||
| namespace Maui.Controls.Sample.Issues | ||
| { | ||
| [Issue(IssueTracker.Github, 33523, "OnBackButtonPressed not firing for Shell Navigation Bar button in .NET 10 SR2", PlatformAffected.Android)] |
There was a problem hiding this comment.
The issue description references '.NET 10 SR2' which is speculative. Based on the repository structure and global.json, the current version should be verified. Consider updating to match the actual .NET version being targeted or removing the version-specific reference if it applies broadly.
| [Issue(IssueTracker.Github, 33523, "OnBackButtonPressed not firing for Shell Navigation Bar button in .NET 10 SR2", PlatformAffected.Android)] | |
| [Issue(IssueTracker.Github, 33523, "OnBackButtonPressed not firing for Shell Navigation Bar button", PlatformAffected.Android)] |
.github/agent-pr-session/pr-33523.md
Outdated
| { | ||
| MainThread.BeginInvokeOnMainThread(async () => | ||
| { | ||
| await DisplayAlertAsync(string.Empty, "OnBackButtonPressed", "cancel"); |
There was a problem hiding this comment.
The method name 'DisplayAlertAsync' appears to be incorrect. The standard MAUI Page API uses 'DisplayAlert' (synchronous) not 'DisplayAlertAsync'. This code sample may not compile.
| await DisplayAlertAsync(string.Empty, "OnBackButtonPressed", "cancel"); | |
| await DisplayAlert(string.Empty, "OnBackButtonPressed", "cancel"); |
|
/rebase |
github-actions
bot
force-pushed
the
fix/issue-33523
branch
from
25cfa7a to
7b62b30
Compare
kubaflo
force-pushed
the
fix/issue-33523
branch
from
7b62b30 to
989cbd2
Compare
kubaflo
added
the
s/ai-reproduction-confirmed
kubaflo
added
s/ai-reproduction-confirmed
… back button Fixes dotnet#33523 The Shell navigation bar back button was calling PopAsync() directly without checking if the page wants to intercept navigation via OnBackButtonPressed(). This fix adds a call to SendBackButtonPressed() before popping, matching the behavior of the system back button and the Windows platform.
kubaflo
force-pushed
the
fix/issue-33523
branch
from
989cbd2 to
6e11869
Compare
🤖 AI Summary📊 Expand Full Review🔍 Pre-Flight — Context & Validation📝 Review Session — [Android] Fix OnBackButtonPressed not firing for Shell Navigation Bar back button ·
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #33531 | Add SendBackButtonPressed() check in OnNavigateBack() before PopAsync() |
⛔ GATE BLOCKED | ShellToolbarTracker.cs (+4) |
Original PR - inline check inside OnNavigateBack() |
| 1 | try-fix | Check in OnClick() before calling OnNavigateBack() |
✅ PASS | ShellToolbarTracker.cs (+5) |
Handles interception at click event level |
| 2 | try-fix | Create virtual ShouldNavigateBack() method |
✅ PASS | ShellToolbarTracker.cs (+17), PublicAPI.Unshipped.txt (+1) |
More extensible, adds public API surface |
| 3 | try-fix | Delegate to _shell.SendBackButtonPressed() entirely |
✅ PASS | ShellToolbarTracker.cs (+2) |
Minimal change, reuses Shell's existing back button logic |
| 4 | try-fix | Guard clause before try block in OnNavigateBack() |
✅ PASS | ShellToolbarTracker.cs (+4) |
Stylistic variation of PR fix |
| 5 | try-fix | Inverted condition in OnClick() |
✅ PASS | ShellToolbarTracker.cs (+5) |
Stylistic variation of attempt 1 |
Cross-Pollination: Completed by prior agent - all 5 models confirmed no new ideas
Exhausted: Yes (all 5 models confirmed no new ideas)
Selected Fix: Gate verification blocked by environment issues; all 5 try-fix attempts passed successfully
Key Insights from Fix Analysis:
- Approach 3 (delegate to Shell) is the most elegant - minimal change, reuses existing Shell infrastructure
- PR's fix and Approach 4 are equivalent - just stylistic differences (check inside vs before try block)
- Approaches 1 & 5 are equivalent - handle at OnClick level vs OnNavigateBack level
- Approach 2 adds public API surface for extensibility but may be over-engineered for this fix
- All approaches fundamentally do the same thing: check SendBackButtonPressed() before PopAsync()
🔧 Try-Fix Analysis: ✅ 5 passed
✅ Fix 1
Approach: Handle Back Button in OnClick Instead of OnNavigateBack
Add SendBackButtonPressed() check in the OnClick() method (click handler) instead of inside OnNavigateBack().
Different from existing fix:
- Existing fix: Adds the check inside
OnNavigateBack()(line 232-236) - This approach: Adds the check in
OnClick()(line 160-174), before callingOnNavigateBack()
Rationale: This keeps OnNavigateBack() purely for navigation, and handles the page interception at the click event level - more aligned with how system back button events are handled.
diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
index 202234f0c3..d53c742649 100644
--- a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
+++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
@@ -167,7 +167,12 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
if (backButtonHandler?.Command != null)
backButtonHandler.Command.Execute(backButtonHandler.CommandParameter);
else if (CanNavigateBack)
+ {
+ // Call OnBackButtonPressed to allow the page to intercept navigation
+ if (Page?.SendBackButtonPressed() == true)
+ return;
OnNavigateBack();
+ }
else
_shell.FlyoutIsPresented = !_shell.FlyoutIsPresented;
}
Analysis
Result: Pass ✅
What happened: The test OnBackButtonPressedShouldFireForShellNavigationBarButton passed on Android. The test verifies that:
- Navigation to TestPage works
- Tapping the Shell navigation bar back button calls
OnBackButtonPressed() - The page can intercept and prevent navigation by returning
true
Why it worked: Moving the SendBackButtonPressed() check from OnNavigateBack() to OnClick() works equally well because:
- Both locations are on the same code path for the navigation bar back button
- The check happens before
PopAsync()is called in either case - The semantics are preserved - page can intercept navigation
Insights: This approach may be slightly cleaner because:
- It keeps
OnNavigateBack()purely for navigation logic - The interception happens at the event handler level (closer to system back button pattern)
- It's consistent with other back button handling patterns in MAUI
Trade-off: The original fix location (inside OnNavigateBack()) has the advantage of being a single point of change if there are other code paths that call OnNavigateBack(). The OnClick() approach only covers clicks on the navigation button.
✅ Fix 2
Approach: Create Virtual ShouldNavigateBack Method
Create a protected virtual ShouldNavigateBack() method that checks SendBackButtonPressed() and can be overridden by derived classes.
Different from existing approaches:
- Original PR fix: Inline check in
OnNavigateBack() - Attempt 1: Inline check in
OnClick() - This approach: Extract to a separate virtual method for better extensibility
Rationale: This creates a cleaner separation of concerns and allows custom Shell implementations to override the back navigation check behavior.
diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
index 202234f0c3..adbb19ba6c 100644
--- a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
+++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
@@ -229,10 +229,24 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
return new ShellSearchView(context, ShellContext);
}
+ /// <summary>
+ /// Checks if back navigation should proceed by calling the page's OnBackButtonPressed.
+ /// Returns true if navigation should proceed, false if the page intercepted it.
+ /// </summary>
+ protected virtual bool ShouldNavigateBack()
+ {
+ // Call OnBackButtonPressed to allow the page to intercept navigation
+ // Returns false if the page handled the event (wants to prevent navigation)
+ return Page?.SendBackButtonPressed() != true;
+ }
+
protected async virtual void OnNavigateBack()
{
try
{
+ if (!ShouldNavigateBack())
+ return;
+
await Page.Navigation.PopAsync();
}
catch (Exception exc)
diff --git a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt
index 7dc5c58110..44fb177fca 100644
--- a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt
+++ b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt
@@ -1 +1,2 @@
#nullable enable
+virtual Microsoft.Maui.Controls.Platform.Compatibility.ShellToolbarTracker.ShouldNavigateBack() -> bool
Analysis
Result: Pass ✅
What happened: The test passed with the virtual ShouldNavigateBack() method approach.
Why it worked: The virtual method approach:
- Extracts the back button interception logic into a separate, overridable method
- Maintains the same semantics as the original fix
- Allows custom Shell implementations to override the behavior if needed
Trade-offs:
- Pro: More extensible - custom Shell handlers can override
ShouldNavigateBack() - Pro: Better separation of concerns - navigation check is isolated
- Con: Requires PublicAPI.Unshipped.txt entry (new public API surface)
- Con: More complex than a simple inline check
Recommendation: The original PR fix (inline in OnNavigateBack()) is simpler and doesn't add public API surface. This approach is better suited if there's a future need for custom Shell handlers to override the back navigation behavior.
✅ Fix 3
Approach: Delegate to Shell.OnBackButtonPressed Entirely
Instead of adding a SendBackButtonPressed() check inside OnNavigateBack(), replace the entire navigation logic in OnClick() by calling _shell.SendBackButtonPressed() when CanNavigateBack is true.
Different from existing approaches:
- Original PR fix: Adds check inside
OnNavigateBack()then callsPopAsync()if not handled - Attempt 1: Adds check in
OnClick()then callsOnNavigateBack()if not handled - Attempt 2: Creates virtual method
ShouldNavigateBack()called fromOnNavigateBack() - This approach: Uses Shell's existing
OnBackButtonPressed()which already handles page interception AND navigation pop in a unified way
Rationale: Shell.OnBackButtonPressed() already contains unified back button handling logic:
- Checks BackButtonBehavior commands
- Calls Page.SendBackButtonPressed()
- Calls Navigation.PopAsync() if needed
By delegating to Shell, we get consistent behavior with the system back button without duplicating logic.
diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
index 202234f0c3..19063188b3 100644
--- a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
+++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
@@ -167,7 +167,8 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
if (backButtonHandler?.Command != null)
backButtonHandler.Command.Execute(backButtonHandler.CommandParameter);
else if (CanNavigateBack)
- OnNavigateBack();
+ // Delegate to Shell's OnBackButtonPressed which handles page interception and navigation
+ _shell.SendBackButtonPressed();
else
_shell.FlyoutIsPresented = !_shell.FlyoutIsPresented;
}
Analysis
Result: Pass ✅
What happened: The test OnBackButtonPressedShouldFireForShellNavigationBarButton passed on Android. The test verifies:
- Navigation to TestPage works
- Tapping the Shell navigation bar back button calls
OnBackButtonPressed() - The page can intercept and prevent navigation by returning
true
Why it worked: Instead of duplicating the back button handling logic, this approach delegates to Shell's existing OnBackButtonPressed() via SendBackButtonPressed(). Shell already has unified handling that:
- Checks BackButtonBehavior commands (handled by
OnClickbefore our change anyway) - Calls
Page.SendBackButtonPressed()to let pages intercept - Calls
Navigation.PopAsync()if not handled
Key advantages over other approaches:
- Minimal change: Only 2 lines changed (1 line of code + 1 comment)
- No code duplication: Reuses Shell's existing back button logic rather than adding parallel logic in ShellToolbarTracker
- Unified behavior: Navigation bar back button now takes the EXACT same code path as the system back button
- No public API surface: No new methods or properties exposed
Trade-offs:
- Con:
OnNavigateBack()method becomes unused (only for custom overrides in subclasses) - Con: Slightly less obvious that page interception is happening (need to follow the code path to Shell)
Comparison to other approaches:
- Original PR: Adds check in
OnNavigateBack()(+4 lines) - works, but duplicates Shell logic - Attempt 1: Adds check in
OnClick()- works, but also duplicates - Attempt 2: Virtual
ShouldNavigateBack()- works, but adds public API surface - This approach: Reuses Shell's
OnBackButtonPressed()- smallest change, unified behavior
Recommendation: This is a cleaner solution that reuses existing Shell infrastructure. However, all 4 approaches work - the choice depends on maintainability preferences.
✅ Fix 4
Approach: Check Page Interception Before Calling OnNavigateBack
Add the SendBackButtonPressed() check as a guard clause at the beginning of OnNavigateBack() method with early return - same location as the original PR but with a cleaner guard clause pattern.
Different from existing approaches:
- Original PR fix: Adds check with if-block and return inside try block
- Attempt 1: Adds check in
OnClick()before callingOnNavigateBack() - Attempt 2: Creates virtual
ShouldNavigateBack()method - Attempt 3: Delegates entirely to
_shell.SendBackButtonPressed() - This approach: Uses guard clause pattern at method start (before try block) for cleaner control flow
Rationale: Guard clauses at method start are a common pattern for early returns. By checking SendBackButtonPressed() before entering the try block, we get cleaner code structure and avoid nesting.
diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
index 202234f0c3..f8df379f29 100644
--- a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
+++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
@@ -231,6 +231,10 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
protected async virtual void OnNavigateBack()
{
+ // Guard clause: Let page intercept back navigation first
+ if (Page?.SendBackButtonPressed() == true)
+ return;
+
try
{
await Page.Navigation.PopAsync();
Analysis
Result: Pass ✅
What happened: The test passed with the guard clause placed before the try block.
Why it worked: The guard clause pattern achieves the same result as the original PR but with slightly different code structure:
- Original PR: Check inside try block
- This approach: Check before try block as a guard clause
Comparison:
This is essentially the same fix as the original PR, just with the check moved outside the try block. Both approaches work identically - the only difference is code style preference.
Trade-offs:
- Pro: Guard clauses at method start are a common clean code pattern
- Pro: Avoids nesting the early return inside try block
- Con: If
SendBackButtonPressed()itself can throw (unlikely), it wouldn't be caught - Neutral: Functionally equivalent to original PR
Verdict: This is a stylistic variation of the original PR fix. Both work equally well.
✅ Fix 5
Approach: Inverse Condition Check in OnClick with Explicit Block
Add the check in OnClick() with an inverted condition and explicit braces around the if-else block for the CanNavigateBack case.
Different from existing approaches:
- Original PR: Check in
OnNavigateBack()inside try block - Attempt 1: Check in
OnClick()withif (handled) return; else OnNavigateBack() - Attempt 2: Virtual
ShouldNavigateBack()method - Attempt 3: Delegate to
_shell.SendBackButtonPressed()entirely - Attempt 4: Guard clause before try block
- This approach: Check in
OnClick()with inverted logic:if (NOT handled) OnNavigateBack()
Rationale: Same location as Attempt 1, but uses different conditional pattern. Instead of "if handled, return early", uses "if NOT handled, proceed with navigation". This is a stylistic variation.
diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
index 202234f0c3..6eb79e41ec 100644
--- a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
+++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs
@@ -167,7 +167,11 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility
if (backButtonHandler?.Command != null)
backButtonHandler.Command.Execute(backButtonHandler.CommandParameter);
else if (CanNavigateBack)
- OnNavigateBack();
+ {
+ // Allow page to intercept back navigation; only navigate if not handled
+ if (Page?.SendBackButtonPressed() != true)
+ OnNavigateBack();
+ }
else
_shell.FlyoutIsPresented = !_shell.FlyoutIsPresented;
}
Analysis
Result: Pass ✅
What happened: The test passed with the inverted condition check in OnClick().
Why it worked: Same mechanism as Attempt 1, but with inverted logic:
- Attempt 1:
if (Page?.SendBackButtonPressed() == true) return;thenOnNavigateBack(); - This approach:
if (Page?.SendBackButtonPressed() != true) OnNavigateBack();
Both are logically equivalent - just different coding styles.
Comparison:
| Approach | Pattern |
|---|---|
| Attempt 1 | Early return: if handled, return |
| This | Conditional: if NOT handled, proceed |
Trade-offs:
- Pro: More explicit block structure with braces
- Con: Slightly more nested than early return pattern
- Neutral: Functionally identical to Attempt 1
Verdict: Stylistic variation of Attempt 1. Both work equally well.
2 participants
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
This PR fixes an issue where the Shell navigation bar back button (the arrow in the top-left corner) does not trigger
OnBackButtonPressed()on Android, while it works correctly on Windows.Root cause: The Shell navigation bar back button click handler (
ShellToolbarTracker.OnClick) was callingPage.Navigation.PopAsync()directly without checking if the page wants to intercept the navigation viaOnBackButtonPressed(). This differs from the system back button behavior, which properly invokesSendBackButtonPressed()through the Android lifecycle event system.Fix: Added a call to
Page?.SendBackButtonPressed()in theOnNavigateBack()method before callingPopAsync(). If the method returnstrue(meaning the page handled the event and wants to prevent navigation), the method returns early and skips thePopAsync()call.Key insight: The navigation bar back button and system back button take completely different code paths in Shell:
Activity.OnBackPressed→AndroidLifecycle.OnBackPressed→Shellhandler →page.SendBackButtonPressed()→OnBackButtonPressed()ShellToolbarTracker.OnClick→OnNavigateBack()→PopAsync()(directly, bypassing the check)The fix unifies these paths by ensuring both buttons check
OnBackButtonPressed()before navigating back.What to avoid: Don't bypass
SendBackButtonPressed()when programmatically popping pages in response to user navigation actions. This method is the public API contract for allowing pages to intercept back navigation.Issues Fixed
Fixes #33523
Verified behavior:
OnBackButtonPressed()truefalse(default behavior)Testing
Added UI tests that verify:
truefrom OnBackButtonPressed prevents navigationTest files:
src/Controls/tests/TestCases.HostApp/Issues/Issue33523.cs- Test page with Shell navigationsrc/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33523.cs- NUnit test verifying OnBackButtonPressed is calledTest verification: