Fix #29459: Switching bindings now triggers propertyChanged correctly

[StephaneDelcroix]

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!

Description

Fixes #29459

When dynamically switching bindings on a TwoWay bindable property (e.g., switching between binding to property A and property B), the propertyChanged callback was not being fired if the new binding's value matched a previously manually-set value that was still cached in the property context.

Reproduction scenario (from issue):

1. Bind control to property A (value = 0)
2. Switch binding to property B (value = 100)
3. Modify B via control's internal ViewModel (value = 101) - TwoWay binding syncs back
4. Switch back to property A (value = 0)
5. Switch back to property B (value = 101) - BUG: propertyChanged not called, internal ViewModel stays at 0

Root cause

In BindableObject.SetBinding(), when a new binding is applied:

1. The current value is moved from its current specificity to FromBinding specificity
2. BUT stale ManualValueSetter entries (from code like control.Value = newValue) were NOT cleared
3. When SetValueActual compared values, it used the highest-specificity value (the stale ManualValueSetter)
4. If the new binding's value equaled this stale value, sameValue was true and propertyChanged didn't fire

Fix

Clear ManualValueSetter entries when switching bindings to ensure proper value comparison.

Changes

• src/Controls/src/Core/BindableObject.cs: Added code to remove stale ManualValueSetter entries when setting a new binding
• src/Controls/tests/Xaml.UnitTests/Issues/Maui29459.xaml/.cs: Added XAML unit test reproducing the issue
• src/Controls/tests/Core.UnitTests/BindingUnitTests.cs: Added Core unit tests for the fix

Testing

• All 5422 Controls.Core.UnitTests pass
• All 1770 Controls.Xaml.UnitTests pass
• Manually verified with the original reproduction project updated to .NET 10

[Copilot]

Pull request overview

This pull request fixes issue #29459 where dynamically switching TwoWay bindings on a bindable property failed to trigger propertyChanged callbacks when the new binding's value matched a stale ManualValueSetter entry.

Key Changes

• Core fix: Modified BindableObject.SetBinding() to explicitly clear ManualValueSetter entries when setting a new binding, ensuring clean state and proper value comparison
• Comprehensive test coverage: Added both XAML-based and Core unit tests that reproduce the exact scenario from the bug report
• Test validation: All existing tests pass (5422 Controls.Core.UnitTests, 1770 Controls.Xaml.UnitTests)

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/Controls/src/Core/BindableObject.cs	Added defensive code to remove stale ManualValueSetter entries when switching bindings, preventing incorrect value comparison
src/Controls/tests/Xaml.UnitTests/Issues/Maui29459.xaml	Added minimal XAML page with custom control for test infrastructure
src/Controls/tests/Xaml.UnitTests/Issues/Maui29459.xaml.cs	Added comprehensive XAML unit tests reproducing the issue with internal ViewModel synchronization pattern
src/Controls/tests/Core.UnitTests/BindingUnitTests.cs	Added Core unit tests mirroring the XAML tests to ensure fix works at the framework level

[Copilot]

[nitpick] The comment on line 240 states "PropertyChanged may or may not fire when values are equal", but then the test only asserts that the value is correct without checking the PropertyChangedCount behavior. This makes the test's assertion incomplete for documenting the actual behavior. Consider either:

