Skip to content

[iOS, Mac] Fix Shell.CurrentState.Location stale in OnNavigated after GoToAsync#34880

Merged
kubaflo merged 2 commits into
dotnet:inflight/currentfrom
Vignesh-SF3580:fix-34662
Apr 17, 2026
Merged

[iOS, Mac] Fix Shell.CurrentState.Location stale in OnNavigated after GoToAsync#34880
kubaflo merged 2 commits into
dotnet:inflight/currentfrom
Vignesh-SF3580:fix-34662

Conversation

@Vignesh-SF3580
Copy link
Copy Markdown
Contributor

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!

Issue Details

After calling GoToAsync("//DashboardPage/Page1/Page2") on iOS, Shell.CurrentState.Location inside Shell.OnNavigated incorrectly reports //DashboardPage instead of the full route //DashboardPage/Page1/Page2.

Regression Details

Introduced by PR #25749 (fix for #25599). The #if IOS || MACCATALYST block added to GetNavigationState was too broad—it executed in both UpdateCurrentState and ProposeNavigationOutsideGotoAsync, causing Shell.CurrentState to be truncated before OnNavigated was triggered.

Root Cause

GetNavigationState is called from two key paths:
ProposeNavigationOutsideGotoAsync — Generates the proposed state for the Navigating event during native tab taps. This is the intended scope of the #25599 fix.
UpdateCurrentState — Updates Shell.CurrentState after navigation completes and triggers OnNavigated. The iOS-specific trimming logic should not execute here.
When navigating to //DashboardPage/Page1/Page2 on iOS, UpdateCurrentState invokes GetNavigationState, and the iOS logic incorrectly detects a match and truncates the route to //DashboardPage. This truncated value is then set as Shell.CurrentState before OnNavigated is fired.

Description of Change

Added an optional bool isNavigateThroughTab = false parameter to GetNavigationState and used it to guard the #if IOS || MACCATALYST route-trimming logic. Only ProposeNavigationOutsideGotoAsync—the specific call site where the #25599 fix is required—passes isNavigateThroughTab: true. All other callers (UpdateCurrentState, IShellController.GetNavigationState, etc.) use the default value (false), ensuring Shell.CurrentState is always set with the correct, untruncated route.

Issues Fixed

Fixes #34662

Screenshots

Before Issue Fix After Issue Fix
34662BeforeFix.mov
34662AfterFix.mov

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 8, 2026

🚀 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 -- 34880

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34880"

@dotnet-policy-service dotnet-policy-service Bot added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Apr 8, 2026
@vishnumenon2684 vishnumenon2684 added the community ✨ Community Contribution label Apr 8, 2026
@sheiksyedm sheiksyedm marked this pull request as ready for review April 8, 2026 13:20
Copilot AI review requested due to automatic review settings April 8, 2026 13:20
@sheiksyedm sheiksyedm added platform/ios area-controls-shell Shell Navigation, Routes, Tabs, Flyout labels Apr 8, 2026
@sheiksyedm sheiksyedm added this to the .NET 10 SR6 milestone Apr 8, 2026
@sheiksyedm
Copy link
Copy Markdown
Contributor

/azp run maui-pr-uitests , maui-pr-devicetests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an iOS/MacCatalyst Shell navigation regression where Shell.CurrentState.Location could be truncated/stale inside Shell.OnNavigated after a deep GoToAsync(...), by limiting the iOS-specific route-trimming logic to the tab-tap “Navigating” path only.

Changes:

  • Add an isNavigateThroughTab parameter to ShellNavigationManager.GetNavigationState to scope the iOS/MacCatalyst route-trimming fix to tab-based navigation proposals.
  • Update ProposeNavigationOutsideGotoAsync to opt into the iOS/MacCatalyst trimming behavior via isNavigateThroughTab: true.
  • Add a new HostApp repro page and Appium UI test for issue #34662 validating Shell.CurrentState.Location inside OnNavigated after absolute deep navigation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/Controls/src/Core/Shell/ShellNavigationManager.cs Gates iOS/MacCatalyst route-trimming behavior behind a new isNavigateThroughTab flag so UpdateCurrentState doesn’t get a truncated route.
src/Controls/tests/TestCases.HostApp/Issues/Issue34662.cs Adds a Shell-based repro that captures CurrentState.Location during OnNavigated and surfaces it in the UI for validation.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34662.cs Adds an Appium UI test asserting the captured OnNavigated location matches the full deep route.

Comment thread src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34662.cs Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 8, 2026

🏷️ Test Categories for Regression Detection

The Shell UI test category was detected directly from [Category(UITestCategories.Shell)] in the added test code (Issue34662.cs). The source change to ShellNavigationManager.cs also confirms Shell as the affected area, and warrants running device tests for the Shell category.

UI Test Categories

Category Source
Shell Detected from [Category(UITestCategories.Shell)] in test diff

Pipeline filter: Shell

Device Test Categories

Category Project Source
Shell Controls Inferred from ShellNavigationManager.cs source change

Recommendation: Run device tests: Yes — Shell navigation manager changes can affect device-level Shell behavior.

🚀 Run Targeted Tests on Existing Pipelines

Both maui-pr-uitests and maui-pr-devicetests now support category filtering parameters. To run only the relevant tests for this PR:

UI Tests — trigger maui-pr-uitests with:

Parameter: uiTestCategories = Shell

Device Tests — trigger maui-pr-devicetests with:

Parameter: deviceTestCategories = Shell

When triggered without parameters (e.g., by normal PR push), all categories run as usual.

📁 Changed files (3)

Test files (2):

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

Source files (1):

  • src/Controls/src/Core/Shell/ShellNavigationManager.cs

ℹ️ Categories are detected from [Category()] attributes in the diff and inferred from changed source file paths.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🏷️ Category detection by Detect Test Categories for Regression Detection

