[C] Fix #29459: Switching bindings now triggers propertyChanged correctly #32914
+474
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When dynamically switching bindings on a TwoWay bindable property, the propertyChanged callback was not being fired if the new binding's value matched a previously manually-set value that was still cached. Root cause: In SetBinding(), when moving the current value to FromBinding specificity, stale ManualValueSetter entries were not cleared. This caused SetValueActual to compare against the wrong baseline value. Fix: Clear ManualValueSetter entries when switching bindings to ensure proper value comparison. Added unit tests in both Xaml.UnitTests and Core.UnitTests.
StephaneDelcroix
changed the title
Copilot finished reviewing on behalf of
StephaneDelcroix
November 29, 2025 05:35
Contributor
There was a problem hiding this comment.
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 clearManualValueSetterentries 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 |
Comment on lines
240
to
241
| // Note: PropertyChanged may or may not fire when values are equal - this is implementation-dependent | ||
| // The key assertion is that the Value is correct |
There was a problem hiding this comment.
[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:
- Removing the comment and keeping just the value assertion, OR
- 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. |
Add the #pragma warning disable CS0219 line to expected output that was added to the source generator in commit 9b3b826.
mattleibow
previously approved these changes
Dec 2, 2025
| page.MyControl.SetBinding(Maui29459CustomControl.ValueProperty, nameof(Maui29459ViewModel.B)); | ||
|
|
||
| Assert.That(page.MyControl.Value, Is.EqualTo(50), "Value should remain 50"); | ||
| // Note: PropertyChanged may or may not fire when values are equal - this is implementation-dependent |
Member
There was a problem hiding this comment.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
propertyChangedcallback 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):
propertyChangednot called, internal ViewModel stays at 0Root cause
In
BindableObject.SetBinding(), when a new binding is applied:FromBindingspecificityManualValueSetterentries (from code likecontrol.Value = newValue) were NOT clearedSetValueActualcompared values, it used the highest-specificity value (the staleManualValueSetter)sameValuewastrueandpropertyChangeddidn't fireFix
Clear
ManualValueSetterentries when switching bindings to ensure proper value comparison.Changes
ManualValueSetterentries when setting a new bindingTesting