[Android] Don't throw when a PanUpdated handler removes the view as the pan starts - #39153
Conversation
<!-- 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. !!!!!!! --> ### Description of Change <!-- Enter description of the fix in this section --> Make the `Issue24284.FlyoutHeaderAdaptsToMinimumHeight` reference visible while the fixture's flyout is initially open. On main, `HeightReferenceLabel` is on Page 1 behind that flyout. Move the existing unchanged `Label` (`HeightRequest="30"`, `AutomationId="HeightReferenceLabel"`, `Text="Hello, World!"`) into `Shell.FlyoutFooter`, leaving Page 1 empty. Main already compares measured reference and header heights. Preserve the exact strict contract `Math.Abs(headerLabel.Height - heightReferenceLabel.Height) < 0.2`; this does not replace a hardcoded 30 comparison or loosen the tolerance. Add a positive-reference-height assertion and measured-height failure diagnostics. Only the fixture XAML and its test change. The constructor remains untouched: `FlyoutIsPresented = true`, header text `Flyout Header`, `MinimumHeightRequest = 30`, and `CollapseOnScroll` behavior are preserved, as is all-platform scope. No product fixes, retries, timeout changes, snapshots, flyout opening/closing workarounds, or shared harness edits. This is a clean main-based replacement for closed predecessor [dotnet#39122](dotnet#39122), not a retarget or a merge of candidate history. Its starting main SHA is `32583cd272cf1ac485243fd13d322bb13f147914`. **Static validation (all passed):** - `dotnet format whitespace --folder --include src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24284.cs` - passed (exit 0). - `dotnet format whitespace --folder --include src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24284.cs --verify-no-changes` - passed (exit 0). - `xmllint --noout src/Controls/tests/TestCases.HostApp/Issues/Issue24284.xaml` - passed (exit 0); XML well-formedness only, not XAML compilation. - `git diff --check` - passed (exit 0). **Validation limitations:** No local native builds, XAML compilation, UI/device tests, simulator/emulator or Appium execution were performed. Actual main-based UI execution and native/XAML compilation remain unverified pending CI. **Historical candidate evidence only (not main validation):** On predecessor dotnet#39122, [UI build 1622631](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1622631) passed the Mac target on both original and subsequent executions (log 5497, lines 719/2422). The contemporaneous [dotnet#39123](dotnet#39123) on the same candidate base without this footer fix failed Mac both times ([UI build 1622632](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1622632), log 5497, lines 720/2422). Fixed Windows, iOSlatest and AndroidMono targets also passed there; AndroidCoreCLR and the full UI run were still incomplete. This supports the fixture approach but does not establish results for this main-based replacement. ### 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. --> Refs dotnet#24284 (fixture reliability only; no claim to fix or close the original product issue). Replaces closed dotnet#39122. <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
…otnet#39128) <!-- 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. !!!!!!! --> ### Description of Change <!-- Enter description of the fix in this section --> Add `App.WaitForElement("Options");` immediately before the first `Options` tap in `VerifyEditorTextWhenAlignedVertically`, matching the neighboring horizontal-alignment test. This synchronizes the initial interaction with element availability using the existing timeout defaults. All subsequent interactions, screenshot assertions, cropping values, pre-existing whitespace, test order/category and helpers are unchanged. No product changes. This is a clean, main-based replacement for closed predecessor [dotnet#39123](dotnet#39123), not a retarget or transplant of its candidate branch history. Base at creation: `32583cd272cf1ac485243fd13d322bb13f147914`. **Candidate evidence, not main validation:** Before the candidate fix, [UI build 1622359](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1622359) showed the Android CoreCLR-labeled API30 null reference at the first `Options` tap (log 5696, line 987), followed by a same-head later pass (log 5696, line 2597). The predecessor's [UI build 1622632](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1622632) subsequently passed this test in all five scheduled configurations / eight observed executions, including original attempts (logs 4688:909, 5332:939+1931, 5090:572, 5366:725+3205, 5475:528+1916). These observations are encouraging but do not prove permanent flake elimination or validate this main-based replacement. **Static validation:** - `dotnet format whitespace --folder --include src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/EditorFeatureTests.cs` — passed. Its unrelated removal of existing `CropBottomValue` trailing whitespace was restored before committing. - `git diff --check` — passed. - Exact byte comparison — passed: precisely the requested insertion; every other byte, including all screenshot checks, unchanged. Command: ```sh python3 -c 'import pathlib,subprocess; p="src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/EditorFeatureTests.cs"; b=subprocess.check_output(["git","show","HEAD:"+p]); n=b"\tpublic void VerifyEditorTextWhenAlignedVertically()\n\t{\n"; assert b.count(n)==1; assert pathlib.Path(p).read_bytes()==b.replace(n,n+b"\t\tApp.WaitForElement(\"Options\");\n",1); print("PASS: exact one-line insertion; every other byte unchanged")' ``` The comparison ran before committing, with `HEAD` pointing to the unchanged main base. Analysis-only `pr-finalize` found the title/body consistent with the tiny diff; no review or approval was posted. Native compilation and UI execution were not run locally and remain pending CI. ### 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. --> No separately filed product issue; addresses the Editor test's initial Options readiness failure documented in predecessor [dotnet#39123](dotnet#39123). <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This pull request updates the following dependencies [marker]: <> (Begin:a71c12d9-5aa4-4b46-e2d6-08da0cf8cd95) ## From https://github.com/dotnet/xharness - **Subscription**: [a71c12d9-5aa4-4b46-e2d6-08da0cf8cd95](https://maestro.dot.net/subscriptions?search=a71c12d9-5aa4-4b46-e2d6-08da0cf8cd95) - **Build**: [20260930.3](https://dev.azure.com/dnceng/internal/_build/results?buildId=3091941) ([334604](https://maestro.dot.net/channel/2/github:dotnet:xharness/build/334604)) - **Date Produced**: September 30, 2026 5:18:20 PM UTC - **Commit**: [3e092b0335764b0b215cb7aaea7c516ed2bf7ed8](dotnet/xharness@3e092b0) - **Branch**: [main](https://github.com/dotnet/xharness/tree/main) [DependencyUpdate]: <> (Begin) - **Dependency Updates**: - From [11.0.0-prerelease.26466.1 to 11.0.0-prerelease.26480.3][1] - Microsoft.DotNet.XHarness.CLI - Microsoft.DotNet.XHarness.TestRunners.Common - Microsoft.DotNet.XHarness.TestRunners.Xunit [1]: dotnet/xharness@22d4210...3e092b0 [DependencyUpdate]: <> (End) [marker]: <> (End:a71c12d9-5aa4-4b46-e2d6-08da0cf8cd95) Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…he pan starts Removing the view from its parent in a PanUpdated handler disconnects its gestures synchronously, which disposes the InnerGestureListener and nulls its delegates. StartScrolling raised the Started event and then called the now-null _scrollDelegate, throwing a NullReferenceException. Stop handling the scroll once the listener has been disposed, as the tap callbacks in the same class already do. Fixes dotnet#38065 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 39153Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 39153" |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hey there @@MykhailoDav! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
Hey there @MykhailoDav! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
This comment has been minimized.
This comment has been minimized.
|
/azp run |
|
Note 🔍
|
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
|
Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection results could not be parsed. Review the workflow run logs for details. Tests Failure Analysis
🧪 CI Analysis — click to expand📊 maui-prNo failures found. 🧪 maui-pr-devicetestsNo failures found. 🧪 maui-pr-uitests
🧭 Follow-up — actions and refreshNext action: Restore UI evidence; compare Android crash/timeout diagnostics at head/base.
|
AI Review Summary
🗂️ Review Sessions — click to expand🚦 Gate — Test Before & After FixGate Result: ✅ PASSEDPlatform: ANDROID · Base: main · Merge base:
🔴 Without fix — 📱 GestureTests (RemovingViewWhenPanStartsDoesNotThrow): FAIL ✅ · 526s(no coded error found; showing last 1200 chars) 🟢 With fix — 📱 GestureTests (RemovingViewWhenPanStartsDoesNotThrow): PASS ✅ · 425s(no coded error found; showing last 1200 chars) 📁 Fix files reverted (1 files)
📋 Pre-Flight — Context & ValidationThe Copilot expert-review task ended before this phase was persisted, usually because the review-stage time budget expired or the CI agent encountered a transient authentication/runtime problem. Earlier completed sections remain valid, but this review is incomplete without this phase. Next step: re-comment 🔬 Code Review — Deep AnalysisThe Copilot expert-review task ended before this phase was persisted, usually because the review-stage time budget expired or the CI agent encountered a transient authentication/runtime problem. Earlier completed sections remain valid, but this review is incomplete without this phase. Next step: re-comment 🛠️ Try-Fix — Analysis & ComparisonThe Copilot expert-review task ended before this phase was persisted, usually because the review-stage time budget expired or the CI agent encountered a transient authentication/runtime problem. Earlier completed sections remain valid, but this review is incomplete without this phase. Next step: re-comment 🏁 Report — Final RecommendationThe Copilot expert-review task ended before this phase was persisted, usually because the review-stage time budget expired or the CI agent encountered a transient authentication/runtime problem. Earlier completed sections remain valid, but this review is incomplete without this phase. Next step: re-comment 📱 UI Tests — Button,Label,LayoutDetected UI test categories:
🧪 UI Test Execution Results (deep, platform pool)
|
Description of Change
On Android, removing a view from its parent in a
PanUpdatedhandler when the pan starts throws:The reporter found that their handler removes the dragged view. Removing or reparenting a view when a drag starts is still a valid thing to do, and the framework shouldn't throw a
NullReferenceExceptionfrom its own gesture listener when it happens.Root cause
InnerGestureListener.StartScrollingraisesPanUpdated(Started) through_scrollStartedDelegateand then calls_scrollDelegate. Removing the view from that handler changes itsWindowto null, andGestureManager.SetupGestureManagerthen disconnects the gestures synchronously:GesturePlatformManager.Dispose→TapAndPanGestureDetector.Dispose→InnerGestureListener.Dispose, which sets_scrollDelegate,_swipeDelegateand the other delegates to null.When the handler returns,
StartScrollingcalls the now-null_scrollDelegate.Fix
Return from
StartScrollingif the listener was disposed by theStartedhandler. The tap callbacks in the same class (OnSingleTapUp,OnSingleTapConfirmed,OnDoubleTap) already return early when_disposed.Only this spot needs it:
Runningupdate,PanGestureHandler.OnPanstill returns true, so_swipeDelegateisn't reached.EndScrollingalready null-checks its delegates.The issue says this worked in 10.0.90. I couldn't find a change in this code path between 10.0.90 and 10.0.100:
GestureManagerandTapAndPanGestureDetectorare unchanged, andInnerGestureListeneronly changedHasAnyGestures.Tests
New Android device test
RemovingViewWhenPanStartsDoesNotThrowinGestureTests.Android.cs(categoryGesture). It adds aBoxViewwith aPanGestureRecognizerwhosePanUpdatedhandler removes the view onStarted. It then dispatches a synthetic down / move / up sequence (the move is beyond the touch slop) to the view's platform view.Android 37 emulator, Release:
NullReferenceExceptionatInnerGestureListener.StartScrolling←OnScrollCategory=GestureWith the fix,
SwipeView(20),Label(63, 1 ignored),CarouselView(10) andScrollView(13) also pass. I didn't run the Appium UI tests locally.Issues Fixed
Fixes #38065
🤖 Generated with Claude Code