[Android] Fix Shell singleton page rendering blank when re-pushed after absolute route reset - #36903
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36903Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36903" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hey there @@HarishwaranVijayakumar! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
/azp run maui-pr-uitests , maui-pr-devicetests |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Fixes an Android Shell regression where DI-singleton pages can render blank when re-pushed after an absolute-route stack reset, by ensuring stale fragments are actually removed and fragment transactions are only committed when needed.
Changes:
- Adjusted Android
ShellItemRendererBase.RemoveAllPushedPagesto lazily create/commit fragment transactions only when fragments are removed. - Added a HostApp repro scenario that uses DI singleton + transient pages with Shell routes to match the regression conditions.
- Added an Appium UITest that validates the singleton page renders after
///reset and re-push.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellItemRendererBase.cs | Avoids committing an empty fragment transaction and ensures pushed-page fragments are removed from the map/FragmentManager. |
| src/Controls/tests/TestCases.HostApp/MauiProgram.cs | Registers DI services needed to reproduce the singleton/transient routing scenario. |
| src/Controls/tests/TestCases.HostApp/Issues/Issue36853.cs | Adds the Shell-based repro page and registers routes + DI lifetimes matching the regression. |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36853.cs | Adds an automated regression UITest covering PopToRoot via absolute route and re-push behavior. |
|
|
||
| _fragmentMap.Remove(kvp.Key); | ||
|
|
||
| if (keepCurrent && kvp.Value.Fragment == _currentFragment) |
This comment has been minimized.
This comment has been minimized.
AI Review Summary
🗂️ Review Sessions — click to expand🚦 Gate — Test Before & After FixGate Result: ✅ PASSEDPlatform: ANDROID · Base: main · Merge base:
🔴 Without fix — 🖥️ Issue36853: FAIL ✅ · 2448sError-relevant lines (filtered from the build log): 🟢 With fix — 🖥️ Issue36853: PASS ✅ · 582s(no coded error found; showing last 1200 chars) 📁 Fix files reverted (1 files)
📱 UI Tests — ShellDetected UI test categories: ✅ Deep UI tests — 310 passed, 0 failed across 1 category on platform-pool agent (replaces in-process counts above). 🧪 UI Test Execution Results (deep, platform pool)
📋 Pre-Flight — Context & ValidationIssue: #36853 - [regression/10.0.90] Shell: DI-Singleton page pushed via a registered route renders blank when re-pushed after an absolute-route stack reset Key Findings
Code Review SummaryVerdict: NEEDS_DISCUSSION Key code review findings:
Fix Candidates
🔬 Code Review — Deep AnalysisCode Review — PR #36903Independent AssessmentWhat this changes: Android Shell Inferred motivation: Prevent stale hidden fragments from being reused for DI-singleton routed pages whose handlers were disconnected during stack reset. Reconciliation with PR NarrativeAuthor claims: Fixes #36853 by removing stale Android Shell fragments during absolute-route Agreement/disagreement: The code matches that claim. The old stack-count early return could skip removal after the logical stack had already reached root; the new map-based removal targets the stale-fragment failure mode described. Prior Review ReconciliationNo prior ❌ Error findings found. Prior surfaces checked: top-level reviews, inline review comments, and issue comments. One prior Copilot inline comment was a non-blocking clarity suggestion about comparing fragment wrapper instances rather than Blast Radius Assessment
CI Status
External Output Contract
FindingsNo ❌ Errors or Failure-Mode Probing
Verdict: NEEDS_DISCUSSIONConfidence: low Summary: I found no code-level correctness issues, and the fix matches the Android Shell stale-fragment failure mode. However, required CI status could not be determined with 🛠️ Fix — Analysis & ComparisonFix Candidates
Cross-Pollination
Exhausted: Yes 🏁 Report — Final RecommendationComparative Fix Report — PR #36903SummaryAll evaluated candidates passed the supplied Android regression gate for Candidate Ranking
Comparative AnalysisThe PR fix is the most direct root-cause repair. The baseline bug is that
WinnerWinner: The raw PR fix wins because it passed the regression gate, has no expert-reviewer findings, and is the simplest targeted fix at the stale fragment-map cleanup source. 🧭 Next Steps — review latest findingsNo alternative fix was selected for this run. Review the session findings and CI results before merging. |
|
/backport to inflight/candidate |
|
Started backporting to |
…nk when re-pushed after absolute route reset (#37113) Backport of #36903 to inflight/candidate /cc @kubaflo @HarishwaranVijayakumar Co-authored-by: HarishwaranVijayakumar <harishwaran.vijayakumar@syncfusion.com>
…er absolute route reset (#36903) <!-- Please let the below note in for people that find this PR --> > [!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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details: - A DI-singleton page in Shell navigation renders blank when re-pushed after a GoToAsync("///MainPage") stack reset. ### Root Cause of the issue - PR #[35476](#35476) moved page.DisconnectHandlers() into RemovePage() , which runs on all pop paths including PopToRoot. This permanently destroys the handler for singleton pages. On Android, a pre-existing bug in RemoveAllPushedPages compounds this — it checks shellSection.Stack.Count which is already 1 by the time the platform event fires, so it early-returns and never removes stale fragments from _fragmentMap . On re-push, the old fragment is reused without calling OnCreateView , resulting in a blank page since the handler is null. **Why It Only Affects Singletons** - Transient pages get a new instance each push → new fragment → renders fine. Singleton pages reuse the same instance → old stale fragment found → no OnCreateView → blank. ### Description of Change ### Bug Fix: Fragment Removal Logic * Improved the logic in `RemoveAllPushedPages` in `ShellItemRendererBase.cs` to ensure that fragment transactions are only committed if any fragments were actually removed, preventing issues where singleton pages could render blank after navigation. ### New Test Case for Regression * Added a new test page and scenario in `Issue36853.cs` (HostApp) to reproduce the bug: navigating Root → SecondPage (singleton) → ThirdPage → PopToRoot → re-push SecondPage, which previously resulted in a blank screen. * Registered `Issue36853SecondPage` as a singleton and `Issue36853ThirdPage` as transient in the DI container, matching the real-world bug scenario. [[1]](diffhunk://#diff-70f90c4c40092b9b58b067b769ce0a5ad5db3b0a8a72359fd7b816a682ad181bR1-R111) [[2]](diffhunk://#diff-d01e2392fa68bd429a87ca25ef57d4ad83147746e650291c69f97c197d92628dL44-R45) ### Automated UITest * Introduced an automated UITest in `TestCases.Shared.Tests/Tests/Issues/Issue36853.cs` to verify that after PopToRoot and re-pushing the singleton page, its content is visible and not blank. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #36853 ### Tested the behavior in the following platforms - [x] Windows - [x] Android - [x] iOS - [x] Mac ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/bb4201ac-5be1-4553-8f34-2b4f4556dfd9"> | <video src="https://github.com/user-attachments/assets/de6b0981-26d4-4c79-9490-9f1c65e17f92"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
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:
Root Cause of the issue
Why It Only Affects Singletons
Description of Change
Bug Fix: Fragment Removal Logic
RemoveAllPushedPagesinShellItemRendererBase.csto ensure that fragment transactions are only committed if any fragments were actually removed, preventing issues where singleton pages could render blank after navigation.New Test Case for Regression
Issue36853.cs(HostApp) to reproduce the bug: navigating Root → SecondPage (singleton) → ThirdPage → PopToRoot → re-push SecondPage, which previously resulted in a blank screen.Issue36853SecondPageas a singleton andIssue36853ThirdPageas transient in the DI container, matching the real-world bug scenario. [1] [2]Automated UITest
TestCases.Shared.Tests/Tests/Issues/Issue36853.csto verify that after PopToRoot and re-pushing the singleton page, its content is visible and not blank.Issues Fixed
Fixes #36853
Fixes #36852
Tested the behavior in the following platforms
Output
Beforefix-36853.mov
Afterfix-36853.mov