Skip to content

[iOS/Mac Catalyst] Add native element registrations for DevFlow - #36821

Open
morning4coffe-dev wants to merge 13 commits into
dotnet:mainfrom
morning4coffe-dev:feature/devflow-native-registration
Open

[iOS/Mac Catalyst] Add native element registrations for DevFlow#36821
morning4coffe-dev wants to merge 13 commits into
dotnet:mainfrom
morning4coffe-dev:feature/devflow-native-registration

Conversation

@morning4coffe-dev

@morning4coffe-dev morning4coffe-dev commented Jul 27, 2026

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!

Why

DevFlow can inspect MAUI controls, but Apple-owned UI such as navigation bars, toolbar items, tab overflow, Shell flyout rows, and native dialogs does not exist in the MAUI visual tree. That leaves useful parts of an app invisible to inspection and automation.

This PR now contains the Apple half of the native-registration work plus the shared diagnostics contract and tests. The Android half is in dotnet/maui#36835, and the companion DevFlow implementation consumes these registrations in dotnet/maui-labs#396. The shared diagnostics files intentionally match the Android PR so each platform can be reviewed independently.

What changed

  • Adds versioned Registered and Unregistered diagnostic events for native elements.
  • Adds a registration set that preserves stable registrations across refreshes and keeps registration/cleanup symmetrical.
  • Registers iOS and Mac Catalyst navigation bars, Shell and TabbedPage tabs/More rows, toolbar and back-button surfaces, search controls, Shell flyout rows, alerts, prompts, and action sheets.
  • Exposes logical dialog actions when UIKit does not provide a stable public view for a system-owned action.
  • Reconciles dynamic tab and alert-action views after layout without retaining stale native controls.
  • Preserves existing registrations while asynchronous icons load and keeps custom toolbar controls correctly sized and tinted.
  • Keeps the contract internal and inactive unless a diagnostics listener is present.
  • Replays cached native metadata when a listener attaches later and avoids diagnostics-only tab traversal while observation is disabled.

The event payload carries the contract version, MAUI owner, native object, semantic role, and whether the object is a realized view or a logical model. DevFlow can therefore attach native nodes to the correct logical owner without MAUI taking a dependency on DevFlow.

Compatibility

There is no public API change. Registrations are explicitly cleaned up with their native lifecycle to avoid retaining handlers, pages, dialog callbacks, or UIKit objects.

Testing

  • NativeElementDiagnosticsTests: 14 passed
  • Controls.Core compiled for net10.0-ios26.0
  • Controls.Core compiled for net10.0-maccatalyst26.0
  • Apple validation covered iOS/Mac Catalyst registration and dialog lifecycles

Copilot AI lite review requested due to automatic review settings July 27, 2026 09:44
@github-actions

Copy link
Copy Markdown
Contributor

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

Or

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

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Hey there @@morning4coffe-dev! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Hey there @morning4coffe-dev! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@github-actions github-actions Bot added area-tooling XAML & C# Hot Reload, XAML Editor, Live Visual Tree, Live Preview, Debugging platform/android platform/ios platform/macos macOS / Mac Catalyst labels Jul 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds an internal diagnostics contract to publish platform-owned/native UI surfaces (toolbars, nav bars, tab overflow, flyouts, dialogs, etc.) alongside their MAUI “owner” so DevFlow can inspect/automate parts of the UI that aren’t in the MAUI visual tree.

Changes:

  • Introduces NativeElementDiagnostics (versioned DiagnosticListener events) and a NativeElementRegistrationSet helper to keep registrations/unregistrations symmetric.
  • Wires up native registrations across Android + iOS/Mac Catalyst Shell/Tabbed/Toolbar/navigation and alert/prompt/action-sheet lifecycles.
  • Adds unit tests validating the diagnostics lifecycle and NativeElementRegistrationSet behavior.

Reviewed changes

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

