resolve stack overflow and improve MaskedTextBox behavior

[busitech]

What does the pull request do?

This PR prevents a stack overflow in the binding system when a property is changed during processing of an OnPropertyChanged event handler tied to the same property.

#16746

How was the solution implemented (if it's not obvious)?

The method call at 1. triggers event handlers in user code. When the method finishes, the event e has obsolete values. If sent out, the old values look like a new change, set the value back to what it was, and cause an infinite loop.

AvaloniaObject lines 754-761:

1.          OnPropertyChangedCore(e);

            if (isEffectiveValue)
            {
                property.NotifyChanged(e);
                _propertyChanged?.Invoke(this, e);
                _inpcChanged?.Invoke(this, new PropertyChangedEventArgs(property.Name));
            }

I added a check to see if the value has changed, and if so, to stop sending obsolete new values.

I also enhanced the MaskedTextBox to remember the original empty value, so clearing the control will return back to null or empty string, depending on the original value.

I added a way for the user application to suppress updating the source value as needed. This prevents the control from updating the source until actual keystrokes are received, instead of filling the source with an empty mask value initially (when the initial value is null or empty string). This is the previous behavior of the control before the binding system went through the major refactoring. See #13970

Fixed issues

Fixes #16746

[EvanHaklar]

#16746 (comment)

[cla-avalonia]

• All contributors have signed the CLA.

[grokys]

Thanks for the PR @busitech but I think I have a simpler solution: #17073 🙂

[timunie]

@cla-avalonia agree

[busitech]

@grokys Your change does address the stack overflow successfully.

In our PR, we also wanted to provide a way to restore the behavior of the MaskedTextBox which existed before #13970. Previously, the control did not write back to source unless the origin of the change was user input (keystrokes).

The best example is when the source begins as a null or empty value, which is commonly the case. The text box gets initialized with the an empty mask value (for display only), but the null or empty string value of the source does not need to be replaced, since no data entry has taken place. The write back to source used to be suppressed when SetCurrentValue was called.

After the implementation of #13970 calls to SetCurrentValue always write back to source, the same as editing the Text property, and the control developer appears to have no control over this.

To achieve the previous and desirable behavior of the control, we added a parameter to AvaloniaObject.SetCurrentValue that allows the control developer to decide if write back to source occurs or not. This provides the control developer with the option to write a temporary value to the Text property (for display only), but not for general consumption, or to write a permanent value which should be propagated back to the source.

Adding this parameter was most of the other changes in our PR.

What are your thoughts?

[MrJul]

The binding stack overflow was fixed in #17073.
MaskedTextBox was enhanced with #17143 to avoid out-of-order notifications.
Consequently, I'm going to close this PR.

Updating the binding source conditionally should be discussed in another issue, as it's a whole new feature.

(In my opinion, it's generally hard for a control to know what's "right" in this regard, as different users might expect different outcomes. Always pushing to the source guarantees that the source is aware of all operations. Doing the opposite leave users without any solution, as no notification would come.)