@MauiBot MauiBot added s/agent-review-incomplete s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) labels Apr 10, 2026
@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented Apr 12, 2026

Code Review — PR #34880

Independent Assessment

What this changes: Adds an optional bool isNavigateThroughTab = false parameter to ShellNavigationManager.GetNavigationState. The #if IOS || MACCATALYST block that trims duplicate route segments now only executes when isNavigateThroughTab is true. Only one call site passes true: ProposeNavigationOutsideGotoAsync (native tab-tap navigation). All other callers (UpdateCurrentState, IShellController, ShellSection) use the default false.

Inferred motivation: The iOS-specific route-trimming block (introduced in PR #25749 to fix #25599 — Navigating event showing same Current and Target on tab re-tap) was executing unconditionally in GetNavigationState, including when called from UpdateCurrentState. During deep navigation via GoToAsync("//DashboardPage/Page1/Page2"), UpdateCurrentState calls GetNavigationState which incorrectly matched the route and truncated it to //DashboardPage, making Shell.CurrentState.Location stale in OnNavigated.

Reconciliation with PR Narrative

Author claims: Regression from PR #25749. The iOS block in GetNavigationState was too broad — it should only apply during native tab-tap navigation (ProposeNavigationOutsideGotoAsync), not when updating Shell.CurrentState.

Agreement: My independent analysis fully matches the author's root cause analysis. The call-site audit confirms that only ProposeNavigationOutsideGotoAsync is the tab-tap path. The fix is surgically scoped.

Findings

✅ Correct — Minimal, surgical fix targeting the exact regression

The fix narrows the existing iOS workaround to its intended scope without removing it. The original #25599 fix remains functional for tab re-tap scenarios. All 7 other callers of GetNavigationState are unaffected (they use the false default). No behavioral change on Android or Windows (the block is #if IOS || MACCATALYST).

✅ Correct — Default parameter preserves backward compatibility

GetNavigationState is public static on ShellNavigationManager, but it's not tracked in PublicAPI.Shipped.txt (only the IShellController.GetNavigationState wrapper is). The default parameter = false means existing callers compile without changes. No API break.

✅ Correct — Existing #25599 test preserved

The test for the original fix (Issue25599.xaml.cs / Issue25599_1.cs) remains in the repo and is not modified. The fix won't regress #25599 because ProposeNavigationOutsideGotoAsync still passes isNavigateThroughTab: true.

✅ Correct — UI test is well-constructed

The test navigates from LoginPage to //DashboardPage/Page1/Page2 via GoToAsync, then verifies Shell.CurrentState.Location inside OnNavigated matches the full route. Uses WaitForTextToBePresentInElement with a 5-second timeout to handle async timing (addressing copilot-reviewer's suggestion). The SetCurrentStateLocation approach is appropriate since OnNavigated fires after OnAppearing.

💡 Suggestion — Consider adding a comment at the GetNavigationState signature

The isNavigateThroughTab parameter name is descriptive, but future contributors may not understand why it exists without reading the call site. A brief <param> XML doc on the parameter would help:

/// <param name="isNavigateThroughTab">When true, applies iOS-specific route deduplication
/// for the Navigating event during native tab taps (#25599). Must be false for
/// UpdateCurrentState to prevent stale Shell.CurrentState (#34662).</param>

💡 Suggestion — Shell component regression risk (§21, §22 of review rules)

Shell/TabBar is in the "frequently regressed components" list (4 regressions tracked). This fix specifically touches the Shell navigation state path. The existing #25599 UI test provides coverage for the original scenario, and the new #34662 test covers the regression. Both should be run on iOS CI to verify no interaction. The maui-pr-uitests and maui-pr-devicetests were triggered — check those results when available.

CI Status

  • maui-pr: ❌ Helix Unit Tests (Debug) failed — only a 2-minute run, likely infrastructure/flake, not related to this Shell-only iOS change
  • All builds, integration tests, template tests: ✅ Pass
  • maui-pr-uitests and maui-pr-devicetests: Were triggered separately — results should be checked for Shell category tests

Devil's Advocate

  1. Could this break the original OnNavigating wrong target when tapping the same tab #25599 fix? No — ProposeNavigationOutsideGotoAsync is the only tab-tap entry point and it passes isNavigateThroughTab: true. The iOS trimming block still fires in exactly the same scenario as before.

  2. Are there other callers we missed? I audited all 7 call sites: Shell.cs:899 (IShellController), Shell.cs:955 (CurrentItem setter), Shell.cs:1041 (UpdateCurrentState), ShellNavigationManager.cs:509 (UpdateCurrentState internal), ShellSection.cs:1131, ShellSection.cs:1269. None of these are tab-tap paths — they all correctly use the default false.

  3. Could a future caller pass true incorrectly? The parameter name isNavigateThroughTab is semantically clear. A brief XML doc (suggested above) would make the contract explicit.

  4. Is this iOS-only fix safe for macCatalyst? Yes — the #if IOS || MACCATALYST scope is unchanged. The behavioral narrowing applies equally to both platforms.

Verdict: LGTM

Confidence: high

Summary: Clean, minimal regression fix that narrows the scope of an existing iOS workaround to its intended call site. The root cause analysis is correct and well-documented. The UI test is robust. No API break, no impact on other platforms, and the original #25599 fix is preserved. Shell is a frequently-regressed component, so iOS UI test CI results should be verified before merge.

@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented Apr 12, 2026

/azp run maui-pr-uitests , maui-pr-devicetests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Copy Markdown
Contributor

@kubaflo kubaflo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the tests are failing

@PureWeen PureWeen modified the milestones: .NET 10 SR6, .NET 10 SR7 Apr 14, 2026
@Vignesh-SF3580
Copy link
Copy Markdown
Contributor Author

Looks like the tests are failing

@kubaflo The device test failures are related to Windows, while my fix is related to iOS. The integration test failures are also occurring in other PRs, so they do not appear to be related to my fix.

@MauiBot
Copy link
Copy Markdown
Collaborator

MauiBot commented Apr 17, 2026

🚦 Gate — Test Before and After Fix

👋 @Vignesh-SF3580 — new gate results are available. Please review the latest session below.

🚦 Gate Sessiond9e4e74 · Update Issue34662.cs · 2026-04-17 09:29 UTC

Gate Result: ✅ PASSED

Platform: IOS · Base: main · Merge base: eb0b82fe

Test Without Fix (expect FAIL) With Fix (expect PASS)
🖥️ Issue34662 Issue34662 ✅ FAIL — 226s ✅ PASS — 91s
🔴 Without fix — 🖥️ Issue34662: FAIL ✅ · 226s
  Determining projects to restore...
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/BindingSourceGen/Controls.BindingSourceGen.csproj (in 561 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 577 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Essentials/src/Essentials.csproj (in 8.6 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 13.34 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj (in 13.34 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/Foldable/src/Controls.Foldable.csproj (in 13.36 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Xaml/Controls.Xaml.csproj (in 13.35 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/BlazorWebView/src/Maui/Microsoft.AspNetCore.Components.WebView.Maui.csproj (in 13.36 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Core/src/Core.csproj (in 13.36 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Core/maps/src/Maps.csproj (in 13.37 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/Maps/src/Controls.Maps.csproj (in 13.38 sec).
/Users/cloudtest/vss/_work/1/s/.dotnet/packs/Microsoft.iOS.Sdk.net10.0_26.0/26.0.11017/targets/Xamarin.Shared.Sdk.targets(309,3): warning : RuntimeIdentifier was set on the command line, and will override the value for RuntimeIdentifiers set in the project file. [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-ios]
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0-ios26.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0-ios26.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0-ios26.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Maps/Debug/net10.0-ios26.0/Microsoft.Maui.Maps.dll
  Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Controls.Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Maps.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Controls.Xaml -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Xaml.dll
  Microsoft.AspNetCore.Components.WebView.Maui -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-ios26.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
  Controls.Foldable -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Foldable.dll
  Detected signing identity:
    Code Signing Key: "" (-)
    Provisioning Profile: "" () - no entitlements
    Bundle Id: com.microsoft.maui.uitests
    App Id: com.microsoft.maui.uitests
  Controls.TestCases.HostApp -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-ios/iossimulator-arm64/Controls.TestCases.HostApp.dll
  Optimizing assemblies for size may change the behavior of the app. Be sure to test after publishing. See: https://aka.ms/dotnet-illink
  Optimizing assemblies for size. This process might take a while.

Build succeeded.

/Users/cloudtest/vss/_work/1/s/.dotnet/packs/Microsoft.iOS.Sdk.net10.0_26.0/26.0.11017/targets/Xamarin.Shared.Sdk.targets(309,3): warning : RuntimeIdentifier was set on the command line, and will override the value for RuntimeIdentifiers set in the project file. [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-ios]
    1 Warning(s)
    0 Error(s)

Time Elapsed 00:01:45.10
  Determining projects to restore...
  Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/UITest.Core/UITest.Core.csproj (in 914 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/VisualTestUtils/VisualTestUtils.csproj (in 914 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/tests/CustomAttributes/Controls.CustomAttributes.csproj (in 2 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/BindingSourceGen/Controls.BindingSourceGen.csproj (in 442 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 1.62 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/UITest.NUnit/UITest.NUnit.csproj (in 1.65 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Essentials/src/Essentials.csproj (in 1.73 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/UITest.Appium/UITest.Appium.csproj (in 1.8 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 1.92 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Core/src/Core.csproj (in 1.94 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/UITest.Analyzers/UITest.Analyzers.csproj (in 2.91 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj (in 3.42 sec).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.iOS.Tests/Controls.TestCases.iOS.Tests.csproj (in 3.44 sec).
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Controls.CustomAttributes -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.CustomAttributes/Debug/net10.0/Controls.CustomAttributes.dll
  Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  UITest.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Core/Debug/net10.0/UITest.Core.dll
  VisualTestUtils -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/VisualTestUtils/Debug/netstandard2.0/VisualTestUtils.dll
  UITest.NUnit -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.NUnit/Debug/net10.0/UITest.NUnit.dll
  VisualTestUtils.MagickNet -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/VisualTestUtils.MagickNet/Debug/netstandard2.0/VisualTestUtils.MagickNet.dll
  UITest.Appium -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Appium/Debug/net10.0/UITest.Appium.dll
  UITest.Analyzers -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Analyzers/Debug/netstandard2.0/UITest.Analyzers.dll
  Controls.TestCases.iOS.Tests -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Debug/net10.0/Controls.TestCases.iOS.Tests.dll
Test run for /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Debug/net10.0/Controls.TestCases.iOS.Tests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (arm64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
/Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Debug/net10.0/Controls.TestCases.iOS.Tests.dll
[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.05]   Discovering: Controls.TestCases.iOS.Tests
[xUnit.net 00:00:00.15]   Discovered:  Controls.TestCases.iOS.Tests
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Debug/net10.0/Controls.TestCases.iOS.Tests.dll
   NUnit3TestExecutor discovered 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 4/17/2026 2:27:16 AM FixtureSetup for Issue34662(iOS)
>>>>> 4/17/2026 2:27:19 AM ShellCurrentStateLocationCorrectAfterAbsoluteNavigation Start
>>>>> 4/17/2026 2:27:25 AM ShellCurrentStateLocationCorrectAfterAbsoluteNavigation Stop
>>>>> 4/17/2026 2:27:25 AM Log types: syslog, crashlog, performance, safariConsole, safariNetwork, server
  Failed ShellCurrentStateLocationCorrectAfterAbsoluteNavigation [6 s]
  Error Message:
     Shell.CurrentState.Location inside OnNavigated should be '//DashboardPage/Page1/Page2', not stale '//DashboardPage'
Assert.That(currentStateText, Is.EqualTo(expectedCurrentState))
  Expected string length 27 but was 15. Strings differ at index 15.
  Expected: "//DashboardPage/Page1/Page2"
  But was:  "//DashboardPage"
  --------------------------^

  Stack Trace:
     at Microsoft.Maui.TestCases.Tests.Issues.Issue34662.ShellCurrentStateLocationCorrectAfterAbsoluteNavigation() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34662.cs:line 24

1)    at Microsoft.Maui.TestCases.Tests.Issues.Issue34662.ShellCurrentStateLocationCorrectAfterAbsoluteNavigation() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34662.cs:line 24


NUnit Adapter 4.5.0.0: Test execution complete

Total tests: 1
     Failed: 1
Test Run Failed.
 Total time: 1.1529 Minutes

🟢 With fix — 🖥️ Issue34662: PASS ✅ · 91s
  Determining projects to restore...
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/BindingSourceGen/Controls.BindingSourceGen.csproj (in 349 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 361 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Essentials/src/Essentials.csproj (in 363 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 393 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Core/src/Core.csproj (in 408 ms).
  6 of 11 projects are up-to-date for restore.
/Users/cloudtest/vss/_work/1/s/.dotnet/packs/Microsoft.iOS.Sdk.net10.0_26.0/26.0.11017/targets/Xamarin.Shared.Sdk.targets(309,3): warning : RuntimeIdentifier was set on the command line, and will override the value for RuntimeIdentifiers set in the project file. [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-ios]
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0-ios26.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0-ios26.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0-ios26.0/Microsoft.Maui.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Maps/Debug/net10.0-ios26.0/Microsoft.Maui.Maps.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Microsoft.AspNetCore.Components.WebView.Maui -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-ios26.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
  Controls.Xaml -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Xaml.dll
  Controls.Foldable -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Foldable.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Controls.Maps -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-ios26.0/Microsoft.Maui.Controls.Maps.dll
  Detected signing identity:
    Code Signing Key: "" (-)
    Provisioning Profile: "" () - no entitlements
    Bundle Id: com.microsoft.maui.uitests
    App Id: com.microsoft.maui.uitests
  Controls.TestCases.HostApp -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-ios/iossimulator-arm64/Controls.TestCases.HostApp.dll
  Optimizing assemblies for size may change the behavior of the app. Be sure to test after publishing. See: https://aka.ms/dotnet-illink
  Optimizing assemblies for size. This process might take a while.

Build succeeded.

/Users/cloudtest/vss/_work/1/s/.dotnet/packs/Microsoft.iOS.Sdk.net10.0_26.0/26.0.11017/targets/Xamarin.Shared.Sdk.targets(309,3): warning : RuntimeIdentifier was set on the command line, and will override the value for RuntimeIdentifiers set in the project file. [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-ios]
    1 Warning(s)
    0 Error(s)

Time Elapsed 00:00:46.94
  Determining projects to restore...
  Restored /Users/cloudtest/vss/_work/1/s/src/Graphics/src/Graphics/Graphics.csproj (in 319 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/BindingSourceGen/Controls.BindingSourceGen.csproj (in 317 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Essentials/src/Essentials.csproj (in 325 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Core/src/Core.csproj (in 345 ms).
  Restored /Users/cloudtest/vss/_work/1/s/src/Controls/src/Core/Controls.Core.csproj (in 356 ms).
  8 of 13 projects are up-to-date for restore.
  Controls.CustomAttributes -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.CustomAttributes/Debug/net10.0/Controls.CustomAttributes.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Graphics -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Essentials -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
  Controls.BindingSourceGen -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.70-ci+azdo.13867187
  Controls.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
  UITest.Core -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Core/Debug/net10.0/UITest.Core.dll
  VisualTestUtils -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/VisualTestUtils/Debug/netstandard2.0/VisualTestUtils.dll
  UITest.NUnit -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.NUnit/Debug/net10.0/UITest.NUnit.dll
  UITest.Appium -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Appium/Debug/net10.0/UITest.Appium.dll
  VisualTestUtils.MagickNet -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/VisualTestUtils.MagickNet/Debug/netstandard2.0/VisualTestUtils.MagickNet.dll
  UITest.Analyzers -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/UITest.Analyzers/Debug/netstandard2.0/UITest.Analyzers.dll
  Controls.TestCases.iOS.Tests -> /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Debug/net10.0/Controls.TestCases.iOS.Tests.dll
Test run for /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Debug/net10.0/Controls.TestCases.iOS.Tests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (arm64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
/Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Debug/net10.0/Controls.TestCases.iOS.Tests.dll
[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.04]   Discovering: Controls.TestCases.iOS.Tests
[xUnit.net 00:00:00.14]   Discovered:  Controls.TestCases.iOS.Tests
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Debug/net10.0/Controls.TestCases.iOS.Tests.dll
   NUnit3TestExecutor discovered 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 4/17/2026 2:28:52 AM FixtureSetup for Issue34662(iOS)
>>>>> 4/17/2026 2:28:56 AM ShellCurrentStateLocationCorrectAfterAbsoluteNavigation Start
>>>>> 4/17/2026 2:28:57 AM ShellCurrentStateLocationCorrectAfterAbsoluteNavigation Stop
  Passed ShellCurrentStateLocationCorrectAfterAbsoluteNavigation [1 s]
NUnit Adapter 4.5.0.0: Test execution complete

Test Run Successful.
Total tests: 1
     Passed: 1
 Total time: 17.1641 Seconds

📁 Fix files reverted (1 files)
  • src/Controls/src/Core/Shell/ShellNavigationManager.cs

@MauiBot
Copy link
Copy Markdown
Collaborator

MauiBot commented Apr 17, 2026

🤖 AI Summary

👋 @Vignesh-SF3580 — new AI review results are available. Please review the latest session below.

📊 Review Sessiond9e4e74 · Update Issue34662.cs · 2026-04-17 10:26 UTC
🔍 Pre-Flight — Context & Validation

Issue: #34662 - Shell.OnNavigated not called for route navigation
PR: #34880 - [iOS, Mac] Fix Shell.CurrentState.Location stale in OnNavigated after GoToAsync
Platforms Affected: iOS, macCatalyst
Files Changed: 1 implementation (ShellNavigationManager.cs), 2 test (Issue34662.cs x2)

Key Findings

  • Regression from PR [iOS] Fixed Shell Navigating event showing same current and target values #25749 (fix for OnNavigating wrong target when tapping the same tab #25599). The #if IOS || MACCATALYST block in GetNavigationState was too broad — it executed in both UpdateCurrentState (should never truncate) and ProposeNavigationOutsideGotoAsync (correct use).
  • After GoToAsync("//DashboardPage/Page1/Page2"), UpdateCurrentState called GetNavigationState which incorrectly matched and truncated the route to //DashboardPage, causing stale Shell.CurrentState in OnNavigated.
  • Fix: adds bool isNavigateThroughTab = false optional parameter. Only ProposeNavigationOutsideGotoAsync passes true; all other 7 callers use false.
  • Prior agent review (kubaflo, LGTM/high) and fresh code review (NEEDS_DISCUSSION/high) both confirm the fix is architecturally correct.
  • Two Windows CI checks failing (Helix Unit Tests Debug, DeviceTests Windows) — likely pre-existing flakes given the change is iOS/macCatalyst-only inside #if block.
  • Inline Copilot suggestion on WaitForTextToBePresentInElement was addressed by the author (return value now properly used + WaitForTextToBePresentInElement before the assertion).

Code Review Summary

Verdict: NEEDS_DISCUSSION
Confidence: high
Errors: 0 | Warnings: 2 | Suggestions: 2

Key code review findings:

  • ⚠️ Required CI checks failing (Windows Helix Unit Tests Debug, DeviceTests Windows) — likely pre-existing flakes, but should be confirmed via retry before merge
  • ⚠️ WaitForTextToBePresentInElement return value discarded — test still catches the bug via subsequent Assert but less self-documenting (TestCases.Shared.Tests/Tests/Issues/Issue34662.cs:21)
  • 💡 Redundant null-conditional after is not null guard in ShellNavigationManager.cs:599 (pre-existing style, not introduced by PR)
  • 💡 Missing XML doc <param> on isNavigateThroughTab to explain its contract

Fix Candidates

# Source Approach Test Result Files Changed Notes
PR PR #34880 Add bool isNavigateThroughTab = false param to GetNavigationState; guard iOS trimming block behind it; only ProposeNavigationOutsideGotoAsync passes true ✅ PASSED (Gate) ShellNavigationManager.cs Original PR

🔬 Code Review — Deep Analysis

Code Review — PR #34880

Independent Assessment

What this changes: GetNavigationState() gains an optional bool isNavigateThroughTab = false parameter. The #if IOS || MACCATALYST block that truncates the route stack to the tab root (previously unconditional on those platforms) is now gated on isNavigateThroughTab. Only ProposeNavigationOutsideGotoAsync passes true; all other 6+ call sites use the default false.

Inferred motivation: A prior fix for a tab re-tap scenario (iOS "Navigating" event showing identical Current and Target) was applied too broadly — it also ran inside UpdateCurrentState, which sets Shell.CurrentState after GoToAsync deep navigation. This caused Shell.CurrentState to be silently truncated (e.g., //DashboardPage/Page1/Page2//DashboardPage) before OnNavigated fired.

Is the approach sound? Yes. The boolean flag is the minimal surgical change. All 6 non-ProposeNavigationOutsideGotoAsync call sites in Shell.cs, ShellSection.cs, and ShellNavigationManager.cs are correctly left with isNavigateThroughTab: false (verified by inspection). A more complex pattern (enum, callback) would be overkill for two clearly distinct contexts.


Reconciliation with PR Narrative

Author claims: The #if IOS || MACCATALYST block introduced in PR #25749 executed in both UpdateCurrentState and ProposeNavigationOutsideGotoAsync, causing stale Shell.CurrentState in OnNavigated after GoToAsync deep navigation.

Agreement: The root cause analysis matches the code exactly. UpdateCurrentState in Shell.cs calls GetNavigationState without the new flag (confirmed at line 1041), so it now correctly skips the truncation. The comment in the #if block accurately describes both the original bug and the constraint being applied.


Findings

⚠️ Warning — Required CI checks are failing

Two required Azure DevOps checks have failed:

  • maui-pr (Run Helix Unit Tests Windows Helix Unit Tests (Debug)) — 2m8s failure
  • maui-pr-devicetests (net10.0 Windows Helix Tests Run DeviceTests Windows) — 27m37s failure

The code changes are entirely inside #if IOS || MACCATALYST blocks plus the optional parameter (which is backward-compatible and takes the false default on all platforms including Windows). These failures are almost certainly pre-existing Windows Helix flakes, not caused by this PR — but they are required checks and should be retried or confirmed green before merge. The activation failure is an infrastructure issue (missing COPILOT_GITHUB_TOKEN secret for the gh-aw workflow), unrelated to the code.

⚠️ Warning — WaitForTextToBePresentInElement return value not asserted

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34662.cs, line 21:

App.WaitForTextToBePresentInElement("OnNavigatedCurrentStateLabel", expectedCurrentState, TimeSpan.FromSeconds(5));

The method returns bool but the return value is discarded. If the expected text does not appear within 5 seconds (i.e., the bug is present), this call silently returns false and execution continues to the Assert.That. The Assert.That will still catch the failure, so the test is functionally correct — but the test would be more self-documenting and precise with:

Assert.That(
    App.WaitForTextToBePresentInElement("OnNavigatedCurrentStateLabel", expectedCurrentState, TimeSpan.FromSeconds(5)),
    Is.True,
    "Timed out waiting for CurrentState label to show expected route");

💡 Suggestion — Redundant null-conditional after is not null guard

ShellNavigationManager.cs, line 599:

if (isNavigateThroughTab && Shell.Current?.CurrentState?.Location is not null)
{
    var currentRoute = Shell.Current?.CurrentState?.Location?.ToString();

The ?. on Location inside the if body is redundant — the guard already proved Location is not null. This is pre-existing style (not introduced by this PR), flagging for completeness.

💡 Suggestion — Consider XML doc <param> on isNavigateThroughTab

The parameter name is descriptive but a brief <param> doc would make the contract explicit for future contributors:

/// <param name="isNavigateThroughTab">When true, applies iOS-specific route deduplication
/// for the Navigating event during native tab taps (#25599). Must be false for
/// UpdateCurrentState to prevent stale Shell.CurrentState (#34662).</param>

Devil's Advocate

"Could another call site also need isNavigateThroughTab: true?"
All 7 call sites verified: the only one that generates a proposed navigation state for the Navigating event (where the iOS tab-retap behavior is relevant) is ProposeNavigationOutsideGotoAsync. All others compute actual navigation state for Shell.CurrentState, ShellNavigationParameters, or navigation proxies. Passing false everywhere else is correct.

"Could the currentPaths.Count == routeStack.Count condition in the #if block still be wrong?"
That logic is unchanged from PR #25749 and is only reached when isNavigateThroughTab: true. The PR's fix doesn't touch that logic — it only prevents it from running outside its intended context.

"Is the test proving the fix, or would it pass even with the bug?"
The test navigates to //DashboardPage/Page1/Page2 and checks that Shell.CurrentState.Location inside OnNavigated equals the full path. Without this fix, on iOS, UpdateCurrentState would truncate the route to //DashboardPage, so the label would show //DashboardPage and Assert.That would fail. The test genuinely catches the regression.


Verdict: NEEDS_DISCUSSION

Confidence: high
Summary: The code change is architecturally sound, correctly scoped, and well-commented. The root cause analysis in the PR description matches the implementation exactly. The test covers the regression meaningfully. The blocking issue before merge is the two failing Windows CI checks (Helix Unit Tests (Debug) and DeviceTests Windows) — they look like pre-existing flakes given the change is iOS/macCatalyst-only, but a human reviewer should confirm via a CI retry before approving.


🔧 Fix — Analysis & Comparison

Fix Candidates

# Source Approach Test Result Files Changed Notes
1 try-fix (claude-opus-4.6) Move iOS tab-trimming block out of GetNavigationState entirely; relocate into ProposeNavigationOutsideGotoAsync after calling GetNavigationState, operating on returned state ✅ PASS ShellNavigationManager.cs Keeps GetNavigationState as pure function; no API signature change
2 try-fix (claude-sonnet-4.6) Guard iOS trimming via AccumulateNavigatedEvents check inside GetNavigationState — skips trimming when GoToAsync is in progress ✅ PASS ShellNavigationManager.cs No API signature change; checks runtime state instead
3 try-fix (gpt-5.3-codex) Scoped AsyncLocal<int> depth counter in ShellNavigationManager; increment in ProposeNavigationOutsideGotoAsync around GetNavigationState call; trimming fires only when counter > 0 ✅ PASS ShellNavigationManager.cs No parameter change, no code relocation; uses scoped context
4 try-fix (gpt-5.4) Validate trimmed route against visible stack before keeping it — fall back to untrimmed if trimming drops the visible destination page ✅ PASS ShellNavigationManager.cs No call-site changes; treats trimming as optional normalization
PR PR #34880 Add bool isNavigateThroughTab = false param to GetNavigationState; guard iOS trimming block behind it; only ProposeNavigationOutsideGotoAsync passes true ✅ PASSED (Gate) ShellNavigationManager.cs Original PR; surgical, well-commented

Cross-Pollination

Model Round New Ideas? Details
claude-opus-4.6 2 Yes Capture target state before navigation in GoToAsync, bypass GetNavigationState for post-nav state update
claude-sonnet-4.6 2 Yes Fix in iOS TabBar platform handler — snapshot stack before re-tap, remove trimming hack entirely
gpt-5.3-codex 2 Yes Track pendingTargetLocation in ShellNavigationManager; prefer during in-flight navigation
gpt-5.4 2 Yes Snapshot pre-selection ShellSection.Stack at platform layer; pass explicit "tab reselect" intent
All models 3 New ideas are significantly more complex — no improvement over existing passing candidates

Exhausted: Yes — all new ideas are more complex than existing passing candidates; no further exploration warranted.
Selected Fix: PR's fix — Clean, minimal, surgical. Well-documented with comment explaining the #25599/#34662 relationship. isNavigateThroughTab parameter explicitly signals intent at call site. All 4 alternative approaches also passed, but the PR's fix is the simplest and passed Gate verification.


📋 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase Status Notes
Pre-Flight ✅ COMPLETE Issue #34662, 1 impl file + 2 test files, iOS/macCatalyst
Code Review NEEDS_DISCUSSION (high) 0 errors, 2 warnings, 2 suggestions
Gate ✅ PASSED ios
Try-Fix ✅ COMPLETE 4 attempts, 4 passing
Report ✅ COMPLETE

Code Review Impact on Try-Fix

The code review found no ❌ Errors — all 4 models started from a clean code-correctness baseline and focused on exploring alternative design approaches. The ⚠️ Warning about WaitForTextToBePresentInElement return value was noted by models but didn't change fix strategies (test logic is separate from fix logic). The NEEDS_DISCUSSION verdict was driven entirely by failing CI checks, not code issues — so models were free to explore pure alternative fix strategies. The cross-pollination round generated more complex ideas (platform-layer snapshots, pending-location tracking) that all exceeded the simplicity of the PR's own approach.

Summary

PR #34880 fixes a regression (from PR #25749) where Shell.CurrentState.Location was stale inside Shell.OnNavigated after GoToAsync deep navigation on iOS/macCatalyst. The fix is surgically correct and all 4 independent fix attempts confirm the PR's approach is sound. The blocker for merge is two failing required Windows CI checks. Based on the change scope (iOS/macCatalyst-only, inside #if block), these are almost certainly pre-existing flakes — but they must be confirmed via a CI retry before approval.

Root Cause

GetNavigationState() had a #if IOS || MACCATALYST block (added by PR #25749 to fix tab re-tap stale Navigating event, #25599) that unconditionally trimmed the route stack when the current shell state matched. This block fired in both ProposeNavigationOutsideGotoAsync (intended) and UpdateCurrentState (unintended). After GoToAsync("//DashboardPage/Page1/Page2"), UpdateCurrentState called GetNavigationState, the iOS block matched and truncated the route to //DashboardPage, and this stale value was set as Shell.CurrentState before OnNavigated fired.

Fix Quality

The PR's fix is minimal and correct (+8/-3 lines in one file). The isNavigateThroughTab = false parameter is semantically clear, backward-compatible (optional with default), and the inline comment accurately documents the #25599/#34662 relationship. All 4 independent Try-Fix models also passed tests with their own approaches, confirming the root cause diagnosis is correct and fixable. Among all 5 passing candidates (4 alternatives + PR), the PR's fix is the simplest. The test is well-constructed and genuinely catches the regression.

Action required before merge:

  1. Retry or verify the two failing Windows CI checks (maui-pr: Helix Unit Tests (Debug), maui-pr-devicetests: DeviceTests Windows) — these are required checks and must be green. Given the change is entirely inside #if IOS || MACCATALYST, a CI retry should be sufficient if they are flakes.
  2. Consider asserting the return value of WaitForTextToBePresentInElement (⚠️ warning) — minor quality improvement, not a blocker.

@MauiBot MauiBot added s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates and removed s/agent-review-incomplete labels Apr 17, 2026
@kubaflo kubaflo changed the base branch from main to inflight/current April 17, 2026 15:24
@kubaflo kubaflo merged commit 3baee59 into dotnet:inflight/current Apr 17, 2026
163 of 169 checks passed
PureWeen pushed a commit that referenced this pull request Apr 22, 2026
… GoToAsync (#34880)

> [!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!

### Issue Details
After calling GoToAsync("//DashboardPage/Page1/Page2") on iOS,
Shell.CurrentState.Location inside Shell.OnNavigated incorrectly reports
//DashboardPage instead of the full route //DashboardPage/Page1/Page2.

### Regression Details
Introduced by PR #25749 (fix for #25599). The #if IOS || MACCATALYST
block added to GetNavigationState was too broad—it executed in both
UpdateCurrentState and ProposeNavigationOutsideGotoAsync, causing
Shell.CurrentState to be truncated before OnNavigated was triggered.

### Root Cause
GetNavigationState is called from two key paths:
**ProposeNavigationOutsideGotoAsync** — Generates the proposed state for
the Navigating event during native tab taps. This is the intended scope
of the #25599 fix.
**UpdateCurrentState** — Updates Shell.CurrentState after navigation
completes and triggers OnNavigated. The iOS-specific trimming logic
should not execute here.
When navigating to //DashboardPage/Page1/Page2 on iOS,
UpdateCurrentState invokes GetNavigationState, and the iOS logic
incorrectly detects a match and truncates the route to //DashboardPage.
This truncated value is then set as Shell.CurrentState before
OnNavigated is fired.
 
### Description of Change
Added an optional bool isNavigateThroughTab = false parameter to
GetNavigationState and used it to guard the #if IOS || MACCATALYST
route-trimming logic. Only ProposeNavigationOutsideGotoAsync—the
specific call site where the #25599 fix is required—passes
isNavigateThroughTab: true. All other callers (UpdateCurrentState,
IShellController.GetNavigationState, etc.) use the default value
(false), ensuring Shell.CurrentState is always set with the correct,
untruncated route.

### Issues Fixed

Fixes #34662

### Screenshots

| Before Issue Fix | After Issue Fix |
|----------|----------|
| <video width="300" height="600"
src="https://github.com/user-attachments/assets/030f345f-ddff-4cab-a0f2-c60732f2496d">
| <video width="300" height="600"
src="https://github.com/user-attachments/assets/b8a68520-1285-4da4-8612-7405dec0e6b0">
|
PureWeen pushed a commit that referenced this pull request Apr 28, 2026
… GoToAsync (#34880)

> [!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!

### Issue Details
After calling GoToAsync("//DashboardPage/Page1/Page2") on iOS,
Shell.CurrentState.Location inside Shell.OnNavigated incorrectly reports
//DashboardPage instead of the full route //DashboardPage/Page1/Page2.

### Regression Details
Introduced by PR #25749 (fix for #25599). The #if IOS || MACCATALYST
block added to GetNavigationState was too broad—it executed in both
UpdateCurrentState and ProposeNavigationOutsideGotoAsync, causing
Shell.CurrentState to be truncated before OnNavigated was triggered.

### Root Cause
GetNavigationState is called from two key paths:
**ProposeNavigationOutsideGotoAsync** — Generates the proposed state for
the Navigating event during native tab taps. This is the intended scope
of the #25599 fix.
**UpdateCurrentState** — Updates Shell.CurrentState after navigation
completes and triggers OnNavigated. The iOS-specific trimming logic
should not execute here.
When navigating to //DashboardPage/Page1/Page2 on iOS,
UpdateCurrentState invokes GetNavigationState, and the iOS logic
incorrectly detects a match and truncates the route to //DashboardPage.
This truncated value is then set as Shell.CurrentState before
OnNavigated is fired.
 
### Description of Change
Added an optional bool isNavigateThroughTab = false parameter to
GetNavigationState and used it to guard the #if IOS || MACCATALYST
route-trimming logic. Only ProposeNavigationOutsideGotoAsync—the
specific call site where the #25599 fix is required—passes
isNavigateThroughTab: true. All other callers (UpdateCurrentState,
IShellController.GetNavigationState, etc.) use the default value
(false), ensuring Shell.CurrentState is always set with the correct,
untruncated route.

### Issues Fixed

Fixes #34662

### Screenshots

| Before Issue Fix | After Issue Fix |
|----------|----------|
| <video width="300" height="600"
src="https://github.com/user-attachments/assets/030f345f-ddff-4cab-a0f2-c60732f2496d">
| <video width="300" height="600"
src="https://github.com/user-attachments/assets/b8a68520-1285-4da4-8612-7405dec0e6b0">
|
PureWeen pushed a commit that referenced this pull request Apr 29, 2026
… GoToAsync (#34880)

> [!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!

### Issue Details
After calling GoToAsync("//DashboardPage/Page1/Page2") on iOS,
Shell.CurrentState.Location inside Shell.OnNavigated incorrectly reports
//DashboardPage instead of the full route //DashboardPage/Page1/Page2.

### Regression Details
Introduced by PR #25749 (fix for #25599). The #if IOS || MACCATALYST
block added to GetNavigationState was too broad—it executed in both
UpdateCurrentState and ProposeNavigationOutsideGotoAsync, causing
Shell.CurrentState to be truncated before OnNavigated was triggered.

### Root Cause
GetNavigationState is called from two key paths:
**ProposeNavigationOutsideGotoAsync** — Generates the proposed state for
the Navigating event during native tab taps. This is the intended scope
of the #25599 fix.
**UpdateCurrentState** — Updates Shell.CurrentState after navigation
completes and triggers OnNavigated. The iOS-specific trimming logic
should not execute here.
When navigating to //DashboardPage/Page1/Page2 on iOS,
UpdateCurrentState invokes GetNavigationState, and the iOS logic
incorrectly detects a match and truncates the route to //DashboardPage.
This truncated value is then set as Shell.CurrentState before
OnNavigated is fired.
 
### Description of Change
Added an optional bool isNavigateThroughTab = false parameter to
GetNavigationState and used it to guard the #if IOS || MACCATALYST
route-trimming logic. Only ProposeNavigationOutsideGotoAsync—the
specific call site where the #25599 fix is required—passes
isNavigateThroughTab: true. All other callers (UpdateCurrentState,
IShellController.GetNavigationState, etc.) use the default value
(false), ensuring Shell.CurrentState is always set with the correct,
untruncated route.

### Issues Fixed

Fixes #34662

### Screenshots

| Before Issue Fix | After Issue Fix |
|----------|----------|
| <video width="300" height="600"
src="https://github.com/user-attachments/assets/030f345f-ddff-4cab-a0f2-c60732f2496d">
| <video width="300" height="600"
src="https://github.com/user-attachments/assets/b8a68520-1285-4da4-8612-7405dec0e6b0">
|
github-actions Bot pushed a commit that referenced this pull request May 6, 2026
… GoToAsync (#34880)

> [!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!

### Issue Details
After calling GoToAsync("//DashboardPage/Page1/Page2") on iOS,
Shell.CurrentState.Location inside Shell.OnNavigated incorrectly reports
//DashboardPage instead of the full route //DashboardPage/Page1/Page2.

### Regression Details
Introduced by PR #25749 (fix for #25599). The #if IOS || MACCATALYST
block added to GetNavigationState was too broad—it executed in both
UpdateCurrentState and ProposeNavigationOutsideGotoAsync, causing
Shell.CurrentState to be truncated before OnNavigated was triggered.

### Root Cause
GetNavigationState is called from two key paths:
**ProposeNavigationOutsideGotoAsync** — Generates the proposed state for
the Navigating event during native tab taps. This is the intended scope
of the #25599 fix.
**UpdateCurrentState** — Updates Shell.CurrentState after navigation
completes and triggers OnNavigated. The iOS-specific trimming logic
should not execute here.
When navigating to //DashboardPage/Page1/Page2 on iOS,
UpdateCurrentState invokes GetNavigationState, and the iOS logic
incorrectly detects a match and truncates the route to //DashboardPage.
This truncated value is then set as Shell.CurrentState before
OnNavigated is fired.
 
### Description of Change
Added an optional bool isNavigateThroughTab = false parameter to
GetNavigationState and used it to guard the #if IOS || MACCATALYST
route-trimming logic. Only ProposeNavigationOutsideGotoAsync—the
specific call site where the #25599 fix is required—passes
isNavigateThroughTab: true. All other callers (UpdateCurrentState,
IShellController.GetNavigationState, etc.) use the default value
(false), ensuring Shell.CurrentState is always set with the correct,
untruncated route.

### Issues Fixed

Fixes #34662

### Screenshots

| Before Issue Fix | After Issue Fix |
|----------|----------|
| <video width="300" height="600"
src="https://github.com/user-attachments/assets/030f345f-ddff-4cab-a0f2-c60732f2496d">
| <video width="300" height="600"
src="https://github.com/user-attachments/assets/b8a68520-1285-4da4-8612-7405dec0e6b0">
|
@github-actions github-actions Bot locked and limited conversation to collaborators May 18, 2026
@kubaflo kubaflo added the s/agent-gate-passed AI verified tests catch the bug (fail without fix, pass with fix) label May 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-controls-shell Shell Navigation, Routes, Tabs, Flyout community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration platform/ios s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates s/agent-gate-passed AI verified tests catch the bug (fail without fix, pass with fix) s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Shell.OnNavigated not called for route navigation

7 participants