Show a summary per file
File Description
src/Core/src/Diagnostics/NativeElementDiagnostics.cs New internal DiagnosticListener contract for native element register/unregister events.
src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs New tests for versioned lifecycle events and registration set semantics.
src/Controls/src/Core/Toolbar/Toolbar.Android.cs Threads a NativeElementRegistrationSet through Android toolbar menu updates and clears on handler changes.
src/Controls/src/Core/Platform/Android/TabbedPageManager.cs Registers bottom tabs and “More” overflow rows/dialog; adds lifecycle cleanup hooks.
src/Controls/src/Core/Platform/Android/Extensions/ToolbarExtensions.cs Registers toolbar chrome + menu item logical/realized surfaces via registrations.
src/Controls/src/Core/Platform/Android/BottomNavigationViewUtils.cs Adds callbacks for menu/row creation to enable native element registration for tabs/overflow.
src/Controls/src/Core/Platform/AlertManager/AlertManager.iOS.cs Registers UIAlertController surfaces + logical/realized dialog actions with robust lifecycle cleanup.
src/Controls/src/Core/Platform/AlertManager/AlertManager.Android.cs Registers native dialog surfaces/actions and disposes registrations on dismiss.
src/Controls/src/Core/Diagnostics/NativeElementRegistrationSet.cs New helper for dedup/exclusive registrations, retain/clear, and role/discriminator constants.
src/Controls/src/Core/Compatibility/Handlers/TabbedPage/iOS/TabbedRenderer.cs Registers tab bar, tab item, visible tab controls, and “More” overflow rows with cleanup.
src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/UIContainerCell.cs Registers flyout cells as native elements and clears on disconnect.
src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellTableViewSource.cs Adds disconnect path to clear cached cells/registrations.
src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellTableViewController.cs Ensures source is disconnected/disposed cleanly during controller disposal.
src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRenderer.cs Registers navigation bar/tab bar items and clears registrations on disconnect.
src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs Registers toolbar items/overflow/back/flyout toggle/search surfaces with lifecycle guards.
src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellItemRenderer.cs Registers visible tab controls + “More” overflow rows; clears observers/registrations on disconnect.
src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellToolbarTracker.cs Registers nav button + search surfaces; clears per-navigation generation.
src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellSectionRenderer.cs Registers shell tab layout and realized tab views; clears on reset/destroy.
src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellItemRenderer.cs Registers bottom tab bar/menu items + “More” sheet dialog/rows; adds cleanup on resets/dismiss.
src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs Registers flyout row native elements per view holder; clears during recycling/disconnect.
src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs Registers navigation bar + toolbar overflow/logical items and clears on disconnect/dispose.
Comments suppressed due to low confidence (1)

src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRecyclerAdapter.cs:99

  • foreach (var item in _listItems ?? new List<AdapterListItem>()) allocates a new list every time _listItems is null (e.g., during Disconnect while RecyclerView is still recycling holders). Avoid the per-call allocation by guarding the loop instead.
				// only clear out the Element if the item has been removed
				bool found = false;
				foreach (var item in _listItems ?? new List<AdapterListItem>())
				{
					if (item.Element == evh.Element)
					{
						found = true;
						break;
					}
				}

Comment thread src/Controls/src/Core/Platform/Android/TabbedPageManager.cs Outdated
@kubaflo

This comment has been minimized.

@github-actions github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Jul 27, 2026

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we should split it into 2 PRs - iOS and Android? You can duplicate the shared files :)

@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 s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) labels Jul 27, 2026
@morning4coffe-dev

Copy link
Copy Markdown
Contributor Author

Thanks @kubaflo, good point. My initial thought was to keep it combined, as I did in the maui-labs part, but the changeset grew larger than expected. I’ll keep this PR focused on Apple plus the shared diagnostics/tests and open a separate Android PR for this.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: ac067ef9-3039-4f24-b3b3-f386f3824bbc
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: ac067ef9-3039-4f24-b3b3-f386f3824bbc
@kubaflo

This comment has been minimized.

@github-actions github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Aug 4, 2026
@MauiBot

MauiBot commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

AI Review Summary

@morning4coffe-dev — new AI review results are available based on this last commit: 18edcdd.

Gate Inconclusive Confidence Low Platform iOS


🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix

Gate Result: ⚠️ INCONCLUSIVE

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

🩺 Base branch does not compile — the without-fix build failed. The gate's "does the test fail without the fix" check is unreliable here; this usually means main is broken or a merge-base file went missing. Note: this PR ADDS 2 new file(s), which the gate removes to reconstruct the pre-fix baseline; if the PR's own (never-reverted) test files reference types defined in those new files, the baseline cannot compile — that reflects a new-feature PR the gate cannot isolate a "before" state for, not necessarily a broken main. The with-fix result below is the reliable signal. Investigate before trusting this gate.

