Fix Shell Navigating event not firing on ShellContent change - #34351
Conversation
|
Hi, just checking if this PR could be reviewed for the upcoming 10.6 servicing release. This fix ensures that the Shell.Navigating event is correctly triggered when the ShellContent changes, restoring expected navigation lifecycle behavior. The change is minimal and scoped to the navigation event flow, without affecting public APIs, so it should be safe for servicing. Let me know if any additional validation or scenarios should be covered 👍 |
kubaflo
left a comment
There was a problem hiding this comment.
Could you please add a test? You can use the write-test agent
Yes sure, let me work on it |
There was a problem hiding this comment.
Pull request overview
This PR addresses a behavioral gap in .NET MAUI Shell on Windows where switching ShellContent within the same ShellSection does not raise the Shell.Navigating event, leading to inconsistent event sequencing (Navigated without Navigating). It also adds a UI test case intended to validate the corrected event ordering.
Changes:
- Update
ShellSection.OnCurrentItemChangedto invoke the navigating pipeline before callingUpdateCurrentStateforShellNavigationSource.ShellContentChanged. - Add a HostApp reproduction (
Issue34318) to switchShellContentand surface a signal whenNavigatingfires. - Add an Appium-based UI test (
Issue34318) that taps a button and asserts the app observed theNavigatingevent.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/Controls/src/Core/Shell/ShellSection.cs | Adds a HandleNavigating call and cancellation check when ShellSection.CurrentItem changes. |
| src/Controls/tests/TestCases.HostApp/Issues/Issue34318.cs | Adds a Shell-based repro page that attempts to broadcast when Shell.Navigating fires and switches ShellContent. |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34318.cs | Adds a UI test that taps a button and asserts the “Navigating” signal is observed. |
kubaflo
left a comment
There was a problem hiding this comment.
Looks like the test is failing
…erage - Ensure Navigating event is triggered when ShellContent changes - Add UITest to validate expected behavior - Replace MessagingCenter usage with direct Shell.Navigating subscription - Fix parent traversal to correctly resolve ShellSection - Remove unused variable to avoid warnings-as-errors
|
Hi team, just a quick follow-up on this PR. CI is passing and the change is scoped to the specific scenario covered here. Thanks! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 2 findings
See inline comments for details.
|
|
||
| Items.Add(item); | ||
|
|
||
| Navigating += (_, __) => |
There was a problem hiding this comment.
[major] Regression Prevention — The regression handler ignores ShellNavigatingEventArgs, so the test only proves that some Navigating event fired once. It would still pass if the fix used the wrong Source, exposed incorrect Current/Target routes, or fired after ShellSection.CurrentItem was already changed. Capture the args here and assert Source == ShellNavigationSource.ShellContentChanged, CanCancel == false, the expected current/target locations, and the selected content observed during the handler so the test protects the Shell navigation semantics, not just the event count.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@jpd21122012 — new AI review results are available based on this last commit:
9bcf38f. To request a fresh review after new comments or commits, comment/review rerun.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate Result: ✅ PASSED
Platform: ANDROID · Base: main · Merge base: 2a323a50
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🖥️ Issue34318 Issue34318 |
✅ FAIL — 2338s | ✅ PASS — 554s |
🔴 Without fix — 🖥️ Issue34318: FAIL ✅ · 2338s
(truncated to last 15,000 chars)
/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Xamarin.Android.Tasks.FastDeploy.InstallPackage(Boolean installed) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Xamarin.Android.Tasks.FastDeploy.RunInstall() [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
Build FAILED.
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: Mono.AndroidTools.InstallFailedException: Unexpected install output: cmd: Failure calling service package: Broken pipe (32) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Mono.AndroidTools.Internal.AdbOutputParsing.CheckInstallSuccess(String output, String packageName) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Mono.AndroidTools.AndroidDevice.<>c__DisplayClass105_0.<InstallPackage>b__0(Task`1 t) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: --- End of stack trace from previous location --- [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: --- End of stack trace from previous location --- [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at AndroidDeviceExtensions.PushAndInstallPackageAsync(AndroidDevice device, PushAndInstallCommand command, CancellationToken token) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at AndroidDeviceExtensions.PushAndInstallPackageAsync(AndroidDevice device, PushAndInstallCommand command, CancellationToken token) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Xamarin.Android.Tasks.FastDeploy.InstallPackage(Boolean installed) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Xamarin.Android.Tasks.FastDeploy.InstallPackage(Boolean installed) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Xamarin.Android.Tasks.FastDeploy.RunInstall() [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
0 Warning(s)
1 Error(s)
Time Elapsed 00:16:32.71
* daemon not running; starting now at tcp:5037
* daemon started successfully
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0-android36.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0-android36.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0-android36.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0-android36.0/Microsoft.Maui.Maps.dll
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-android36.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Foldable.dll
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-android36.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Xaml.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Maps.dll
Controls.TestCases.HostApp -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Controls.TestCases.HostApp.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Graphics -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Essentials -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Maps.dll
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Foldable.dll
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Xaml.dll
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Maps.dll
Build succeeded.
0 Warning(s)
0 Error(s)
Time Elapsed 00:08:21.94
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Starting: Intent { act=android.settings.SETTINGS }
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Determining projects to restore...
Restored /home/vsts/work/1/s/src/TestUtils/src/VisualTestUtils/VisualTestUtils.csproj (in 4.31 sec).
Restored /home/vsts/work/1/s/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj (in 3.78 sec).
Restored /home/vsts/work/1/s/src/Controls/tests/TestCases.Android.Tests/Controls.TestCases.Android.Tests.csproj (in 8.24 sec).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.Core/UITest.Core.csproj (in 7 ms).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.Appium/UITest.Appium.csproj (in 2 ms).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.NUnit/UITest.NUnit.csproj (in 290 ms).
Restored /home/vsts/work/1/s/src/Controls/tests/CustomAttributes/Controls.CustomAttributes.csproj (in 6 ms).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.Analyzers/UITest.Analyzers.csproj (in 1.61 sec).
5 of 13 projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
Controls.CustomAttributes -> /home/vsts/work/1/s/artifacts/bin/Controls.CustomAttributes/Debug/net10.0/Controls.CustomAttributes.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
VisualTestUtils -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils/Debug/netstandard2.0/VisualTestUtils.dll
UITest.Core -> /home/vsts/work/1/s/artifacts/bin/UITest.Core/Debug/net10.0/UITest.Core.dll
VisualTestUtils.MagickNet -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils.MagickNet/Debug/netstandard2.0/VisualTestUtils.MagickNet.dll
UITest.Appium -> /home/vsts/work/1/s/artifacts/bin/UITest.Appium/Debug/net10.0/UITest.Appium.dll
UITest.NUnit -> /home/vsts/work/1/s/artifacts/bin/UITest.NUnit/Debug/net10.0/UITest.NUnit.dll
UITest.Analyzers -> /home/vsts/work/1/s/artifacts/bin/UITest.Analyzers/Debug/netstandard2.0/UITest.Analyzers.dll
Controls.TestCases.Android.Tests -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
NUnit3TestExecutor discovered 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 06/27/2026 13:09:06 FixtureSetup for Issue34318(Android)
>>>>> 06/27/2026 13:09:09 NavigatingFiresWhenShellContentChanges Start
>>>>> 06/27/2026 13:09:29 NavigatingFiresWhenShellContentChanges Stop
>>>>> 06/27/2026 13:09:29 Log types: logcat, bugreport, server
Failed NavigatingFiresWhenShellContentChanges [20 s]
Error Message:
Navigating event should have fired and updated the label text
Assert.That(result, Is.True)
Expected: True
But was: False
Stack Trace:
at Microsoft.Maui.TestCases.Tests.Issues.Issue34318.NavigatingFiresWhenShellContentChanges() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34318.cs:line 30
1) at Microsoft.Maui.TestCases.Tests.Issues.Issue34318.NavigatingFiresWhenShellContentChanges() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34318.cs:line 30
NUnit Adapter 4.5.0.0: Test execution complete
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.16] Discovering: Controls.TestCases.Android.Tests
[xUnit.net 00:00:00.63] Discovered: Controls.TestCases.Android.Tests
Results File: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue34318.trx
Total tests: 1
Failed: 1
Test Run Failed.
Total time: 50.2252 Seconds
>>> TRX_RESULT_FILE: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue34318.trx
🟢 With fix — 🖥️ Issue34318: PASS ✅ · 554s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0-android36.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0-android36.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0-android36.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0-android36.0/Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-android36.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Maps.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Xaml.dll
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Foldable.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-android36.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.TestCases.HostApp -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Controls.TestCases.HostApp.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Graphics -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Essentials -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Maps.dll
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Foldable.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Maps.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Xaml.dll
Build succeeded.
0 Warning(s)
0 Error(s)
Time Elapsed 00:06:59.20
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Starting: Intent { act=android.settings.SETTINGS }
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Determining projects to restore...
All projects are up-to-date for restore.
Controls.CustomAttributes -> /home/vsts/work/1/s/artifacts/bin/Controls.CustomAttributes/Debug/net10.0/Controls.CustomAttributes.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14508053
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
VisualTestUtils -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils/Debug/netstandard2.0/VisualTestUtils.dll
UITest.Core -> /home/vsts/work/1/s/artifacts/bin/UITest.Core/Debug/net10.0/UITest.Core.dll
UITest.Appium -> /home/vsts/work/1/s/artifacts/bin/UITest.Appium/Debug/net10.0/UITest.Appium.dll
UITest.NUnit -> /home/vsts/work/1/s/artifacts/bin/UITest.NUnit/Debug/net10.0/UITest.NUnit.dll
VisualTestUtils.MagickNet -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils.MagickNet/Debug/netstandard2.0/VisualTestUtils.MagickNet.dll
UITest.Analyzers -> /home/vsts/work/1/s/artifacts/bin/UITest.Analyzers/Debug/netstandard2.0/UITest.Analyzers.dll
Controls.TestCases.Android.Tests -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
NUnit3TestExecutor discovered 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 06/27/2026 13:28:28 FixtureSetup for Issue34318(Android)
>>>>> 06/27/2026 13:28:30 NavigatingFiresWhenShellContentChanges Start
>>>>> 06/27/2026 13:28:34 NavigatingFiresWhenShellContentChanges Stop
Passed NavigatingFiresWhenShellContentChanges [4 s]
NUnit Adapter 4.5.0.0: Test execution complete
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.14] Discovering: Controls.TestCases.Android.Tests
[xUnit.net 00:00:00.56] Discovered: Controls.TestCases.Android.Tests
Results File: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue34318.trx
Test Run Successful.
Total tests: 1
Passed: 1
Total time: 19.8016 Seconds
>>> TRX_RESULT_FILE: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue34318.trx
📁 Fix files reverted (1 files)
src/Controls/src/Core/Shell/ShellSection.cs
📱 UI Tests — Shell
Detected UI test categories: Shell
✅ Deep UI tests — 308 passed, 0 failed across 1 category on platform-pool agent (replaces in-process counts above).
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
Shell |
308/308 ✓ | — |
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs) |
📋 Pre-Flight — Context & Validation
Issue: #34318 - Shell Navigating event should fire on ShellContent change
PR: #34351 - ShellContent change should raise Shell.Navigating
Platforms Affected: All; gate platform Android
Files Changed: 1 implementation, 2 test
Key Findings
- PR changes
src/Controls/src/Core/Shell/ShellSection.csso direct, non-handlerShellSection.CurrentItemchanges proposeShellNavigationSource.ShellContentChangednavigation beforeUpdateCurrentState. - PR adds HostApp/UI coverage for Issue34318; gate artifact reports the Android UI test fails without the fix and passes with the PR fix.
- GitHub CLI is unauthenticated in this environment, so live PR metadata, comments, and required-check status could not be fetched; context comes from the checked-out PR commit, local diff, and existing gate artifacts.
- Impacted UI category:
Shell.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 2 | Suggestions: 1
Key code review findings:
- ⚠
src/Controls/src/Core/Shell/ShellSection.cs:1032-1051—Navigatingfires frompropertyChanged, afterShellSection.CurrentItemis already committed; this differs from theShell.CurrentItempropertyChangingpattern. - ⚠
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34318.cs:28-35— UI regression verifies event count but notShellNavigatingEventArgssemantics. - ℹ
src/Controls/tests/TestCases.HostApp/Issues/Issue34318.cs:73-82— test handler could update labels synchronously instead of usingMainThread.BeginInvokeOnMainThread.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #34351 | Fire a non-cancellable ProposeNavigationOutsideGotoAsync from ShellSection.OnCurrentItemChanged for visible, non-handler ShellContent changes. |
✅ PASSED (Gate) | ShellSection.cs, Issue34318 UI test files |
Original PR fix; Android gate passed. |
🔬 Code Review — Deep Analysis
Code Review — PR #34351
Independent Assessment
What this changes: Adds a ShellSection.CurrentItem change path that raises Shell.Navigating for visible ShellContent changes, plus a UI regression test.
Inferred motivation: Direct ShellSection.CurrentItem changes updated Shell.CurrentState/Navigated without first notifying Navigating.
Reconciliation with PR Narrative
Author claims: Fixes missing Navigating for ShellContent changes and respects cancellation.
Agreement/disagreement: The event now fires before UpdateCurrentState, matching the main goal. However, the current code passes canCancel: false, so the “respect cancellation” claim no longer matches implementation.
Prior Review Reconciliation
| Prior ❌ Error Finding | Source | Status | Evidence |
|---|---|---|---|
Duplicate Navigating from both propertyChanging and propertyChanged |
MauiBot inline/reviews | ✅ Fixed | Current diff only calls ProposeNavigationOutsideGotoAsync from OnCurrentItemChanged; no propertyChanging callback is added. |
| Post-commit cancellable navigation corrupts state | MauiBot | ✅ Fixed / Obsolete | Current call uses canCancel: false; no cancellable post-commit early-return path exists. |
AccumulateNavigatedEvents bypass via direct HandleNavigating |
MauiBot | ✅ Fixed | Current code uses ProposeNavigationOutsideGotoAsync, which checks AccumulateNavigatedEvents. |
CurrentItem = null skips old item disappearing |
MauiBot | ✅ Fixed | oldShellItem.SendDisappearing() remains before if (newValue == null) return; at ShellSection.cs:1054-1058. |
Blast Radius Assessment
- Runs for all instances: Yes — every visible
ShellSection.CurrentItemmutation not markedFromHandler. - Startup impact: Low; no static/startup path changed.
- Static/shared state: No.
CI Status
- Required-check result: unavailable via
gh pr checksbecause GitHub CLI auth is unavailable. - Classification: undetermined for required checks. Gate artifact in this workspace reports the Android Issue34318 gate passed.
- Action taken: capped confidence low; no GitHub comments/reviews posted.
Findings
⚠️ Warning — Navigating fires after ShellSection.CurrentItem is already committed
src/Controls/src/Core/Shell/ShellSection.cs:1032-1051
This differs from Shell.CurrentItem, which fires its navigation proposal from propertyChanging (Shell.cs:1217-1220, Shell.cs:1888-1899). Event args should still have the old/current route because UpdateCurrentState runs later, but subscribers that inspect shellSection.CurrentItem inside Navigating will already see newContent. This may be acceptable, but it is an observable semantic difference worth confirming or documenting.
⚠️ Warning — Regression test verifies event count, but not event semantics
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34318.cs:28-35
The UI test confirms Navigating fires once, but it cannot assert Source == ShellContentChanged, CanCancel == false, or current/target routes. Existing unit-test helpers already validate navigation args for other Shell paths; a focused unit test for direct ShellSection.CurrentItem assignment would cover this more reliably than UI-only coverage.
💡 Suggestion — Avoid unnecessary async hop in the test handler
src/Controls/tests/TestCases.HostApp/Issues/Issue34318.cs:73-82
Navigating should already run on the UI thread for this button-driven change. Updating labels synchronously would make the test less timing-dependent than MainThread.BeginInvokeOnMainThread.
Failure-Mode Probing
- Direct
section.CurrentItem = section.Items[1]: fires one non-cancellableNavigating, then updates current state. - Handler-originated selection: skipped by
SetterSpecificity.FromHandler, avoiding duplicate native-path events. newValue == null: old content still receivesSendDisappearing()before returning.GoToAsyncaccumulation: guarded byProposeNavigationOutsideGotoAsync.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: The current code appears to address prior blocking duplicate/cancellation issues, but the event now fires post-commit for ShellSection.CurrentItem, which is a semantic difference from Shell.CurrentItem. Required CI status could not be verified through gh, so this should not be marked LGTM here.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix-1 | Move ShellContent navigation proposal to ShellSection.CurrentItemProperty propertyChanging, preserving handler-origin suppression and canCancel: false. |
✅ PASS | 1 file | Better event ordering than the PR: Navigating fires before ShellSection.CurrentItem is committed, matching the Shell.CurrentItem pattern. |
| PR | PR #34351 | Fire ProposeNavigationOutsideGotoAsync from ShellSection.OnCurrentItemChanged for visible, non-handler ShellContent changes. |
✅ PASSED (Gate) | 3 files | Original PR fix; Android gate passed but fires Navigating after CurrentItem is committed. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| maui-expert-reviewer | 1 | Yes | Recommended the pre-commit propertyChanging candidate, plus broader ShellItem/ShellSection consistency and navigation-manager-helper variants if the first candidate failed. |
Exhausted: No — stopped because candidate #1 passed all Android regression criteria and is better than the PR's current fix on event-ordering semantics.
Selected Fix: Candidate #1 — same tested behavior as the PR, but raises Shell.Navigating before the ShellSection.CurrentItem mutation is observable to subscribers.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the winning fix changes the PR from a late propertyChanged event to pre-commit propertyChanging semantics, and the current description incorrectly says cancellation is respected even though this ShellContent change remains non-cancellable.
Recommended title
Shell: Fire Navigating before ShellContent changes commit
Recommended description
Fixes #34318
### Problem
When switching between `ShellContent` items within the same `ShellSection` (for example A1 → A2), the `Navigating` event is not fired on Windows.
Only the `Navigated` event is raised.
This creates inconsistent behavior compared to other navigation scenarios where `Navigating` is expected to fire before the navigation completes.
### Root Cause
`ShellSection.CurrentItem` changes update Shell state through `UpdateCurrentState(ShellNavigationSource.ShellContentChanged)` without first invoking the navigation pipeline through `ShellNavigationManager`.
Because of this, the `Navigating` event is skipped for direct `ShellSection.CurrentItem` changes.
### Fix
Trigger the navigation pipeline from `ShellSection.CurrentItemProperty` `propertyChanging` before the new `ShellContent` is committed:
1. Build the proposed navigation state for `ShellNavigationSource.ShellContentChanged`
2. Call `NavigationManager.ProposeNavigationOutsideGotoAsync`
3. Keep the ShellContent change non-cancellable with `canCancel: false`
4. Let `OnCurrentItemChanged` continue updating lifecycle, Shell state, structure, appearance, and displayed page after the property value changes
The fix preserves handler-origin suppression so native handler-driven tab/content selection does not double-raise navigation events.
### Result
Switching between `ShellContent` items now correctly raises:
Navigating → Navigated
with `ShellNavigationSource.ShellContentChanged`.
`Navigating` is raised before `ShellSection.CurrentItem` is committed to the target content, so event handlers observe the navigation as pending instead of already applied.
### Tested
Reproduced with the sample from the issue:
- Navigate to Page2
- Go to Tab A
- Switch between Content A1 and Content A2
`Navigating` now fires correctly.
Regression coverage verifies:
- `Navigating` fires exactly once
- `Source` is `ShellNavigationSource.ShellContentChanged`
- `CanCancel` is `false`
- current and target routes point to the expected ShellContent items
- `ShellSection.CurrentItem` still points to the previous content while `Navigating` is raised
🏁 Report — Final Recommendation
Comparative Fix Report — PR #34351
Candidates compared
| Rank | Candidate | Regression result | Assessment |
|---|---|---|---|
| 1 | pr-plus-reviewer |
Not re-run; core implementation is the passed try-fix-1 approach plus stronger assertions |
Winner. Raises Shell.Navigating before ShellSection.CurrentItem is committed, preserves handler-origin suppression and non-cancellable ShellContent navigation semantics, and strengthens the regression test to verify Source, CanCancel, routes, and observable selection state. |
| 2 | try-fix-1 |
✅ PASS on Android | Correct core implementation. It moves the proposal to propertyChanging, matching Shell's existing pre-commit navigation pattern, and passed the Android regression criteria. It ranks below pr-plus-reviewer only because it does not include the expert reviewer's expanded test coverage. |
| 3 | pr |
✅ PASSED gate | Fixes the missing event and passed the gate, but raises Navigating from propertyChanged after ShellSection.CurrentItem is already set to the target. That means event subscribers can observe navigation as already applied while the args still describe the previous current route. |
Regression-test ordering rule
No candidate with a failed regression test ranked above a passing candidate. The raw PR and try-fix-1 both passed their Android regression evidence; no failed STEP 5a candidates were present in the saved try-fix context.
Key comparison
The raw PR adds the missing ProposeNavigationOutsideGotoAsync call, but places it in OnCurrentItemChanged. That is enough to raise the event, but it weakens Shell's event contract because Navigating is expected to describe a pending transition. try-fix-1 improves this by moving the proposal to CurrentItemProperty propertyChanging, matching the pattern used by Shell.CurrentItem changes.
The expert reviewer independently identified the same ordering issue and also found that the regression test should verify event semantics, not only event count. Applying that feedback creates pr-plus-reviewer: the try-fix-1 implementation plus assertions for ShellNavigationSource.ShellContentChanged, CanCancel == false, expected current/target routes, and the current content observed while Navigating is raised.
Winning candidate
Winner: pr-plus-reviewer
Rationale: It keeps the tested behavioral fix, corrects the event-ordering flaw in the submitted PR, and adds targeted regression coverage for the Shell navigation semantics that would catch the original late-event implementation. The raw PR remains functional for the narrow gate scenario, but it is less correct for subscribers that inspect Shell state during Navigating.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
Fixes #34318
Problem
When switching between
ShellContentitems within the sameShellSection(for example A1 → A2), theNavigatingevent is not fired on Windows.Only the
Navigatedevent is raised.This creates inconsistent behavior compared to other navigation scenarios where
Navigatingis expected to fire before the navigation completes.Root Cause
ShellSection.OnCurrentItemChangeddirectly calls:UpdateCurrentState(ShellNavigationSource.ShellContentChanged)
without first invoking the navigation pipeline through
ShellNavigationManager.HandleNavigating.Because of this, the
Navigatingevent is skipped.Fix
Trigger the navigation pipeline before updating the current state:
NavigationManager.HandleNavigatingUpdateCurrentStateResult
Switching between
ShellContentitems now correctly raises:Navigating → Navigated
with
ShellNavigationSource.ShellContentChanged.Tested
Reproduced with the sample from the issue:
Navigatingnow fires correctly.