dotnet / wpf

WPF is a .NET Core UI framework for building Windows desktop applications.
MIT License
7.1k stars 1.17k forks source link

One-Way Binding immediately lost when the target Dependency Property is internally updated #9910

Open amine2050 opened 1 month ago

amine2050 commented 1 month ago

Description

Hi,

I've been writing advanced WPF stuff for the last 8 years. This is the first time I notice this odd behavior of One-Way bindings in such a basic use case:

The One-Way binding is cleared by the WPF engine when the target Dependency Property (in a custom Control/UserControl) is internally updated (in code or via internal binding).

I prepared a very simple sample to reproduce that, to which I added a BindingObserver control that continuously outputs the state of the BindingExpression on the UI.

Here is the sample: https://github.com/amine2050/wpf-lost-binding

Am I missing a basic knowledge of how One-Way bindings behave? Or is this a known issue? I could not find any literature about the topic.

Reproduction Steps

Expected behavior

One-way binding to ViewModel is expected to continue working even after changing the value of the DependencyProperty (by changing TextBox content).

The new content of the TextBox should not propagate to the binding Source (which is correct here), but changes in the Source are expected to be reflected on the TextBox (this does not happen anymore).

I want the same behavior of a simple TextBox bound to ViewModel using One-Way binding.

Actual behavior

One-way binding to ViewModel has been cleared by the WPF engine without any warnings or output. Changes in ViewModel are no loger propagating to UI.

Regression?

I tested this with .NET 8.0 and .NET Framework 7.2.1 and got the same behavior. So no regression is known.

Known Workarounds

The only workaround I found is to use Two-Way binding instead of One-Way. But this is not necessarily what I want actually.

Impact

This applies to any custom Control/UserControl that has properties you want to change internally while allowing binding them using One-Way binding.

Configuration

Other information

I added PresentationTraceSources.TraceLevel=High to the binding and got the following output when the binding gets broken:

System.Windows.Data Warning: 79 : BindingExpression (hash=53180767): Deactivate
System.Windows.Data Warning: 103 : BindingExpression (hash=53180767): Replace item at level 0 with {NullDataItem}
System.Windows.Data Warning: 63 : BindingExpression (hash=53180767): Detach
h3xds1nz commented 1 month ago

The UserControl property needs to have OneWay mode as well.

amine2050 commented 1 month ago

The UserControl property needs to have OneWay mode as well.

Sorry, can you explain what you mean?

h3xds1nz commented 1 month ago

Sorry, I went with 8 years of professional WPF experience but probably also accounted for telepathy:

In MyCustomInput.xaml, adjust the binding to TextBox.Text property with Mode=OneWay.

https://source.dot.net/#PresentationFramework/System/Windows/Controls/TextBox.cs,666 Text property binds TwoWay by default, so I think you know what happens there.

If you set UpdateSourceTrigger=PropertyChanged on the binding and type into the TextBox without setting it as OneWay, you will notice the binding is thrown away instantly and has nothing to do with you updating the backing property programmatically.

amine2050 commented 1 month ago

Thanks for explaining! 😅 telepathy didn't work as expected

The TwoWay binding between TextBox.Text and MyCustomInput.Value was intended.

That is my "internal technique" inside of MyCustomInput to update Value. I want Value to be internally synchronized with the TextBox, but not necessarily with the external consumers (ViewModel). I even tried updating Value manually in code (without using a TwoWay binding), but got same "broken binding" behavior.

Using PropertyChangedor on LostFocus as trigger does not matter, you'll get the binding broken as soon as you try to update the DependencyProperty Value.

Needless to say this is just a sample to reproduce the issue, not my productive code, so it does not make a lot of sense on first sight.

h3xds1nz commented 1 month ago

I just wanted to show you with the trigger that you updating the property via code-behind has no effect on whether it breaks or not (so the "reproduction" steps could have been simplified).