/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(22,29): error CS0103: The name 'NativeElementDiagnostics' does not exist in the current context [/User...

Test Without Fix (expect FAIL) With Fix (expect PASS)
🧪 NativeElementDiagnosticsCollection NativeElementDiagnosticsCollection 🛠️ BUILD ERROR 🔍 NO MATCH
🔴 Without fix — 🧪 NativeElementDiagnosticsCollection: 🛠️ BUILD ERROR · 28s

Error-relevant lines (filtered from the build log):

/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(272,29): error CS0103: The name 'NativeElementDiagnostics' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(273,49): error CS0103: The name 'NativeElementRoles' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(277,17): error CS0103: The name 'NativeElementDiagnostics' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(287,29): error CS0103: The name 'NativeElementDiagnostics' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(288,34): error CS0246: The type or namespace name 'NativeElementRegistrationSet' could not be found (are you missing a using directive or an assembly reference?) [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(289,44): error CS0103: The name 'NativeElementRoles' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(290,43): error CS0103: The name 'NativeElementRoles' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(295,17): error CS0103: The name 'NativeElementDiagnostics' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(303,34): error CS0246: The type or namespace name 'NativeElementRegistrationSet' could not be found (are you missing a using directive or an assembly reference?) [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(317,29): error CS0103: The name 'NativeElementDiagnostics' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(318,34): error CS0246: The type or namespace name 'NativeElementRegistrationSet' could not be found (are you missing a using directive or an assembly reference?) [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(319,49): error CS0103: The name 'NativeElementRoles' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(330,17): error CS0103: The name 'NativeElementDiagnostics' does not exist in the current context [/Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
🟢 With fix — 🧪 NativeElementDiagnosticsCollection: 🔍 NO MATCH · 18s
  Determining projects to restore...
  All projects are up-to-date for restore.
Test run for /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.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.
[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.08]   Discovering: Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.54]   Discovered:  Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.55]   Starting:    Microsoft.Maui.Controls.Core.UnitTests
[xUnit.net 00:00:00.56]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
No test matches the given testcase filter `NativeElementDiagnosticsCollection` in /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.Core.UnitTests/Debug/net10.0/Microsoft.Maui.Controls.Core.UnitTests.dll

⚠️ Failure Details

  • 🛠️ NativeElementDiagnosticsCollection without fix: build failed before tests could run
    • /Users/cloudtest/vss/_work/1/s/src/Controls/tests/Core.UnitTests/NativeElementDiagnosticsTests.cs(22,29): error CS0103: The name 'NativeElementDiagnostics' does not exist in the current context [/User...
  • 🔍 NativeElementDiagnosticsCollection with fix: test filter matched 0 tests
    • filter: NativeElementDiagnosticsCollection
📁 Fix files reverted (9 files)
  • src/Controls/src/Core/Compatibility/Handlers/NavigationPage/iOS/NavigationRenderer.cs
  • src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellItemRenderer.cs
  • src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellPageRendererTracker.cs
  • src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellSectionRenderer.cs
  • src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellTableViewController.cs
  • src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/ShellTableViewSource.cs
  • src/Controls/src/Core/Compatibility/Handlers/Shell/iOS/UIContainerCell.cs
  • src/Controls/src/Core/Compatibility/Handlers/TabbedPage/iOS/TabbedRenderer.cs
  • src/Controls/src/Core/Platform/AlertManager/AlertManager.iOS.cs

New files (not reverted):

  • src/Controls/src/Core/Diagnostics/NativeElementRegistrationSet.cs
  • src/Core/src/Diagnostics/NativeElementDiagnostics.cs

📱 UI Tests — Shell,TabbedPage

Detected UI test categories: Shell,TabbedPage

Deep UI tests — 343 passed, 0 failed across 2 categories on platform-pool agent (replaces in-process counts above).

🧪 UI Test Execution Results (deep, platform pool)

Category Tests Snapshot diffs
Shell 297/312 ✓
TabbedPage 46/51 ✓
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)

📋 Pre-Flight — Context & Validation

Issue: #36821 - [iOS/Mac Catalyst] Add native element registrations for DevFlow
PR: #36821 - [iOS/Mac Catalyst] Add native element registrations for DevFlow
Platforms Affected: iOS, Mac Catalyst
Files Changed: 11 implementation, 1 test

Key Findings

  • PR adds an internal versioned native-element diagnostics contract plus iOS/Mac Catalyst registrations for navigation, Shell, tabbed pages, flyout rows, toolbar/search/dialog surfaces, and lifecycle cleanup.
  • The PR is a feature/instrumentation change rather than a conventional bug fix; the prior gate was inconclusive because removing newly added diagnostics files makes the newly added tests fail to compile in the reconstructed baseline.
  • Prior review discussion already raised listener-gating/performance concerns around alert action scanning; the current diff includes active-listener checks to avoid most diagnostics work when there is no listener.
  • Independent code review found a remaining reconnect/replay failure: a new diagnostics observer can miss stable native elements when a previous active token is still cached.

Code Review Summary

Verdict: NEEDS_CHANGES
Confidence: low
Errors: 1 | Warnings: 0 | Suggestions: 0

Key code review findings:

  • src/Controls/src/Core/Diagnostics/NativeElementRegistrationSet.cs:37-39 returns early for unchanged active registrations, so observer A disconnect → observer B attach → repeated register does not emit a Registered event to observer B.

Fix Candidates

# Source Approach Test Result Files Changed Notes
PR PR #36821 Cache registration metadata in NativeElementRegistrationSet, emit native diagnostics only while a listener is enabled, and promote disabled registrations when a listener attaches. ⚠️ INCONCLUSIVE (Gate) NativeElementDiagnostics.cs, NativeElementRegistrationSet.cs, iOS compatibility handlers, alert manager, tests Original PR; gate could not isolate a without-fix baseline for newly added diagnostics files.

🔬 Code Review — Deep Analysis

Code Review — PR #36821

Independent Assessment

What this changes: Adds an internal DiagnosticListener contract and iOS/Mac Catalyst registrations for UIKit-owned native UI surfaces: nav/tool bars, Shell/TabbedPage tabs and More rows, flyout rows, search, dialogs, and actions.
Inferred motivation: Make native-only UI discoverable to DevFlow without adding public API or taking a dependency on DevFlow.

Reconciliation with PR Narrative

Author claims: Internal, listener-gated native element registration with lifecycle cleanup and late-listener replay support.
Agreement/disagreement: Broadly matches, but one late-listener/reconnect case still misses registrations.

Prior Review Reconciliation

Prior ❌ Error Finding Source Status Evidence
Late diagnostics listeners could miss cached native registrations MauiBot / prior review ❌ Unresolved Current code fixes disabled→enabled promotion, but still skips re-emitting when an old active token remains and a new observer attaches.

Blast Radius Assessment

  • Runs for all instances: Mostly diagnostics-gated, but registration-set state is updated from common Shell/TabbedPage/layout paths.
  • Startup impact: Low; no startup execution beyond static listener availability.
  • Static/shared state: Yes — static DiagnosticListener; instance registration sets retain owner/native references until cleared.

CI Status

  • Required-check result: fail (gh unavailable unauthenticated; public check-runs show maui-pr failed)
  • Classification: Likely infra/MSBuild failure, but CI is red
  • Action taken: invoked azdo-build-investigator; failure is MSBuild NullReferenceException while loading Xaml.UnitTests.InternalsHiddenAssembly.csproj. Confidence capped low.

External Output Contract

Consumer token/pattern Producer location Producer emission condition Consumer assumption Ordinary negative case Downstream effect
N/A N/A No changed external-output classifiers found N/A N/A N/A

Findings

❌ Error — Reconnected diagnostics observers can miss existing native elements

NativeElementRegistrationSet.Register returns early when same owner/native/role metadata is already active and NativeElementDiagnostics.IsRegistrationEnabled is true (src/Controls/src/Core/Diagnostics/NativeElementRegistrationSet.cs:37-39). If observer A was attached when a native element registered, then observer A disconnects, observer B attaches, and a layout/refresh calls Register again, existing.IsActive == registrationEnabled remains true and no Registered event is emitted to observer B. DiagnosticListener does not replay old writes to new subscribers.

The tests cover active→disabled refresh→enabled replay (NativeElementDiagnosticsTests.cs:257-278), but not active observer replacement without an intermediate disabled refresh. This affects stable Shell/TabbedPage/native surfaces that rely on repeated layout registration calls.

Failure-Mode Probing

  • Late listener reconnect: missed, as described above.
  • Disabled then enabled before reconnect: covered by tests and works.
  • Handler disconnect/reconnect: registration sets are generally cleared on dispose/disconnect.
  • Async icon callbacks after page changes: generation guards prevent stale registrations.
  • Null/default state: most callers guard page/item/view state before registering.

Verdict: NEEDS_CHANGES

Confidence: low
Summary: The main design is sound, but diagnostics replay still fails for observer reconnects while an active token is cached. CI is also red, so this cannot be LGTM.


🛠️ Fix — Analysis & Comparison

Fix Candidates

# Source Approach Test Result Files Changed Notes
1 try-fix-1 Diagnostics-layer replay on observer subscription using a Core-level active-registration snapshot. ✅ PASS — NativeElementDiagnosticsTests 15/15 after compile fixes 2 files Fixes replacement observers immediately, but Core globally retains active native registration payloads, increasing lifecycle/memory risk.
2 try-fix-2 Observer activation epoch: Core tracks only registration-enabled observer epochs; registration sets re-emit stale active tokens on the next same-metadata refresh. ✅ PASS — NativeElementDiagnosticsTests 15/15 after compile/analyzer fixes 3 files Fixes the unresolved PR defect with less global state than Candidate 1. Selected.
PR PR #36821 Metadata cache plus disabled-to-enabled promotion in NativeElementRegistrationSet. ⚠️ INCONCLUSIVE (Gate) 12 files Original PR misses observer replacement when no disabled refresh occurs.

Cross-Pollination

Model Round New Ideas? Details
maui-expert-reviewer 1 Yes Proposed Candidate 1: replay active registrations from the diagnostics layer at observer subscription time.
maui-expert-reviewer 2 Yes After Candidate 1 passed but showed lifecycle risk, proposed Candidate 2: observer activation epoch scoped to registration sets.

Exhausted: No — stopped because Candidate 2 passes the targeted regression tests and is demonstrably better than both the PR's current enabled-state check and Candidate 1's global native snapshot.
Selected Fix: Candidate #2 — It covers the concrete observer-replacement failure, preserves existing duplicate-suppression within a single observer epoch, suppresses stale unregisters across observer epochs, and keeps native references scoped to existing Controls registration-set lifetimes.


📝 Recommended PR Title & Description

Assessment: ✏️ Recommend updating — the current metadata accurately describes the raw PR, but it does not mention the winning expert-feedback fix for replacement diagnostics observers.

Recommended title

[iOS/Mac Catalyst] DevFlow: Add native element registrations and observer replay

Recommended description

## Why

DevFlow can inspect MAUI controls, but Apple-owned UI such as navigation bars, toolbar items, tab overflow, Shell flyout rows, and native dialogs does not exist in the MAUI visual tree. That leaves useful parts of an app invisible to inspection and automation.

This PR now contains the Apple half of the native-registration work plus the shared diagnostics contract and tests. The Android half is in [dotnet/maui#36835](https://github.com/dotnet/maui/pull/36835), and the companion DevFlow implementation consumes these registrations in [dotnet/maui-labs#396](https://github.com/dotnet/maui-labs/pull/396). The shared diagnostics files intentionally match the Android PR so each platform can be reviewed independently.

## What changed

- Adds versioned `Registered` and `Unregistered` diagnostic events for native elements.
- Adds a registration set that preserves stable registrations across refreshes and keeps registration/cleanup symmetrical.
- Registers iOS and Mac Catalyst navigation bars, Shell and `TabbedPage` tabs/More rows, toolbar and back-button surfaces, search controls, Shell flyout rows, alerts, prompts, and action sheets.
- Exposes logical dialog actions when UIKit does not provide a stable public view for a system-owned action.
- Reconciles dynamic tab and alert-action views after layout without retaining stale native controls.
- Preserves existing registrations while asynchronous icons load and keeps custom toolbar controls correctly sized and tinted.
- Keeps the contract internal and inactive unless a diagnostics listener is present.
- Replays cached native metadata when a listener attaches later and avoids diagnostics-only tab traversal while observation is disabled.
- Re-emits stable active registrations for replacement diagnostics observers by tracking registration-observer activation epochs, without moving owner/native references into a global diagnostics snapshot.

The event payload carries the contract version, MAUI owner, native object, semantic role, and whether the object is a realized view or a logical model. DevFlow can therefore attach native nodes to the correct logical owner without MAUI taking a dependency on DevFlow.

## Compatibility

There is no public API change. Registrations are explicitly cleaned up with their native lifecycle to avoid retaining handlers, pages, dialog callbacks, or UIKit objects.

## Testing

- `NativeElementDiagnosticsTests`: 15 passed
- `Controls.Core` compiled for `net10.0-ios26.0`
- `Controls.Core` compiled for `net10.0-maccatalyst26.0`
- Apple validation covered iOS/Mac Catalyst registration and dialog lifecycles

🏁 Report — Final Recommendation

Comparative Report — PR #36821

Candidates

Rank Candidate Regression result Assessment
1 pr-plus-reviewer ✅ PASS by equivalence to try-fix-2 targeted diagnostics run Best candidate. Applies the expert review finding with observer-activation epochs scoped to existing registration sets, fixing replacement-observer replay without introducing global native-reference retention.
2 try-fix-2 ✅ PASS — NativeElementDiagnosticsTests 15/15 Same core fix as pr-plus-reviewer: epoch-aware duplicate suppression and stale-unregister suppression. Technically strong, but as an alternate try-fix candidate it is ranked behind the PR-derived candidate that applies the same feedback to the submitted fix.
3 try-fix-1 ✅ PASS — NativeElementDiagnosticsTests 15/15 Correctly replays active registrations to replacement observers, but does so by keeping a Core-level active-registration snapshot containing owner/native references. That broadens lifecycle and memory risk compared with the registration-set-scoped epoch design.
4 pr ⚠️ INCONCLUSIVE gate; targeted review found a concrete bug The raw PR implements the diagnostics contract and platform registrations, but the same-metadata fast path can skip Registered replay for a replacement observer. It ranks below passing candidates that fix that defect.

Key Comparison

The decisive issue is whether stable active native registrations are replayed after all previous registration-enabled observers disconnect and a new observer attaches. The raw PR only compares cached token active state with current listener-enabled state, so it can incorrectly suppress the next same-metadata registration. Both try-fix candidates address this, and both passed the focused diagnostics tests.

try-fix-1 solves replay at subscription time by storing active registrations in Core. That gives immediate replay, but it also makes Core globally retain owner/native payload metadata until disposal, increasing the risk profile for an internal diagnostics feature. try-fix-2 solves the same bug with scalar observer epoch state and leaves owner/native references scoped to the existing NativeElementRegistrationSet lifetimes introduced by the PR.

pr-plus-reviewer is the same winning design applied as expert feedback to the PR fix: keep the PR's architecture, add observer-epoch awareness to avoid the stale fast path, and suppress stale unregisters across observer epochs. This is the best balance of correctness, lifecycle safety, and minimal architectural expansion.

Winning Candidate

Winner: pr-plus-reviewer

Rationale: It fixes the expert-confirmed lifecycle bug while preserving the PR's registration-set ownership model and avoiding try-fix-1's global native-reference snapshot. Candidates with passing targeted regression tests rank above the raw PR, and among passing candidates the epoch-scoped design has the smallest lifecycle blast radius.


🧭 Next Steps — review latest findings

No alternative fix was selected for this run. Review the session findings and CI results before merging.

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Expert Review — 1 findings

See inline comments for details.

{
var registrationEnabled = NativeElementDiagnostics.IsRegistrationEnabled;
if (existing.IsActive == registrationEnabled)
return;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[major] Diagnostics lifecycle — This duplicate-registration fast path only compares whether a cached token is active with whether diagnostics are currently enabled, but it does not distinguish which observer saw that token. Concrete trace: observer A subscribes, Register emits and stores a non-null Token; observer A disposes while the native element stays registered; observer B subscribes; the next same-metadata Register sees existing.IsActive == registrationEnabled and returns here, so observer B never receives the required Registered event for that stable native element. Please invalidate/replay active tokens when the registration-enabled observer epoch changes (or equivalent) before suppressing the duplicate registration.

@kubaflo

kubaflo commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Empirical iOS/Android review: changes required

I tested commit 18edcdd4fa9 on an iPhone 16 Pro Max simulator running iOS 18.5 and Android API 36 (emulator-5556). I also ran targeted unit/build validation and had the recordings, screenshots, logs, and controls independently reviewed against the PR diff.

Verdict: do not merge this revision. The Apple registration coverage is broad and many lifecycle paths work correctly, but two high-severity correctness issues are reproducible.

Blocking runtime findings

  1. A diagnostics listener can make multi-section Shell enter an infinite navigation loop.
    With any observer subscribed to Microsoft.Maui.NativeElements, entering the six-section Shell causes GoToAsync("//a2") to never return while Shell oscillates among sections indefinitely. One run recorded 17,218 navigations and another 15,109.

    This was isolated three ways:

    • listener attached + normal probe: hangs;
    • listener detached: the full Shell/TabbedPage/NavigationPage scenario passes;
    • listener attached + a no-op observer that immediately returns: hangs.

    The sharpest control occurs in one process: Shell passes while detached, the listener is attached later, NavigationPage and TabbedPage still pass, and the next Shell entry hangs. Android completes the same full flow because this PR has no Android registration call sites.

  2. Observer handoff loses every existing registration.
    Observer A established 49 live registrations and was disposed without recreating native UI. Observer B subscribed, then duplicate refresh paths were triggered through tab switches and toolbar rebuilds:

    • B received 0/49 existing registrations;
    • after UI teardown, B received 49 Unregistered events for elements it had never seen registered.

    This empirically confirms the existing inline finding at NativeElementRegistrationSet.cs:38: existing.IsActive == registrationEnabled tracks only whether the cached token is active, not whether the current observer epoch saw it.

Additional finding

  • A 40-action sheet still allocates logical MenuItem/Command objects while diagnostics are disabled: about 252–255 KB of transient allocation per sheet. The listener guard is downstream of AddDialogAction, so the "skip diagnostics work without listeners" behavior is incomplete.

Controls and passing areas

  • Alerts, confirmation dialogs, action sheets, and prompts returned exactly to baseline after dismissal; no duplicate/rebind/orphan events.
  • NavigationPage and TabbedPage flows, toolbar rebuild churn, overflow activation, and listener detach/re-attach reconciliation passed outside the direct A→B handoff.
  • The overflow accessibility identifier produced exactly one visible automation element.
  • Controlled GC/heap comparisons did not show a PR-attributable owner-retention leak.
  • Android completed the full 52-step cross-platform flow twice, remained responsive, and emitted zero Apple registrations as expected.
  • Targeted validation passed: 14/14 diagnostics unit tests and Controls.Core builds for iOS and Mac Catalyst with zero warnings/errors.
  • Official maui-pr still has one unmatched macOS Debug MSBuild internal NullReferenceException; the same leg passed locally, but CI should be rerun.

Video evidence

iOS Shell liveness failure

01-ios-shell-liveness-hang.mp4

Observer handoff: 0/49 replayed, then 49 orphan unregistrations

02-ios-observer-handoff.mp4

Android full-flow control

03-android-smoke.mp4

Key frames

iOS runtime evidence

Android control evidence

Changes needed before merge

  • Remove the Shell registration/layout re-entrancy that feeds tab selection/navigation while diagnostics are active, and add an iOS device/UI regression test using a no-op observer.
  • Track listener/subscription generations so live registrations replay once to a replacement observer without emitting unmatched lifecycle events.
  • Avoid constructing logical alert action models unless native diagnostics are enabled.
  • Add tests for direct observer replacement, multi-section Shell with diagnostics enabled, and reconnect after native UI is already stable.

@kubaflo kubaflo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please check the latest suggestions :)

Copilot AI review requested due to automatic review settings August 26, 2026 13:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comment on lines +410 to +412
var title = GetActionViewTitle(actionView);
if (!actionsByTitle.ContainsKey(title))
continue;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-tooling XAML & C# Hot Reload, XAML Editor, Live Visual Tree, Live Preview, Debugging community ✨ Community Contribution platform/android platform/ios platform/macos macOS / Mac Catalyst 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-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.

4 participants