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

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34969"

Code Review — PR #34969

Independent Assessment

What this changes: Adds a null check for mauiMkMapView.Overlays in MauiMKMapView.OnMapClicked (iOS) before iterating the overlay collection for hit-testing. MKMapView.Overlays returns null (not an empty array) when no overlays exist. Also adds unit tests and an iOS device test.

Inferred motivation: Tapping a Map with no MapElements on iOS crashes with NullReferenceException because OnMapClicked iterates .Overlays without a null check.

Reconciliation with PR Narrative

Author claims: MKMapView.Overlays returns null when no overlays exist; OnMapClicked didn't guard against this. Introduced by PR #29101 (circle/polygon/polyline click events).

Agreement: Root cause matches exactly. The fix follows the existing ClearMapElements() pattern at line 139-142.

Findings

✅ Correct — Early return is safe; click event fires BEFORE overlay iteration

The method structure is:

1. Fire IMap.Clicked event (line 558)
2. Hit-test overlays for element-specific click (line 568+)

The null check and return at line 569-570 only skips the overlay hit-testing, which is correct — there are no overlays to test. The IMap.Clicked event has already been dispatched at line 558, so the MapClicked event still fires on maps without overlays.

✅ Correct — Fix follows existing codebase pattern

The null guard mirrors ClearMapElements() at line 139-142:

var elements = Overlays;
if (elements == null)
    return;

Storing in a local variable before null-checking is the correct iOS pattern — prevents potential TOCTOU if the native property could change between check and use.

✅ Correct — Device test exercises the actual crash path

MapTests.iOS.cs registers Map handlers, creates a Map with no elements, and invokes OnMapClicked via reflection. The author confirmed this test fails without the fix (NullReferenceException) and passes with it. This is proper regression test coverage.

✅ Correct — Unit tests provide supplementary managed-layer coverage

The two unit tests verify IMap.Clicked doesn't throw and fires MapClicked correctly at the control layer. These don't exercise the iOS crash path directly (as the copilot-reviewer correctly noted and the author acknowledged), but they validate the event contract.

⚠️ Warning — Device test uses reflection on private static method

OnMapClicked is accessed via BindingFlags.Static | BindingFlags.NonPublic. If the method is renamed or its signature changes, the test silently falls through to the else branch (creates a new recognizer and calls the method — which may still work but with different semantics). This is a known fragility, acceptable for testing a private method without changing its visibility.

💡 Suggestion — Maps project references added to Controls.DeviceTests

Adding Maps.csproj and Controls.Maps.csproj to the device tests project increases build scope. This is the right call — Maps has zero device test coverage today, and this establishes the foundation. Just worth noting for CI build time awareness.

CI Status

CI failures are infrastructure-level — SDK resolver errors (Microsoft.Build.NoTargets, Microsoft.DotNet.Arcade.Sdk not found) affecting all macOS builds on the net11.0 branch. Completely unrelated to this PR's code changes.

Devil's Advocate

1. Are there other places where .Overlays is iterated without null check? Only two references in the file: ClearMapElements() (already guarded) and OnMapClicked() (fixed here). No other unguarded access.

2. Could the return cause issues if future code is added after the loop? If someone adds code after the overlay foreach loop that should always run, the early return would skip it. But currently there's nothing after the loop — the method ends. The code is well-commented, reducing this risk.

3. Does this need to target main instead of net11.0? The bug was introduced by Add Circle, Polygon, and Polyline click events for Map control #29101 which landed on net11.0. The fix correctly targets the same branch. If the code was cherry-picked to main, a corresponding fix would be needed there too.

Verdict: LGTM

Confidence: high

Summary: Clean, minimal one-line null check fix for a genuine NullReferenceException crash. Follows the established pattern in the same file. The IMap.Clicked event correctly fires before the overlay iteration, so the early return doesn't suppress map tap events. Device test directly exercises the crash path. CI failures are infrastructure-level, not PR-related.

Waiting for #34978 so we can run the tests

/rebase

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