Your "internal technique" has a basic design flaw if I understand you correctly, and that's you wanting to have TwoWay (TextBox.Text<->MyCustomInput.Value) binding on one side but OneWay (MyCustomInput.Value<-TheValue) onto the other side with one property. That's not really OneWay if you look at it from the property perspective, is it.

You're gonna need two dependency properties (one for each binding, TwoWay for TextBox, OneWay for your case) and handle the "OneWay" synchronization via SetValue inside PropertyChangedCallback on the other, that's the best I can think of rn for this. At least you didn't want generics 😀

amine2050 commented 1 month ago

What you understood is correct: In my sample, I want to have this: TextBox.Text<->MyCustomInput.Value <- TheValue

However, the internal binding (TextBox.Text<->MyCustomInput.Value) does not matter, it is just how MyCustomInput.Value gets updated in the example.

My requirement in more simple words:

I have a custom Control/UserControl with an exposed DependencyProperty Value that may change its value by some internal logic. As a consumer of that control, I want to bind Value to my ViewModel (or other DependencyProperty) using OneWay binding, so that changes in my VM are reflected in the Control, but changes from the control itself do not propagate back to my ViewModel.

The predefined TextBox in WPF perfectly fulfills this simple requirement with its DP Text. However when trying to implement this on my own in a straightforward way, the first change on the DP breaks the binding silently!

Is the TextBox doing some special magic to prevent breaking the binding?

Is this a desired behavior of WPF OneWay binding? Any documentation/literature?

(PS: I'll give it a try later with 2 DPs, but I don't think that would make a difference, I already tried setting the DP value in code instead of the binding, no chance)

h3xds1nz commented 1 month ago

There's no magic, the difference is that your VM property is not a DependencyProperty but a raw CLR POCO, you're mixing up binding DP-DP and DP-POCO.

Here's the working example for your desired effect, this way TextBox is living its life, changes propagated into TextBound and once VMBound changes, it is propagated into TextBound/TextBox.Text. For clarity I've noted the binding modes in markup and named the properties accordingly.

MyCustomInput.xaml.cs:

public static readonly DependencyProperty VMBoundProperty = DependencyProperty.Register(
  nameof(VMBound), typeof(string), typeof(MyCustomInput),
  new FrameworkPropertyMetadata(default(string), (d, e) => { d.SetValue(TextBoundProperty, e.NewValue); Debug.WriteLine($"VM-Old: {e.OldValue}"); Debug.WriteLine($"VM-New: {e.NewValue}"); }));

public string VMBound
{
    get => (string)GetValue(VMBoundProperty); 
    set => SetValue(VMBoundProperty, value); 
}

public static readonly DependencyProperty TextBoundProperty = DependencyProperty.Register(
  nameof(TextBound), typeof(string), typeof(MyCustomInput),
  new FrameworkPropertyMetadata(default(string), (d, e) => { Debug.WriteLine($"Text-Old: {e.OldValue}"); Debug.WriteLine($"Text-New: {e.NewValue}"); }));

public string TextBound
{
    get => (string)GetValue(TextBoundProperty);
    set => SetValue(TextBoundProperty, value);
}

MyCustomInput.xaml:

<TextBox Text="{Binding ElementName=UserControl, Path=TextBound, UpdateSourceTrigger=PropertyChanged, Mode=TwoWay}"/>

MyView.xaml:

<lostBindingSample:MyCustomInput x:Name="MyCustomInput" VMBound="{Binding TheValue, Mode=OneWay}"/>
amine2050 commented 1 month ago

Thanks a lot for the working example! I see now what you mean by using 2 DPs. You propagate the OneWay binding manually into the "internal" DP, I first thought you meant the other way around, which would cause the same issue. Got that!

I consider this proposition an alternative workaround for the problem.

