Commit d247876
[Issue-Resolver] Explicit fallback for BackButtonBehavior lookup (#33204)
Fixes #28570
Fixes #33139
(I poked around a bit and I think there is indeed an issue in MAUI here.
When a control is added to the visual tree, MAUI checks the parent’s
BackButtonBehavior and propagates it to the child. The problem is that,
at that point, the Command is not yet assigned, so the propagated
behavior ends up in an incomplete state.
Because BackButtonBehavior depends on bindings and command resolution,
its propagation is more complex than that of simple properties. For this
reason, it probably shouldn’t be propagated automatically in the same
way as other properties.)
> [!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](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from
this PR and let us know in a comment if this change resolves your issue.
Thank you!
## Summary
This PR provides an **alternative solution** to issue #28570, which was
previously fixed via property propagation in PR #28615. The issue:
setting `BackButtonBehavior` with `IsVisible="False"` or
`IsEnabled="False"` on a Shell in XAML doesn't work - the back button
still appears.
**This alternative approach uses explicit fallback lookup instead of
automatic property propagation**, making the behavior more predictable
and avoiding side effects.
**Quick verification:**
- ✅ Tested on Android - Issue resolved
- ✅ Tested on iOS - Issue resolved
- ✅ UI tests passing (existing Issue28570 test)
- ✅ No propagation side effects
<details>
<summary><b>📋 Click to expand full PR details</b></summary>
## Problem Statement
When a user sets `BackButtonBehavior` on a `Shell` in XAML:
```xml
<Shell>
<Shell.BackButtonBehavior>
<BackButtonBehavior IsVisible="False"/>
</Shell.BackButtonBehavior>
<!-- Shell content -->
</Shell>
```
The back button should be hidden on all child pages, but it still
appears. The `BackButtonBehavior` set on the Shell is not being applied
to navigated pages.
---
## Original Fix (PR #28615)
The merged PR #28615 solved this by adding `BackButtonBehaviorProperty`
to the property propagation system in
`PropertyPropagationExtensions.cs`:
```csharp
if (propertyName == null || propertyName == Shell.BackButtonBehaviorProperty.PropertyName)
BaseShellItem.PropagateFromParent(Shell.BackButtonBehaviorProperty, element);
```
**How it works**: Automatically propagates `BackButtonBehavior` from
parent (Shell) to child (Page) through the property propagation
infrastructure.
**Issues with this approach**:
1. **Hidden magic**: Developers don't expect attached properties to
propagate automatically
2. **BindingContext conflicts**: Propagation can cause `BindingContext`
inheritance issues (as seen in the Sandbox app testing)
3. **Performance**: Checks and propagates on every property change event
4. **Complexity**: Relies on the property propagation system which is
already complex
---
## Alternative Solution (This PR)
Instead of automatic propagation, this PR implements **explicit fallback
lookup**:
### New Method: `GetEffectiveBackButtonBehavior()`
```csharp
internal static BackButtonBehavior GetEffectiveBackButtonBehavior(BindableObject page)
{
if (page == null)
return null;
// First check if the page has its own BackButtonBehavior
var behavior = GetBackButtonBehavior(page);
if (behavior != null)
return behavior;
// Fallback: check if the Shell itself has a BackButtonBehavior
if (page is Element element)
{
var shell = element.FindParentOfType<Shell>();
if (shell != null)
{
behavior = GetBackButtonBehavior(shell);
if (behavior != null)
return behavior;
}
}
return null;
}
```
### How It Works
1. **Check page first**: Look for `BackButtonBehavior` on the page
itself
2. **Check Shell if not found**: Walk up the tree to find the parent
Shell and check its `BackButtonBehavior`
3. **Return what's found**: First match wins (page overrides Shell)
### Where It's Used
All call sites that previously used `GetBackButtonBehavior()` now use
`GetEffectiveBackButtonBehavior()`:
**Cross-platform**:
- `Shell.OnBackButtonPressed()` - Windows back button handling
- `ShellToolbar.UpdateBackbuttonBehavior()` - Toolbar updates
**Android**:
- `ShellToolbarTracker.OnClick()` - Back button click
- `ShellToolbarTracker.SetPage()` - Page changes
- `ShellToolbarTracker.OnPagePropertyChanged()` - Property updates
- `ShellToolbarTracker.UpdateDrawerArrowFromBackButtonBehavior()` -
Drawer arrow
- `ShellToolbarTracker.UpdateToolbarIconAccessibilityText()` -
Accessibility
**iOS**:
- `ShellSectionRenderer` - Back button in navigation
- `ShellPageRendererTracker.OnPagePropertyChanged()` - Property updates
- `ShellPageRendererTracker.UpdateToolbar()` - Toolbar updates
---
## Advantages of Alternative Approach
| Aspect | Propagation (Original) | Fallback Lookup (This PR) |
|--------|----------------------|---------------------------|
| **Predictability** | Hidden, automatic | Explicit, clear intent |
| **BindingContext** | Can cause conflicts | No interference |
| **Performance** | Checks on every property change | Only checks when
needed |
| **Debugging** | Hard to trace | Easy to follow |
| **Mental Model** | "Magic happens" | "Check page, then Shell" |
| **Side Effects** | Propagation system interactions | None |
### Specific Benefits
1. **No BindingContext Issues**: Avoids the propagation-related
`BindingContext` inheritance problems
2. **Clearer Intent**: The code explicitly says "get from page, or fall
back to Shell"
3. **Better Performance**: Only performs lookup when
`BackButtonBehavior` is actually needed
4. **Easier Maintenance**: No coupling with property propagation
infrastructure
5. **Simpler Mental Model**: Developers can understand the lookup logic
without knowing propagation internals
---
## Root Cause
The original issue existed because Shell's back button handling code
only checked the current page for `BackButtonBehavior`:
```csharp
var backButtonBehavior = GetBackButtonBehavior(GetVisiblePage());
```
If the page didn't have a `BackButtonBehavior` set, it would return
`null`, even though the Shell had one defined. The code had no fallback
mechanism.
---
## Testing
### Before Fix
Setting `BackButtonBehavior` on Shell:
```xml
<Shell>
<Shell.BackButtonBehavior>
<BackButtonBehavior IsVisible="False" TextOverride="BackButton"/>
</Shell.BackButtonBehavior>
</Shell>
```
**Result**: Back button still visible ❌
### After Fix
Same XAML code.
**Result**: Back button hidden ✅
### Test Evidence
```bash
# Run Issue28570 test
pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform android -TestFilter "Issue28570"
# Output:
# >>>>> BackButtonShouldNotBeVisible Start
# >>>>> BackButtonShouldNotBeVisible Stop
# Passed BackButtonShouldNotBeVisible [1 s]
# ✅ All tests passed
```
The test verifies:
1. Navigate to detail page
2. Back button should NOT be visible (because Shell has
`IsVisible="False"`)
3. Test uses `App.WaitForNoElement("BackButton")` to confirm
---
## Files Changed
### Core Changes
**`src/Controls/src/Core/Shell/Shell.cs`**
- ➕ Added `GetEffectiveBackButtonBehavior()` method (lines 200-225)
- ✏️ Modified `OnBackButtonPressed()` to use new method (line 1570)
**`src/Controls/src/Core/Internals/PropertyPropagationExtensions.cs`**
- ➖ Removed `BackButtonBehaviorProperty` propagation (lines 44-45)
**`src/Controls/src/Core/ShellToolbar.cs`**
- ✏️ Modified `UpdateBackbuttonBehavior()` to use new method (line 134)
### Platform-Specific Changes
**Android** -
`src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs`
- ✏️ Updated 5 call sites to use `GetEffectiveBackButtonBehavior()`
- `OnClick()` (line 162)
- `SetPage()` (line 258)
- `OnPagePropertyChanged()` (line 312)
- `UpdateDrawerArrowFromBackButtonBehavior()` (line 410)
- `UpdateToolbarIconAccessibilityText()` (line 543)
**iOS** - Platform-specific handlers
- ✏️ `ShellSectionRenderer.cs` (line 152)
- ✏️ `ShellPageRendererTracker.cs` (lines 136, 216)
**Total**: 8 files changed, 11 call sites updated
---
## Edge Cases Tested
✅ **BackButtonBehavior on Shell only**: Works (this is the fix)
✅ **BackButtonBehavior on Page only**: Works (page takes precedence)
✅ **BackButtonBehavior on both**: Page overrides Shell (expected
behavior)
✅ **No BackButtonBehavior anywhere**: Returns `null` (graceful fallback)
✅ **Multiple navigation levels**: Each page correctly looks up to Shell
---
## Breaking Changes
**None**. This is a pure bug fix that makes the documented behavior work
correctly.
**API Surface**: No public API changes. The new method is `internal`.
**Behavior Changes**:
- ✅ Previously broken: Setting `BackButtonBehavior` on Shell had no
effect
- ✅ Now works: Shell's `BackButtonBehavior` applies to child pages as
expected
---
## Comparison with Original PR #28615
| | PR #28615 (Propagation) | This PR (Fallback Lookup) |
|---|---|---|
| **Lines Changed** | +3 lines | +30 lines |
| **Approach** | Add to propagation list | Explicit lookup method |
| **Call Sites Modified** | 0 | 11 |
| **BindingContext Safe** | 1 parent edda146 commit d247876
File tree
8 files changed
+256
-13
lines changed- src/Controls
- src/Core
- Compatibility/Handlers/Shell
- Android
- iOS
- Internals
- Shell
- tests
- TestCases.HostApp/Issues
8 files changed
+256
-13
lines changedLines changed: 5 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
159 | 159 | | |
160 | 160 | | |
161 | 161 | | |
162 | | - | |
| 162 | + | |
163 | 163 | | |
164 | 164 | | |
165 | 165 | | |
| |||
255 | 255 | | |
256 | 256 | | |
257 | 257 | | |
258 | | - | |
| 258 | + | |
259 | 259 | | |
260 | 260 | | |
261 | 261 | | |
| |||
309 | 309 | | |
310 | 310 | | |
311 | 311 | | |
312 | | - | |
| 312 | + | |
313 | 313 | | |
314 | 314 | | |
315 | 315 | | |
| |||
407 | 407 | | |
408 | 408 | | |
409 | 409 | | |
410 | | - | |
| 410 | + | |
411 | 411 | | |
412 | 412 | | |
413 | 413 | | |
| |||
540 | 540 | | |
541 | 541 | | |
542 | 542 | | |
543 | | - | |
| 543 | + | |
544 | 544 | | |
545 | 545 | | |
546 | 546 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
133 | 133 | | |
134 | 134 | | |
135 | 135 | | |
136 | | - | |
| 136 | + | |
137 | 137 | | |
138 | 138 | | |
139 | 139 | | |
| |||
217 | 217 | | |
218 | 218 | | |
219 | 219 | | |
220 | | - | |
| 220 | + | |
221 | 221 | | |
222 | 222 | | |
223 | 223 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
129 | 129 | | |
130 | 130 | | |
131 | 131 | | |
132 | | - | |
| 132 | + | |
133 | 133 | | |
134 | 134 | | |
135 | 135 | | |
| |||
Lines changed: 0 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
40 | 40 | | |
41 | 41 | | |
42 | 42 | | |
43 | | - | |
44 | | - | |
45 | | - | |
46 | 43 | | |
47 | 44 | | |
48 | 45 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
202 | 202 | | |
203 | 203 | | |
204 | 204 | | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
205 | 233 | | |
206 | 234 | | |
207 | 235 | | |
| |||
1556 | 1584 | | |
1557 | 1585 | | |
1558 | 1586 | | |
1559 | | - | |
| 1587 | + | |
1560 | 1588 | | |
1561 | 1589 | | |
1562 | 1590 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
131 | 131 | | |
132 | 132 | | |
133 | 133 | | |
134 | | - | |
| 134 | + | |
135 | 135 | | |
136 | 136 | | |
137 | 137 | | |
| |||
Lines changed: 174 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
Lines changed: 44 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
0 commit comments