Repository navigation
Fix Picker reacts to data changes after the page is closed - #33733
Conversation
|
/rebase |
059e1b8 to
e2cd03a
Compare
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the Picker control continues to react to ObservableCollection changes after the page containing it has been closed and removed from the navigation stack. The fix unsubscribes from collection change events when the Picker's handler is detached.
Changes:
- Added unsubscription logic in
OnHandlerChangedwhen handler becomes null - Created comprehensive UI test that reproduces the issue and validates the fix
- Test verifies that Picker selection remains correct after navigating away and deleting items
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Controls/src/Core/Picker/Picker.cs | Adds unsubscription from ItemsSource collection changes when handler is detached |
| src/Controls/tests/TestCases.HostApp/Issues/Issue33307.cs | Implements test page with navigation scenario, Picker bound to ObservableCollection, and item deletion workflow |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33307.cs | NUnit test that validates Picker doesn't incorrectly update after page is closed and items are deleted |
| // If Handler is being detached, unsubscribe from ItemsSource collection changes | ||
| if (Handler is null) | ||
| { | ||
| if (ItemsSource is INotifyCollectionChanged notifyCollection) | ||
| { | ||
| notifyCollection.CollectionChanged -= CollectionChanged; | ||
| } | ||
| } |
There was a problem hiding this comment.
The fix only unsubscribes when the handler is detached, but doesn't re-subscribe when a new handler is attached. This creates an asymmetry: if a Picker's handler is detached and then re-attached (e.g., when a page is popped and pushed again), the Picker will no longer respond to ItemsSource collection changes.
Consider also subscribing when Handler becomes non-null to maintain symmetric behavior. For example:
When Handler transitions from null to non-null, check if ItemsSource is INotifyCollectionChanged and re-subscribe if needed. This ensures the Picker continues to work correctly if it's reused after being detached.
|
|
||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, 33307, "The Picker is still binding to the property and reacts to data changes after the page is closed.", PlatformAffected.UWP)] |
There was a problem hiding this comment.
The PlatformAffected attribute indicates "UWP", but according to the PR description and testing notes, this issue affects both Windows and Android platforms. The PlatformAffected value should be updated to reflect all affected platforms accurately, or changed to PlatformAffected.All if it affects all platforms.
| [Issue(IssueTracker.Github, 33307, "The Picker is still binding to the property and reacts to data changes after the page is closed.", PlatformAffected.UWP)] | |
| [Issue(IssueTracker.Github, 33307, "The Picker is still binding to the property and reacts to data changes after the page is closed.", PlatformAffected.Windows | PlatformAffected.Android)] |
) > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could <a href="https://github.com/dotnet/maui/wiki/Testing-PR-Builds">test the resulting artifacts</a> from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Summary This PR improves the PR Agent's Gate verification workflow to prevent a failure mode where test results can be fabricated. ## Problem During PR review of #33733, the agent ran a **single** test command but reported that tests "failed both with and without the fix" - which is impossible to determine from one test run. The Gate verification requires TWO test runs: 1. Revert fix → run tests (should FAIL) 2. Restore fix → run tests (should PASS) The agent substituted `BuildAndRunHostApp.ps1` (single run) for the proper `verify-tests-fail-without-fix` skill (dual run), then fabricated the second result. ## Solution ### 1. Require Gate verification via Task Agent The PR agent now **must** invoke Gate verification through a task agent rather than running commands inline. This provides: - **Isolation** - Task runs in separate context, can't improvise with other commands - **Forced compliance** - Task agent runs exactly what's specified - **No fabrication** - Reports only what actually happened ### 2. Reference skill by name, not script path Instead of hardcoding: ```bash pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 ... ``` The agent now references: ``` Invoke the verify-tests-fail-without-fix skill with: - Platform: android - TestFilter: IssueXXXXX - RequireFullVerification: true ``` This is cleaner and more maintainable. ### 3. Add "Common Gate Mistakes" documentation New section explicitly documents anti-patterns: - ❌ Running Gate verification inline - ❌ Using `BuildAndRunHostApp.ps1` for Gate - ❌ Claiming "fails both ways" from a single test run ### 4. Fix ai-summary-comment regex The `post-ai-summary-comment.ps1` script's regex didn't handle `<details open>` - only `<details>`. Updated regex from `<details>` to `<details[^>]*>` to handle optional attributes. ## Files Changed | File | Change | |------|--------| | `.github/agents/pr.md` | Gate must use task agent; added Common Gate Mistakes section | | `.github/skills/ai-summary-comment/scripts/post-ai-summary-comment.ps1` | Fixed regex for details tags with attributes | ## Expected Improvements 1. **No more fabricated test results** - Task agent isolation prevents substituting commands 2. **Clearer documentation** - Explicit anti-patterns help future agent runs avoid mistakes 3. **More reliable PR reviews** - Gate verification actually runs both directions before reporting
…net#33806) > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could <a href="https://github.com/dotnet/maui/wiki/Testing-PR-Builds">test the resulting artifacts</a> from this PR and let us know in a comment if this change resolves your issue. Thank you! ## Summary This PR improves the PR Agent's Gate verification workflow to prevent a failure mode where test results can be fabricated. ## Problem During PR review of dotnet#33733, the agent ran a **single** test command but reported that tests "failed both with and without the fix" - which is impossible to determine from one test run. The Gate verification requires TWO test runs: 1. Revert fix → run tests (should FAIL) 2. Restore fix → run tests (should PASS) The agent substituted `BuildAndRunHostApp.ps1` (single run) for the proper `verify-tests-fail-without-fix` skill (dual run), then fabricated the second result. ## Solution ### 1. Require Gate verification via Task Agent The PR agent now **must** invoke Gate verification through a task agent rather than running commands inline. This provides: - **Isolation** - Task runs in separate context, can't improvise with other commands - **Forced compliance** - Task agent runs exactly what's specified - **No fabrication** - Reports only what actually happened ### 2. Reference skill by name, not script path Instead of hardcoding: ```bash pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 ... ``` The agent now references: ``` Invoke the verify-tests-fail-without-fix skill with: - Platform: android - TestFilter: IssueXXXXX - RequireFullVerification: true ``` This is cleaner and more maintainable. ### 3. Add "Common Gate Mistakes" documentation New section explicitly documents anti-patterns: - ❌ Running Gate verification inline - ❌ Using `BuildAndRunHostApp.ps1` for Gate - ❌ Claiming "fails both ways" from a single test run ### 4. Fix ai-summary-comment regex The `post-ai-summary-comment.ps1` script's regex didn't handle `<details open>` - only `<details>`. Updated regex from `<details>` to `<details[^>]*>` to handle optional attributes. ## Files Changed | File | Change | |------|--------| | `.github/agents/pr.md` | Gate must use task agent; added Common Gate Mistakes section | | `.github/skills/ai-summary-comment/scripts/post-ai-summary-comment.ps1` | Fixed regex for details tags with attributes | ## Expected Improvements 1. **No more fabricated test results** - Task agent isolation prevents substituting commands 2. **Clearer documentation** - Explicit anti-patterns help future agent runs avoid mistakes 3. **More reliable PR reviews** - Gate verification actually runs both directions before reporting
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the ai's suggestions?
|
/review -b feature/enhanced-reviewer -p android |
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@devanathan-vaithiyanathan — new AI review results are available based on this last commit:
58b1d14. To request a fresh review after new comments or commits, comment/review rerun.
🚀 Next Steps — alternative fix proposed (try-fix-1)
Automated review — alternative fix proposed
The expert-reviewer evaluation compared the PR fix against automatically generated candidates and selected try-fix-1 as the strongest fix.
Why: try-fix-1 won because it fixes the detached Picker collection-change leak while preserving existing handlerless Picker ItemsSource synchronization. It also passed the targeted Picker unit suite and the requested Android UI regression, while the raw PR fix failed the handlerless synchronization regression.
Please consider applying the candidate diff below (or use it as guidance). Once you push an update, this workflow will re-trigger and re-evaluate.
Candidate diff (try-fix-1)
diff --git a/src/Controls/src/Core/Picker/Picker.cs b/src/Controls/src/Core/Picker/Picker.cs
index 2b8f363bbf..1918800ae0 100644
--- a/src/Controls/src/Core/Picker/Picker.cs
+++ b/src/Controls/src/Core/Picker/Picker.cs
@@ -304,16 +304,15 @@ namespace Microsoft.Maui.Controls
void OnItemsSourceChanged(IList oldValue, IList newValue)
{
- var oldObservable = oldValue as INotifyCollectionChanged;
- if (oldObservable != null)
- oldObservable.CollectionChanged -= CollectionChanged;
-
- var newObservable = newValue as INotifyCollectionChanged;
- if (newObservable != null)
+ // Always unsubscribe from old source
+ if (ReferenceEquals(oldValue, _subscribedItemsSourceCollection))
{
- newObservable.CollectionChanged += CollectionChanged;
+ UnsubscribeFromItemsSourceCollection();
}
+ // Always subscribe to new source (preserves handlerless model sync)
+ SubscribeToItemsSourceCollection(newValue as INotifyCollectionChanged);
+
if (newValue != null)
{
((LockableObservableListWrapper)Items).IsLocked = true;
@@ -328,6 +327,7 @@ namespace Microsoft.Maui.Controls
}
readonly Queue<Action> _pendingIsOpenActions = new Queue<Action>();
+ INotifyCollectionChanged _subscribedItemsSourceCollection;
void OnIsOpenPropertyChanged(bool oldValue, bool newValue)
{
@@ -343,8 +343,25 @@ namespace Microsoft.Maui.Controls
protected override void OnHandlerChanged()
{
+ if (Handler is null)
+ {
+ // Detached from visual tree: stop reacting to ItemsSource changes
+ UnsubscribeFromItemsSourceCollection();
+ }
+
base.OnHandlerChanged();
+ if (Handler is not null)
+ {
+ // Reattached: resubscribe and resync display items
+ SubscribeToItemsSourceCollection(ItemsSource as INotifyCollectionChanged);
+
+ if (ItemsSource is not null)
+ {
+ ResetItems();
+ }
+ }
+
// Process any pending actions when handler becomes available
while (_pendingIsOpenActions.Count > 0 && Handler != null)
{
@@ -364,6 +381,29 @@ namespace Microsoft.Maui.Controls
picker.Closed?.Invoke(picker, PickerClosedEventArgs.Empty);
}
+ void SubscribeToItemsSourceCollection(INotifyCollectionChanged collection)
+ {
+ if (collection is null || ReferenceEquals(collection, _subscribedItemsSourceCollection))
+ {
+ return;
+ }
+
+ UnsubscribeFromItemsSourceCollection();
+ _subscribedItemsSourceCollection = collection;
+ _subscribedItemsSourceCollection.CollectionChanged += CollectionChanged;
+ }
+
+ void UnsubscribeFromItemsSourceCollection()
+ {
+ if (_subscribedItemsSourceCollection is null)
+ {
+ return;
+ }
+
+ _subscribedItemsSourceCollection.CollectionChanged -= CollectionChanged;
+ _subscribedItemsSourceCollection = null;
+ }
+
void CollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
{
switch (e.Action)
🗂️ Review Sessions — click to expand
🧪 Gate — Test Before & After Fix
Gate Result: ✅ PASSED
Platform: ANDROID · Base: main · Merge base: 4567a055
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🖥️ Issue33307 Issue33307 |
✅ FAIL — 2283s | ✅ PASS — 531s |
🔴 Without fix — 🖥️ Issue33307: FAIL ✅ · 2283s
(truncated to last 15,000 chars)
: Failure calling service package: Broken pipe (32) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Mono.AndroidTools.Internal.AdbOutputParsing.CheckInstallSuccess(String output, String packageName) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Mono.AndroidTools.AndroidDevice.<>c__DisplayClass105_0.<InstallPackage>b__0(Task`1 t) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: --- End of stack trace from previous location --- [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: --- End of stack trace from previous location --- [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at AndroidDeviceExtensions.PushAndInstallPackageAsync(AndroidDevice device, PushAndInstallCommand command, CancellationToken token) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at AndroidDeviceExtensions.PushAndInstallPackageAsync(AndroidDevice device, PushAndInstallCommand command, CancellationToken token) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Xamarin.Android.Tasks.FastDeploy.InstallPackage(Boolean installed) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Xamarin.Android.Tasks.FastDeploy.InstallPackage(Boolean installed) [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
/home/vsts/work/1/s/.dotnet/packs/Microsoft.Android.Sdk.Linux/36.1.2/tools/Xamarin.Android.Common.Debugging.targets(333,5): error ADB0010: at Xamarin.Android.Tasks.FastDeploy.RunInstall() [/home/vsts/work/1/s/src/Controls/tests/TestCases.HostApp/Controls.TestCases.HostApp.csproj::TargetFramework=net10.0-android]
0 Warning(s)
1 Error(s)
Time Elapsed 00:16:41.84
* daemon not running; starting now at tcp:5037
* daemon started successfully
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0-android36.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0-android36.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0-android36.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0-android36.0/Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-android36.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-android36.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Xaml.dll
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Foldable.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Maps.dll
Controls.TestCases.HostApp -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Controls.TestCases.HostApp.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Graphics -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Essentials -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Maps.dll
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Xaml.dll
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Foldable.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Maps.dll
Build succeeded.
0 Warning(s)
0 Error(s)
Time Elapsed 00:08:14.32
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Determining projects to restore...
Restored /home/vsts/work/1/s/src/TestUtils/src/VisualTestUtils/VisualTestUtils.csproj (in 1.56 sec).
Restored /home/vsts/work/1/s/src/TestUtils/src/VisualTestUtils.MagickNet/VisualTestUtils.MagickNet.csproj (in 3.94 sec).
Restored /home/vsts/work/1/s/src/Controls/tests/TestCases.Android.Tests/Controls.TestCases.Android.Tests.csproj (in 5.68 sec).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.Core/UITest.Core.csproj (in 6 ms).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.Appium/UITest.Appium.csproj (in 2 ms).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.NUnit/UITest.NUnit.csproj (in 373 ms).
Restored /home/vsts/work/1/s/src/Controls/tests/CustomAttributes/Controls.CustomAttributes.csproj (in 18 ms).
Restored /home/vsts/work/1/s/src/TestUtils/src/UITest.Analyzers/UITest.Analyzers.csproj (in 1.72 sec).
5 of 13 projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
Controls.CustomAttributes -> /home/vsts/work/1/s/artifacts/bin/Controls.CustomAttributes/Debug/net10.0/Controls.CustomAttributes.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
UITest.Core -> /home/vsts/work/1/s/artifacts/bin/UITest.Core/Debug/net10.0/UITest.Core.dll
UITest.Appium -> /home/vsts/work/1/s/artifacts/bin/UITest.Appium/Debug/net10.0/UITest.Appium.dll
UITest.NUnit -> /home/vsts/work/1/s/artifacts/bin/UITest.NUnit/Debug/net10.0/UITest.NUnit.dll
VisualTestUtils -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils/Debug/netstandard2.0/VisualTestUtils.dll
VisualTestUtils.MagickNet -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils.MagickNet/Debug/netstandard2.0/VisualTestUtils.MagickNet.dll
UITest.Analyzers -> /home/vsts/work/1/s/artifacts/bin/UITest.Analyzers/Debug/netstandard2.0/UITest.Analyzers.dll
Controls.TestCases.Android.Tests -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
NUnit3TestExecutor discovered 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 06/21/2026 18:59:39 FixtureSetup for Issue33307(Android)
>>>>> 06/21/2026 18:59:41 VerifyPickerItemsinNavigation Start
>>>>> 06/21/2026 18:59:45 VerifyPickerItemsinNavigation Stop
>>>>> 06/21/2026 18:59:45 Log types: logcat, bugreport, server
Failed VerifyPickerItemsinNavigation [5 s]
Error Message:
System.NullReferenceException : Object reference not set to an instance of an object.
Stack Trace:
at UITest.Appium.HelperExtensions.Click(IUIElement element) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 412
at UITest.Appium.HelperExtensions.<>c__DisplayClass2_0.<Tap>b__0() in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 36
at UITest.Appium.HelperExtensions.<>c__DisplayClass186_0.<RunWithTimeout>b__0() in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 3075
at System.Threading.Tasks.Task`1.InnerInvoke()
at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread)
--- End of stack trace from previous location ---
at UITest.Appium.HelperExtensions.RunWithTimeout[T](Func`1 action, Nullable`1 timeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 3063
at UITest.Appium.HelperExtensions.RunWithTimeout(Action action, Nullable`1 timeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 3075
at UITest.Appium.HelperExtensions.Tap(IApp app, String element) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 36
at Microsoft.Maui.TestCases.Tests.Issues.Issue33307.VerifyPickerItemsinNavigation() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33307.cs:line 17
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
NUnit Adapter 4.5.0.0: Test execution complete
[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.14] Discovering: Controls.TestCases.Android.Tests
[xUnit.net 00:00:00.56] Discovered: Controls.TestCases.Android.Tests
Results File: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue33307.trx
Total tests: 1
Failed: 1
Test Run Failed.
Total time: 32.7236 Seconds
>>> TRX_RESULT_FILE: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue33307.trx
🟢 With fix — 🖥️ Issue33307: PASS ✅ · 531s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0-android36.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0-android36.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0-android36.0/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Maps -> /home/vsts/work/1/s/artifacts/bin/Maps/Debug/net10.0-android36.0/Microsoft.Maui.Maps.dll
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0-android36.0/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Microsoft.AspNetCore.Components.WebView.Maui/Debug/net10.0-android36.0/Microsoft.AspNetCore.Components.WebView.Maui.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.Xaml/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Xaml.dll
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.Maps/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Maps.dll
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.Foldable/Debug/net10.0-android36.0/Microsoft.Maui.Controls.Foldable.dll
Controls.TestCases.HostApp -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Controls.TestCases.HostApp.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Graphics -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Essentials -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Maps.dll
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Controls.Maps -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Maps.dll
Controls.Xaml -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Xaml.dll
Controls.Foldable -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.Maui.Controls.Foldable.dll
Microsoft.AspNetCore.Components.WebView.Maui -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.HostApp/Debug/net10.0-android/Microsoft.AspNetCore.Components.WebView.Maui.dll
Build succeeded.
0 Warning(s)
0 Error(s)
Time Elapsed 00:06:58.71
Broadcasting: Intent { act=android.intent.action.CLOSE_SYSTEM_DIALOGS flg=0x400000 }
Broadcast completed: result=0
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Graphics -> /home/vsts/work/1/s/artifacts/bin/Graphics/Debug/net10.0/Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Essentials -> /home/vsts/work/1/s/artifacts/bin/Essentials/Debug/net10.0/Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Core -> /home/vsts/work/1/s/artifacts/bin/Core/Debug/net10.0/Microsoft.Maui.dll
Controls.CustomAttributes -> /home/vsts/work/1/s/artifacts/bin/Controls.CustomAttributes/Debug/net10.0/Controls.CustomAttributes.dll
Controls.BindingSourceGen -> /home/vsts/work/1/s/artifacts/bin/Controls.BindingSourceGen/Debug/netstandard2.0/Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14442869
Controls.Core -> /home/vsts/work/1/s/artifacts/bin/Controls.Core/Debug/net10.0/Microsoft.Maui.Controls.dll
UITest.Core -> /home/vsts/work/1/s/artifacts/bin/UITest.Core/Debug/net10.0/UITest.Core.dll
UITest.Appium -> /home/vsts/work/1/s/artifacts/bin/UITest.Appium/Debug/net10.0/UITest.Appium.dll
UITest.NUnit -> /home/vsts/work/1/s/artifacts/bin/UITest.NUnit/Debug/net10.0/UITest.NUnit.dll
VisualTestUtils -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils/Debug/netstandard2.0/VisualTestUtils.dll
VisualTestUtils.MagickNet -> /home/vsts/work/1/s/artifacts/bin/VisualTestUtils.MagickNet/Debug/netstandard2.0/VisualTestUtils.MagickNet.dll
UITest.Analyzers -> /home/vsts/work/1/s/artifacts/bin/UITest.Analyzers/Debug/netstandard2.0/UITest.Analyzers.dll
Controls.TestCases.Android.Tests -> /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
Test run for /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in /home/vsts/work/1/s/artifacts/bin/Controls.TestCases.Android.Tests/Debug/net10.0/Controls.TestCases.Android.Tests.dll
NUnit3TestExecutor discovered 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 06/21/2026 19:17:48 FixtureSetup for Issue33307(Android)
>>>>> 06/21/2026 19:17:49 VerifyPickerItemsinNavigation Start
>>>>> 06/21/2026 19:18:05 VerifyPickerItemsinNavigation Stop
Passed VerifyPickerItemsinNavigation [16 s]
NUnit Adapter 4.5.0.0: Test execution complete
[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.12] Discovering: Controls.TestCases.Android.Tests
[xUnit.net 00:00:00.54] Discovered: Controls.TestCases.Android.Tests
Results File: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue33307.trx
Test Run Successful.
Total tests: 1
Passed: 1
Total time: 27.5388 Seconds
>>> TRX_RESULT_FILE: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests/TestResults/Issue33307.trx
📁 Fix files reverted (1 files)
src/Controls/src/Core/Picker/Picker.cs
📱 UI Tests — Picker
Detected UI test categories: Picker
✅ Deep UI tests — 51 passed, 0 failed across 1 category on platform-pool agent (replaces in-process counts above).
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
Picker |
51/51 ✓ | — |
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs) |
🛫 Pre-Flight — Context & Validation
Issue: #33307 - The Picker is still binding to the property and reacts to data changes after the page is closed.
PR: #33733 - Picker ItemsSource collection subscription lifecycle fix
Platforms Affected: Android testing requested; issue metadata says UWP; Picker core behavior affects all platforms using Picker.ItemsSource.
Files Changed: 1 implementation, 2 test
Key Findings
- GitHub CLI was unauthenticated, so PR description, linked issue body/comments, review comments, and required-check status could not be gathered through
gh; public web fetches were also blocked. Pre-flight continued from the checked-outpr-review-33733branch and local diff againstorigin/main. - The PR changes
Pickerso observableItemsSourcecollection subscriptions are active only while a handler is attached, then resubscribes and resets items when the handler is reattached. - The included UI test reproduces navigation between pages with shared data and asserts that a detached Picker no longer reacts after its page is closed.
Code Review Summary
Verdict: NEEDS_CHANGES
Confidence: high
Errors: 1 | Warnings: 1 | Suggestions: 1
Key code review findings:
- ❌
src/Controls/src/Core/Picker/Picker.cs:312-315only subscribes whenHandler is not null, regressing existing handlerless PickerItemsSourcesynchronization semantics; local targeted test evidence from the reviewer reportedPickerTests.TestItemsSourceCollectionChangedAppendexpected 4 items but saw 3. ⚠️ src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33307.cs:21-28does not assert thatSelectSecondItemchanged the label before navigating away, so the final"None"assertion can pass even if selection never worked.- 💡
src/Controls/tests/TestCases.HostApp/Issues/Issue33307.cs:6uses stalePlatformAffected.UWPmetadata despite Android testing being requested.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #33733 | Track observable ItemsSource subscriptions only while the Picker has a handler; resubscribe/reset on handler attach. |
✅ PASSED (Gate) | Picker.cs, UI test files |
Original PR; gate was completed before this run and must not be rerun here. |
🔬 Code Review — Deep Analysis
Code Review — PR #33733
Independent Assessment
What this changes: Picker now subscribes to ItemsSource.CollectionChanged only while it has a handler, unsubscribing on detach and resubscribing/resetting on attach. Adds UI repro coverage.
Inferred motivation: Prevent closed/navigated-away Picker instances from reacting to shared ObservableCollection changes.
Reconciliation with PR Narrative
Author claims: Prevent unwanted Picker updates after handler detach/page close.
Agreement/disagreement: Motivation matches, but implementation regresses existing handlerless Picker model behavior.
Prior Review Reconciliation
| Prior ❌ Error Finding | Source | Status | Evidence |
|---|---|---|---|
| PR fix still fails gate / regresses handlerless Picker synchronization | MauiBot issue comment | ❌ Unresolved | Local targeted test fails: TestItemsSourceCollectionChangedAppend, expected 4 actual 3 |
| Missing resubscribe on handler reattach | MauiBot/Copilot inline | ✅ Fixed | Picker.cs:354-362 now subscribes/resets on handler attach |
| UI test lacks precondition after selection | MauiBot inline | ❌ Unresolved | Test still asserts only final "None" |
Blast Radius Assessment
- Runs for all instances: Yes — all
Pickerwith observableItemsSource. - Startup impact: No direct startup path.
- Static/shared state: No.
CI Status
- Required-check result:
gh pr checks --requiredunavailable:gh auth loginrequired. - Public check-run fallback: commit shows
maui-prfailure, Build Analysis failure, Windows Debug build failure, Windows Helix unit failures; combined status pending. - Classification: failing / partly undetermined; local targeted unit failure is PR-caused.
- Action taken: Used local branch diff fallback and public REST/curl. No PR comments posted.
azdo-build-investigatorloaded, but authenticated GitHub/AzDO surfaces were unavailable.
Findings
❌ Error — Picker no longer tracks observable ItemsSource before handler attach
src/Controls/src/Core/Picker/Picker.cs:312-315 only subscribes when Handler is not null. Existing Core behavior keeps Picker.Items synchronized even for handlerless Picker instances. This breaks existing unit tests such as PickerTests.TestItemsSourceCollectionChangedAppend (PickerTests.cs:285): local run failed with expected count 4, actual 3.
⚠️ Warning — Regression test can pass without proving selection occurred
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33307.cs:21-28 taps SelectSecondItem but never asserts the label became itemB2 before navigating away. Since the final expected value is the label’s initial "None", failures in the selection path can mask the regression.
💡 Suggestion — Platform metadata looks stale
src/Controls/tests/TestCases.HostApp/Issues/Issue33307.cs:6 says PlatformAffected.UWP, while the PR says Android/Windows/iOS/Mac were tested.
Failure-Mode Probing
- Handlerless Picker with observable source: collection mutations are ignored; confirmed by failing unit test.
- Detach/reattach: resubscribe exists, but initial model synchronization regression remains.
- Null handler/default ItemsSource: no crash seen, but behavior changes globally.
Verdict: NEEDS_CHANGES
Confidence: high
Summary: The PR addresses the lifecycle leak directionally, but it breaks established Picker ItemsSource synchronization semantics and CI is not green/fully verifiable from this environment. No comments were posted.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix-1 | Handler-lifecycle-only subscription management: preserve unconditional OnItemsSourceChanged subscription for handlerless model sync, but unsubscribe on handler detach and resubscribe/reset on reattach. |
✅ PASS | 1 file | Picker unit tests passed 46/46 and Android UI test VerifyPickerItemsinNavigation passed. Better than PR fix because it preserves existing handlerless Picker.ItemsSource synchronization. |
| PR | PR #33733 | Gate observable ItemsSource subscription on Handler is not null in OnItemsSourceChanged, with reattach resubscribe/reset support. |
✅ PASSED (Gate) | 3 files | Original PR fix; pre-flight review found it regresses handlerless Picker synchronization. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 1 | Yes | Produced Candidate #1, which passed all requested tests and was demonstrably better than the PR fix, satisfying the stop condition before additional candidates were needed. |
Exhausted: No
Selected Fix: Candidate #1 — It passes the Android UI regression test and targeted Picker unit regressions while addressing the PR's lifecycle leak without breaking existing handlerless Picker synchronization behavior.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the current title lacks the component/title formula and the description does not capture the winning fix's key distinction: preserve normal Picker.ItemsSource synchronization while only stopping detached handlers from receiving collection changes.
Recommended title
Picker: Stop detached handlers reacting to ItemsSource changes
Recommended description
<!--
!!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!!
-->
### Issue Details
The issue occurs when a Picker and a CollectionView are both bound to the same ObservableCollection. If you remove an item from the collection using the CollectionView, and that item is currently selected in the Picker, a Picker from a closed page can continue reacting to collection changes and may clear its selection or select the wrong item.
### Description of Change
Picker now tracks its ObservableCollection ItemsSource subscription and disconnects that subscription when its handler is detached, which prevents Pickers on closed pages from reacting to later data changes.
The fix preserves the existing Picker model behavior where ObservableCollection changes still keep Picker.Items synchronized even before a handler is attached. Handler lifecycle management is limited to detach/reattach: unsubscribe when the handler detaches, then resubscribe and reset display items when the handler attaches again.
### Issues Fixed
Fixes #33307
<!--
Are you targeting main? All PRs should target the main branch unless otherwise noted.
-->
**Tested the behavior in the following platforms.**
- [x] Android
- [x] Windows
- [x] iOS
- [x] Mac
| Before | After |
|---------|--------|
| **Android**<br> <video src="https://github.com/user-attachments/assets/c14797c7-a2c5-46a4-a20b-6e71a6977819" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/d2eb0891-4b3f-4528-8117-3f291c9e0029" width="300" height="600"> |
🏁 Report — Final Recommendation
Comparative Report — PR #33733
Candidates compared
| Candidate | Approach | Regression result | Rank |
|---|---|---|---|
try-fix-1 |
Preserve normal OnItemsSourceChanged observable subscription behavior, but unsubscribe on handler detach and resubscribe/reset on reattach. |
✅ Passed 46/46 Picker unit tests and Android UI test VerifyPickerItemsinNavigation. |
1 |
pr-plus-reviewer |
Apply expert Picker feedback to the PR by replacing handler-gated initial subscription with lifecycle-only handler detach/reattach management. The completed expert reviewer also found two broader branch infrastructure issues outside the Picker fix. | Not independently rerun in this phase; Picker implementation is equivalent to try-fix-1, but broader branch findings remain if those infra files are in scope. |
2 |
pr |
Subscribe to observable ItemsSource only when Handler is not null; unsubscribe on detach and resubscribe/reset on attach. |
❌ Gate passed for issue repro, but targeted Picker unit regression failed: handlerless observable ItemsSource no longer updates Picker.Items. |
3 |
Analysis
The raw PR fix addresses the reported page-close leak directionally: once a Picker handler detaches, it unsubscribes from collection changes and no longer reacts to mutations from another page. The problem is the initial subscription gate in OnItemsSourceChanged; by subscribing only when a handler exists, the PR changes non-visual/model behavior for handlerless Pickers. Existing Picker tests rely on Picker.Items tracking an observable ItemsSource even when no handler has been attached, and the recorded targeted regression failed with expected item count 4 but actual 3.
pr-plus-reviewer applies the expert reviewer's actionable Picker implementation feedback and converges on the same product-code strategy as try-fix-1: keep normal ItemsSource collection synchronization, then use handler lifecycle only to stop detached visual Pickers from receiving collection change events. The completed expert reviewer additionally flagged two broader branch infrastructure issues (Get-ReleaseReadiness.ps1 invalid gh pr list field and verify-tests-fail.ps1 simulator retry recovery). Those are outside the Picker candidate comparison, but they remain actionable if these infrastructure changes are included in the PR branch.
try-fix-1 is the strongest candidate because it is both conceptually correct and empirically validated. It preserves existing handlerless Picker synchronization semantics, fixes the detached-handler stale subscription, resynchronizes items on reattach, and passed both the targeted Picker unit suite and the requested Android UI regression.
Winner
Winner: try-fix-1
Rationale: try-fix-1 is the only fully validated candidate that fixes the lifecycle leak without regressing existing handlerless Picker ItemsSource behavior. Per ranking rules, the raw PR must rank below passing candidates because it failed a targeted regression test.
<!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The issue occurs when a Picker and a CollectionView are both bound to the same ObservableCollection. If you remove an item from the collection using the CollectionView, and that item is currently selected in the Picker, the Picker may not clear its selection or may select the wrong item. ### Description of Change <!-- Enter description of the fix in this section --> Picker now unsubscribes from collection change events when its handler is detached, which prevents unwanted updates ### 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 #33307 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Android**<br> <video src="https://github.com/user-attachments/assets/c14797c7-a2c5-46a4-a20b-6e71a6977819" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/d2eb0891-4b3f-4528-8117-3f291c9e0029" width="300" height="600"> |
<!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The issue occurs when a Picker and a CollectionView are both bound to the same ObservableCollection. If you remove an item from the collection using the CollectionView, and that item is currently selected in the Picker, the Picker may not clear its selection or may select the wrong item. ### Description of Change <!-- Enter description of the fix in this section --> Picker now unsubscribes from collection change events when its handler is detached, which prevents unwanted updates ### 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 #33307 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Android**<br> <video src="https://github.com/user-attachments/assets/c14797c7-a2c5-46a4-a20b-6e71a6977819" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/d2eb0891-4b3f-4528-8117-3f291c9e0029" width="300" height="600"> |
<!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The issue occurs when a Picker and a CollectionView are both bound to the same ObservableCollection. If you remove an item from the collection using the CollectionView, and that item is currently selected in the Picker, the Picker may not clear its selection or may select the wrong item. ### Description of Change <!-- Enter description of the fix in this section --> Picker now unsubscribes from collection change events when its handler is detached, which prevents unwanted updates ### 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 #33307 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Android**<br> <video src="https://github.com/user-attachments/assets/c14797c7-a2c5-46a4-a20b-6e71a6977819" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/d2eb0891-4b3f-4528-8117-3f291c9e0029" width="300" height="600"> |
<!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The issue occurs when a Picker and a CollectionView are both bound to the same ObservableCollection. If you remove an item from the collection using the CollectionView, and that item is currently selected in the Picker, the Picker may not clear its selection or may select the wrong item. ### Description of Change <!-- Enter description of the fix in this section --> Picker now unsubscribes from collection change events when its handler is detached, which prevents unwanted updates ### 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 #33307 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Android**<br> <video src="https://github.com/user-attachments/assets/c14797c7-a2c5-46a4-a20b-6e71a6977819" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/d2eb0891-4b3f-4528-8117-3f291c9e0029" width="300" height="600"> |
<!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The issue occurs when a Picker and a CollectionView are both bound to the same ObservableCollection. If you remove an item from the collection using the CollectionView, and that item is currently selected in the Picker, the Picker may not clear its selection or may select the wrong item. ### Description of Change <!-- Enter description of the fix in this section --> Picker now unsubscribes from collection change events when its handler is detached, which prevents unwanted updates ### 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 #33307 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Android**<br> <video src="https://github.com/user-attachments/assets/c14797c7-a2c5-46a4-a20b-6e71a6977819" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/d2eb0891-4b3f-4528-8117-3f291c9e0029" width="300" height="600"> |
<!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The issue occurs when a Picker and a CollectionView are both bound to the same ObservableCollection. If you remove an item from the collection using the CollectionView, and that item is currently selected in the Picker, the Picker may not clear its selection or may select the wrong item. ### Description of Change <!-- Enter description of the fix in this section --> Picker now unsubscribes from collection change events when its handler is detached, which prevents unwanted updates ### 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 #33307 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Android**<br> <video src="https://github.com/user-attachments/assets/c14797c7-a2c5-46a4-a20b-6e71a6977819" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/d2eb0891-4b3f-4528-8117-3f291c9e0029" width="300" height="600"> |
…e PR 36411 (#36470) ### Issue Details PR #33733 fixed a memory/reactivity leak where a Picker would continue reacting to ItemsSource collection changes even after its page was closed (handler detached). The fix gated the INotifyCollectionChanged subscription behind Handler is not null: ``` if (Handler is not null) { SubscribeToItemsSourceCollection(newValue as INotifyCollectionChanged); } ``` This introduced a regression: any Picker without a handler attached never subscribes to collection change events. Unit tests don't attach a platform handler, so setting ItemsSource then adding/removing/clearing items has no effect on picker.Items — causing test failures. ### Description of Change <!-- Enter description of the fix in this section --> Removed the Handler is not null guard in OnItemsSourceChanged. The subscription now always happens when ItemsSource is set. ### 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 #36465 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
<!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The issue occurs when a Picker and a CollectionView are both bound to the same ObservableCollection. If you remove an item from the collection using the CollectionView, and that item is currently selected in the Picker, the Picker may not clear its selection or may select the wrong item. ### Description of Change <!-- Enter description of the fix in this section --> Picker now unsubscribes from collection change events when its handler is detached, which prevents unwanted updates ### 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 #33307 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Android**<br> <video src="https://github.com/user-attachments/assets/c14797c7-a2c5-46a4-a20b-6e71a6977819" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/d2eb0891-4b3f-4528-8117-3f291c9e0029" width="300" height="600"> |
<!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The issue occurs when a Picker and a CollectionView are both bound to the same ObservableCollection. If you remove an item from the collection using the CollectionView, and that item is currently selected in the Picker, the Picker may not clear its selection or may select the wrong item. ### Description of Change <!-- Enter description of the fix in this section --> Picker now unsubscribes from collection change events when its handler is detached, which prevents unwanted updates ### 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 #33307 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Android**<br> <video src="https://github.com/user-attachments/assets/c14797c7-a2c5-46a4-a20b-6e71a6977819" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/d2eb0891-4b3f-4528-8117-3f291c9e0029" width="300" height="600"> |
<!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details The issue occurs when a Picker and a CollectionView are both bound to the same ObservableCollection. If you remove an item from the collection using the CollectionView, and that item is currently selected in the Picker, the Picker may not clear its selection or may select the wrong item. ### Description of Change <!-- Enter description of the fix in this section --> Picker now unsubscribes from collection change events when its handler is detached, which prevents unwanted updates ### 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 #33307 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac | Before | After | |---------|--------| | **Android**<br> <video src="https://github.com/user-attachments/assets/c14797c7-a2c5-46a4-a20b-6e71a6977819" width="300" height="600"> | **Android**<br> <video src="https://github.com/user-attachments/assets/d2eb0891-4b3f-4528-8117-3f291c9e0029" width="300" height="600"> |
Issue Details
The issue occurs when a Picker and a CollectionView are both bound to the same ObservableCollection. If you remove an item from the collection using the CollectionView, and that item is currently selected in the Picker, the Picker may not clear its selection or may select the wrong item.
Description of Change
Picker now unsubscribes from collection change events when its handler is detached, which prevents unwanted updates
Issues Fixed
Fixes #33307
Tested the behavior in the following platforms.
Before.mp4
After.mp4