[iOS] Alert popup may be displayed on wrong window when modal page navigation is in progress - fix#31016
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an iOS-specific issue where alert popups could be displayed on wrong windows during modal page navigation, potentially causing deadlocks. The fix prevents alerts from being presented on view controllers that are currently being dismissed.
- Adds condition to skip view controllers that are being dismissed when finding the top view controller
- Includes comprehensive test coverage with both HostApp UI and automated test cases
- Targets iOS platform specifically where this navigation timing issue occurs
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Controls/src/Core/Platform/AlertManager/AlertManager.iOS.cs |
Core fix that checks IsBeingDismissed property before traversing to presented view controllers |
src/Controls/tests/TestCases.HostApp/Issues/Issue30970.cs |
UI test page that reproduces the modal navigation scenario with alert display |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue30970.cs |
Automated test that validates the alert popup appears correctly after modal dismissal |
| protected override async void OnAppearing() | ||
| { | ||
| base.OnAppearing(); | ||
| if (_modalPageWasShown) |
There was a problem hiding this comment.
Accessing Application.Current.Windows[0] could throw an IndexOutOfRangeException if no windows exist. Consider checking if the Windows collection has items before accessing the first element.
| if (_modalPageWasShown) | |
| if (_modalPageWasShown && Application.Current.Windows.Count > 0) |
| { | ||
| base.OnAppearing(); | ||
| if (_modalPageWasShown) | ||
| await Application.Current.Windows[0].Page!.DisplayAlert("My alert", "Can you see this alert?", "Yes"); |
There was a problem hiding this comment.
The null-forgiving operator ! on Page suggests potential null values. Consider using null-conditional operator or explicit null check for better error handling.
| await Application.Current.Windows[0].Page!.DisplayAlert("My alert", "Can you see this alert?", "Yes"); | |
| { | |
| var page = Application.Current.Windows[0].Page; | |
| if (page != null) | |
| { | |
| await page.DisplayAlert("My alert", "Can you see this alert?", "Yes"); | |
| } | |
| } |
|
Hey there @@kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
🚀 Dogfood this PR with:
curl -fsSL https://github.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 31016Or
iex "& { $(irm https://github.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 31016" |
StephaneDelcroix
left a comment
There was a problem hiding this comment.
Multi-Model Consensus Review (5 models: claude-opus-4.6 ×2, claude-sonnet-4.6, gemini-3-pro-preview, gpt-5.3-codex)
CI Status: ✅ All checks passing
Consensus Findings
No consensus bugs found. The fix is narrowly scoped to iOS/macOS only.
The PR adds an IsBeingDismissed guard in AlertManager.iOS.cs to skip view controllers that are in the process of being dismissed when searching for the top VC to present alerts on. This prevents alerts from being shown on a VC that is mid-dismiss, which could cause deadlocks or the alert appearing on the wrong window. Checking IsBeingDismissed before traversing to PresentedViewController is the standard iOS pattern for this scenario.
Test Coverage
✅ Includes both a TestCases.HostApp UI page (Issue30970.cs) and corresponding NUnit test (Issue30970.cs). Coverage is appropriate for the platform-specific scenario.
Verdict: ✅ Approve
Correct, minimal iOS fix using standard UIKit pattern.
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #31016 | Skip PresentedViewController that has IsBeingDismissed == true in GetTopUIViewController() loop |
⏳ PENDING (Gate) | AlertManager.iOS.cs |
Simple 1-line guard; stops traversal at dismissing VCs |
🚦 Gate — Test Verification
Gate Result: ✅ PASSED
Platform: iOS
| # | Type | Test Name | Filter |
|---|---|---|---|
| 1 | UITest | Issue30970.PopupShouldBeDisplayed | Issue30970 |
| Step | Expected | Actual | Result |
|---|---|---|---|
| Without fix | FAIL | FAIL (TimeoutException, WaitForElement("Can you see this alert?") timed out after 16s) | ✅ |
| With fix | PASS | PASS (1s, Test Run Successful, Passed: 1) | ✅ |
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) | TransitionCoordinator guard: stop traversal if GetTransitionCoordinator() != null |
✅ PASS | 1 file | More general than IsBeingDismissed — catches all mid-transition VCs |
| 2 | try-fix (claude-sonnet-4.6) | Break-inside-loop with IsBeingDismissed || IsBeingPresented check |
✅ PASS | 1 file | Two-directional guard; more defensive than PR fix |
| 3 | try-fix (gpt-5.3-codex) | Call-site validation: ViewIfLoaded?.Window != null check in PresentPopUp |
❌ FAIL | 1 file | Approach too late — presenter not exposed at call site; TimeoutException |
| 4 | try-fix (gemini-3-pro-preview) | Backtracking: traverse to leaf VC, then backtrack via PresentingViewController if IsBeingDismissed |
✅ PASS | 1 file | Two-pass approach; finds deepest stable VC |
| 5 | try-fix (claude-opus-4.6, cross-poll R1) | TransitionCoordinator deferral: present after dismiss animation completes via AnimateAlongsideTransition |
✅ PASS | 1 file | Most complex; preserves original VC traversal unchanged |
| 6 | try-fix (gpt-5.3-codex, cross-poll R2) | Per-hop multi-guard: IsViewLoaded && View.Window != null && !IsBeingDismissed && !IsBeingPresented |
✅ PASS | 1 file | Most thorough validity check per traversal hop |
| PR | PR #31016 | Skip PresentedViewController with IsBeingDismissed == true |
✅ PASSED (Gate) | 1 file | Simple, targeted |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 1 | Yes | TransitionCoordinator deferral via AnimateAlongsideTransition |
| claude-sonnet-4.6 | 1 | Yes | Same TransitionCoordinator deferral idea |
| gpt-5.3-codex | 1 | Yes | TransitionCoordinator deferral/enqueue |
| gemini-3-pro-preview | 1 | Yes | TransitionCoordinator deferral |
| claude-opus-4.6 | 2 | Yes | Dedicated UIWindow at UIWindowLevel.Alert (bypasses VC chain) |
| claude-sonnet-4.6 | 2 | No | NO NEW IDEAS |
| gpt-5.3-codex | 2 | Yes | Per-hop IsViewLoaded && View.Window != null (run as Attempt 6) |
| claude-opus-4.6 | 3 | No | NO NEW IDEAS — all strategy categories covered |
| gpt-5.3-codex | 3 | Yes | Check presented.ViewIfLoaded?.Window == platformWindow (same window check) |
| gemini-3-pro-preview | 3 | Yes | TransitionCoordinator.GetViewController(ToViewControllerKey) |
Exhausted: Yes (max 3 rounds reached)
Selected Fix: PR #31016 — Simplest, most targeted fix (single !IsBeingDismissed guard). All 5 passing alternatives are valid but more complex without being meaningfully more robust for this specific bug. The PR's fix directly addresses the root cause with minimal code change.
📋 Report — Final Recommendation
✅ Final Recommendation: APPROVE
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #30970, iOS/macOS, 1 fix file + 2 test files |
| Gate | ✅ PASSED | iOS — tests FAIL without fix, PASS with fix |
| Try-Fix | ✅ COMPLETE | 6 attempts: 5 passing, 1 failing; 3 cross-poll rounds |
| Report | ✅ COMPLETE |
Summary
PR #31016 fixes a deadlock in DisplayAlert() on iOS/macOS when called after closing a modal page. The fix is a minimal, targeted 1-line change to GetTopUIViewController() in AlertManager.iOS.cs. A UI test was added to prevent regression. Gate verification confirmed the test correctly catches the bug and passes with the fix. 6 alternative approaches were explored: 5 passed but all were more complex with no clear benefit for this specific bug.
Root Cause
GetTopUIViewController() traverses PresentedViewController chain without checking if any VC in the chain is mid-dismissal (IsBeingDismissed == true). When DisplayAlert() is called during/immediately after PopModalAsync(), the dismissing modal's VC is still in the hierarchy but its view is detached from the window, causing PresentViewControllerAsync to present on an invalid host — the alert never appears and the awaited task deadlocks.
Fix Quality
The fix is excellent: minimal, correct, and well-tested. Adding !topUIViewController.PresentedViewController.IsBeingDismissed to the traversal loop condition stops at the last stable VC before a dismissing one. The test covers the exact reported scenario (open modal → close modal → DisplayAlert). Copilot flagged a minor null-safety concern (Windows[0] indexing without bounds check) in the test's HostApp page that is worth addressing, but it doesn't affect correctness in the test context.
…vigation is in progress - fix (#31016) ### Description of Change By skipping view controllers that are being dismissed (animation is happening, for example ), we ensure DisplayAlert() and other popups are shown only on valid, active UI stacks, preventing the deadlock and improving reliability on iOS/macOS. ### Issues Fixed Fixes #30970
…vigation is in progress - fix (dotnet#31016) ### Description of Change By skipping view controllers that are being dismissed (animation is happening, for example ), we ensure DisplayAlert() and other popups are shown only on valid, active UI stacks, preventing the deadlock and improving reliability on iOS/macOS. ### Issues Fixed Fixes dotnet#30970
…vigation is in progress - fix (#31016) ### Description of Change By skipping view controllers that are being dismissed (animation is happening, for example ), we ensure DisplayAlert() and other popups are shown only on valid, active UI stacks, preventing the deadlock and improving reliability on iOS/macOS. ### Issues Fixed Fixes #30970
…vigation is in progress - fix (dotnet#31016) ### Description of Change By skipping view controllers that are being dismissed (animation is happening, for example ), we ensure DisplayAlert() and other popups are shown only on valid, active UI stacks, preventing the deadlock and improving reliability on iOS/macOS. ### Issues Fixed Fixes dotnet#30970
…vigation is in progress - fix (#31016) ### Description of Change By skipping view controllers that are being dismissed (animation is happening, for example ), we ensure DisplayAlert() and other popups are shown only on valid, active UI stacks, preventing the deadlock and improving reliability on iOS/macOS. ### Issues Fixed Fixes #30970
…vigation is in progress - fix (#31016) ### Description of Change By skipping view controllers that are being dismissed (animation is happening, for example ), we ensure DisplayAlert() and other popups are shown only on valid, active UI stacks, preventing the deadlock and improving reliability on iOS/macOS. ### Issues Fixed Fixes #30970
Description of Change
By skipping view controllers that are being dismissed (animation is happening, for example ), we ensure DisplayAlert() and other popups are shown only on valid, active UI stacks, preventing the deadlock and improving reliability on iOS/macOS.
Issues Fixed
Fixes #30970