The requirement is having one single exposed DP that can be bound either OneWay or TwoWay (sorry didn't mention the latter explicitly) depending on the consumer of the control, just like the WPF TextBox.Text property.

I am more concerned about understanding the theoretical root cause of why the OneWay binding breaks and if it is a documented expected behavior, than finding a workaround for my current problem.

amine2050 commented 1 month ago

A very interesting point about TextBox when binding Text DP using OneWay binding:

When you edit the text via TextBox UI, the property Text gets instantly updated without breaking the binding. However, when you try setting the TextBox.Text value via code, the same "broken binding" behavior occurs.

This is what I called previously "some special magic". I'll try to dig into TextBox code to find out how that works.

h3xds1nz commented 1 month ago

Don't dig into TextBox but dig into BindingExpression documentation because setting the value explicitly breaks this.

And possibly into understanding the differences between SetValue and SetCurrentValue (and why do they exist).

Same as what happens when you attempt to have a single DependencyProperty operate in two exclusive modes.

And one more EDIT: It breaks because the expression tries to assign a value to DependencyProperty in OneWay mode, which fails, and therefore it detaches.

amine2050 commented 1 month ago

Thanks for bringing up SetCurrentValue(), I just dug into TextBox code and found out it is internally using SetCurrentValue() (actually an internal SetCurrentDeferredValue()).

To summarize (correct me if I am wrong):

That explains that this Binding chain has a "design flow" (as you mentioned): TextBox.Text <-> MyCustomInput.Value <- TheValue

So, either this: TextBox.Text <- MyCustomInput.Value <- TheValue

or this: TextBox.Text <-> MyCustomInput.Value <-> TheValue

If all of this is correct, this introduces new rules to write custom Control/UserControl that does not accidentally break eventual OneWay bindings when used:

Unfortunately, these rules, despite being rational, seems to be unnatural and would complicate writing custom controls and bloat the code.

Alternatively, we do not introduce any new rules. Instead of that we make our WPF developers aware, that OneWay bindings are "easy to break" and should be used with caution only with "pure input" DPs.

Actually, I barely use OneWay bindings on properties that might change from inside the control, OneWay bindings are normally for "read only" information that you feed your control with.

h3xds1nz commented 1 month ago

What a nice summary you've made, this would be pretty much a good answer to someone. Unluckily I don't have that gift, haha.

Yeah, based on your models, there's also OneWayToSource that does "->" as you surely know (which has also its funny but expected quirk, yet may cause confusion for some from my experience: if you have a default value on ur POCO, then while it globs on that value, it will be replaced by whatever the DP has right after init, so there's no way to ever retrieve that default value on this specific binding without replacing it).

SetValue indeed works that way, the TwoWay binding will survive, which may cause confusion.

I do not agree with the "new rules", you should be aware how binding expressions work and what may break it.

Imho what should be said is; use SetCurrentValue if you're touching any DP (exposed) externally (unless you intend to replace/update binding), and preferrably use SetValue internally if value update is what you're after.

The main takeaway is, don't expect one DP to work in two exclusive modes as it was expected in this case. Binding between DPs is not the same as binding between POCO property and a DP:

amine2050 commented 1 month ago

I would also say, the awareness of the "breakability" of OneWay bindings is the way to go. Although it is a delicate topic and not easy to fully explain to newbies.

I like always to have clear best practices to "enforce" within my team to avoid such "silent" problems. But I think this is not the case.

To close up our discussion:

WPF bindings have the following behaviors that should be known by developers

Although these behaviors are somehow confusing and can theoretically be improved, I see nothing that may be touched here in the deepest WPF codes. We do not want to break half the world again 🤣

Nevertheless, I suggest adding a binding Error/Warning that can be seen in Output and in the "XAML Binding Failures" window in case a OneWay binding has been broken/detached by a TwoWay binding update. Probably also when it gets detached by using SetProperty() (maybe just a Warning here).

Such an Error/Warning would be very helpful for every developer struggling with such broken bindings. Since we have no clear best practices to 100% eliminate the risk.

What do you think?

h3xds1nz commented 1 month ago

The tracing could be theoretically improved as to reason why the detach has happened. Touching anything around that is a thin ice though, it still uses goto statements, haha.

Documentation could be improved as to what is by design, I agree.