1. Removing the comment and keeping just the value assertion, OR
2. Adding an explicit assertion about PropertyChangedCount behavior (e.g., // PropertyChangedCount behavior is implementation-defined when values are equal)

Suggested change

	// Note: PropertyChanged may or may not fire when values are equal - this is implementation-dependent
	// The key assertion is that the Value is correct
	// PropertyChangedCount behavior is implementation-defined when values are equal; no assertion is made.

[mattleibow]

The test name is "..DoesNotTriggerPropertyChanged" yet this comment says "may or may not fire". Which is the correct one?

// FROM:
public void SwitchingBindingToSameValueDoesNotTriggerPropertyChanged([Values] XamlInflator inflator)

// TO:
public void SwitchingBindingToSameValueMaintainsCorrectValue([Values] XamlInflator inflator)

// FROM:
// PropertyChanged should NOT fire (optimization)

// TO:
// The value should be correct regardless of PropertyChanged firing behavior

[github-actions]

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 32914

Or

• Run remotely in PowerShell:

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

[MauiBot]

⚠️ Merge Conflict Detected — This PR has merge conflicts with its target branch. Please rebase onto the target branch and resolve the conflicts.

[StephaneDelcroix]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[MauiBot]

🚦 Gate - Test Before and After Fix

📊 Expand Full Gate — 55c4cb8 · Fix CS0103: qualify internal types in binding test

---

Gate Result: ❌ FAILED

Platform: android

---

[MauiBot]

🤖 AI Summary

📊 Expand Full Review — 55c4cb8 · Fix CS0103: qualify internal types in binding test

---

🔍 Pre-Flight — Context & Validation

---

Issue: #29459 - Dynamically switching binding to a custom control does not trigger property changed event of the control
PR: #32914 - Fix #29459: Switching bindings now triggers propertyChanged correctly
Platforms Affected: Android, Windows
Files Changed: 1 implementation (BindableObject.cs), 3 test (BindingUnitTests.cs, Maui29459.xaml, Maui29459.xaml.cs)

Key Findings

• Bug: When dynamically switching TwoWay bindings, propertyChanged callback was not fired if the new binding's value matched a stale ManualValueSetter value cached in the property context
• Root cause: In BindableObject.SetBinding(), stale ManualValueSetter entries were not cleared when switching bindings. SetValueActual then used the highest-specificity value (the stale ManualValueSetter) for comparison, causing sameValue = true and suppressing propertyChanged
• Fix: In SetBinding(), before moving the current value to FromBinding specificity, remove any ManualValueSetter entry from context.Values (unless the current specificity IS the ManualValueSetter)
• PR also includes unrelated changes: refactoring Application.Current?.FindMauiContext()?.CreateLogger<>() to MauiLogger<T>.Log(), switching from dictionary indexer [] to SetValue() method on SetterSpecificityList, and lazy initialization of Bindings/Values/_triggerSpecificity
• Gate ❌ FAILED on android — tests did NOT behave as expected (likely: unit tests misclassified as Android tests by gate detector, or compilation issue on PR branch)
• Review comment from @mattleibow: test name SwitchingBindingToSameValueDoesNotTriggerPropertyChanged contradicts the comment "may or may not fire"; suggests renaming to SwitchingBindingToSameValueMaintainsCorrectValue — PR author committed a later fix addressing this with "Fix CS0103: qualify internal types in binding test"
• Tests are xUnit unit tests (Core.UnitTests + Xaml.UnitTests), not UI tests or device tests
• The XAML unit test uses Maui29459CustomControl which mirrors the exact reproduction scenario from the issue

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #32914	Remove ManualValueSetter from context.Values in SetBinding() before applying new binding	❌ Gate FAILED	BindableObject.cs	Original PR fix

---

🔧 Fix — Analysis & Comparison

---

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix (claude-opus-4.6)	In SetValueActual, remove ManualValueSetter when FromHandler arrives and it's the top entry	✅ PASS	1 file	Reactive fix — mirrors existing FromHandler cleanup pattern
2	try-fix (claude-sonnet-4.6)	In SetBinding, while-loop removes ALL entries above FromBinding	✅ PASS	1 file	Most general — handles all stale specificity entries
3	try-fix (gpt-5.3-codex)	In SetValueActual, normalize ManualValueSetter writes to FromHandler	❌ FAIL	—	Blocked by unrelated RS0016 PublicAPI errors
4	try-fix (gpt-4.1, gemini unavailable)	In SetValueActual, skip ManualValueSetter for sameValue when applying binding	❌ BLOCKED	—	Blocked by unrelated RS0016 PublicAPI errors
5	try-fix (claude-sonnet-4.6, round 2)	In SetValueActual, compare against binding-layer slot instead of stale ManualValueSetter value	✅ PASS	1 file	Comparison redirect; avoids stack mutation
6	try-fix (gpt-5.3-codex, round 2)	Per-context force-notify flag in BindablePropertyContext, consumed in SetValueActual	❌ FAIL	—	Blocked by unrelated RS0016 PublicAPI errors
PR	PR #32914	In SetBinding, explicitly remove ManualValueSetter if currentSpecificity != ManualValueSetter	❌ Gate FAILED	BindableObject.cs	Original PR fix

Cross-Pollination

Model	Round	New Ideas?	Details
claude-opus-4.6	2	No	All viable locations covered
claude-sonnet-4.6	2	Yes	→ Attempt 5 (compare against binding-layer slot)
gpt-5.3-codex	2	Yes	→ Attempt 6 (force-notify flag)
gpt-4.1	2	No	No new ideas
claude-sonnet-4.6	3	Yes	BindingBase.Apply + RaiseOnEqual — risk of behavioral regression; not pursued

Exhausted: Yes (3 rounds complete, 3 passing candidates found)
Selected Fix: Candidate #2 — while-loop in SetBinding() removes all stale entries above FromBinding. More general and robust than the PR's explicit ManualValueSetter check. Simpler than Attempt 5's comparison-redirect approach.

---

📋 Report — Final Recommendation

---

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase	Status	Notes
Pre-Flight	✅ COMPLETE	Issue #29459, 1 impl file + 3 test files
Gate	❌ FAILED	android — tests did NOT behave as expected
Try-Fix	✅ COMPLETE	6 attempts, 3 passing alternatives found
Report	✅ COMPLETE

Summary

PR #32914 fixes a real bug (#29459) where switching TwoWay bindings suppresses propertyChanged when the new binding's value matches a stale ManualValueSetter entry. The underlying fix concept is correct — clearing stale ManualValueSetter entries on binding switch. However:

1. Gate failed: The PR's test suite did not reproduce and verify the fix as expected on the android platform
2. A better alternative exists: The try-fix exploration found a cleaner, more general implementation (Candidate Update README.md #2: while-loop removes all entries above FromBinding)
3. Review comments are unaddressed: @mattleibow raised concerns about the test naming inconsistency (SwitchingBindingToSameValueDoesNotTriggerPropertyChanged vs. the comment saying "may or may not fire"). While the author pushed a compile fix, the test naming concern remains
4. PR scope vs. description mismatch: The PR description states changes to 4 files, but the branch also contains unrelated changes: logging refactoring (MauiLogger<T>.Log), SetterSpecificityList.SetValue() method (replacing [] indexer), and lazy initialization of Bindings/Values/_triggerSpecificity

Root Cause

In BindableObject.SetBinding(), when switching from an old binding to a new one, the code correctly moves the current value to FromBinding specificity, but does not clear stale ManualValueSetter entries left over from TwoWay write-backs. When the new binding applies in SetValueActual, original is read from the stale ManualValueSetter (the highest specificity), and sameValue = true suppresses propertyChanged.

Fix Quality

PR's fix: Adds context.Values.Remove(SetterSpecificity.ManualValueSetter) guarded by if (currentSpecificity != SetterSpecificity.ManualValueSetter). This is correct in concept but fragile: it only handles ManualValueSetter and does not account for other potential stale entries that could accumulate above FromBinding.

Better alternative (Candidate #2):

// Remove ALL entries above FromBinding (not just ManualValueSetter) to prevent
// stale intermediate entries from causing false sameValue=true on binding switch (#29459).
while (context.Values.GetSpecificity() > SetterSpecificity.FromBinding)
    context.Values.Remove(context.Values.GetSpecificity());

This is more defensive, simpler to reason about, and future-proof.

Test quality issues:

• Core.UnitTests adds tests for issue MVVM binding doen't work properly for int types #8342 (unrelated to this PR's stated fix)
• XAML test SwitchingBindingToSameValueDoesNotTriggerPropertyChanged has contradictory documentation (comment says "may or may not fire" but test name says "DoesNotTrigger")
• The XAML test's Maui29459CustomControl must be public (not internal) due to XAML namespace requirements, which is fine

---