From db10780e15c1de4c4d02028b2ce3dfc4a4ace7ab Mon Sep 17 00:00:00 2001
From: Julien Lebosquain <julien@lebosquain.net>
Date: Tue, 1 Oct 2024 00:28:53 +0200
Subject: [PATCH] Use coercion for MaskedTextBox.Text (#17143)

---
 src/Avalonia.Controls/MaskedTextBox.cs        | 37 +++++++++----------
 src/Avalonia.Controls/TextBox.cs              | 20 +++++++---
 .../MaskedTextBoxTests.cs                     | 37 ++++++++++++++++++-
 3 files changed, 69 insertions(+), 25 deletions(-)

diff --git a/src/Avalonia.Controls/MaskedTextBox.cs b/src/Avalonia.Controls/MaskedTextBox.cs
index 7278bb17c9a..a483f2758a1 100644
--- a/src/Avalonia.Controls/MaskedTextBox.cs
+++ b/src/Avalonia.Controls/MaskedTextBox.cs
@@ -4,10 +4,7 @@
 using System.Globalization;
 using System.Linq;
 using Avalonia.Input;
-using Avalonia.Input.Platform;
 using Avalonia.Interactivity;
-using Avalonia.Styling;
-using Avalonia.VisualTree;
 
 namespace Avalonia.Controls
 {
@@ -82,8 +79,8 @@ public MaskedTextBox() { }
         /// <summary>
         ///  Constructs the MaskedTextBox with the specified MaskedTextProvider object.
         /// </summary>
-        [System.Diagnostics.CodeAnalysis.SuppressMessage("AvaloniaProperty", 
-            "AVP1012:An AvaloniaObject should use SetCurrentValue when assigning its own StyledProperty or AttachedProperty values", 
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("AvaloniaProperty",
+            "AVP1012:An AvaloniaObject should use SetCurrentValue when assigning its own StyledProperty or AttachedProperty values",
             Justification = "These values are being explicitly provided by a constructor parameter.")]
         public MaskedTextBox(MaskedTextProvider maskedTextProvider)
         {
@@ -305,20 +302,7 @@ void UpdateMaskProvider()
                 }
                 RefreshText(MaskProvider, 0);
             }
-            if (change.Property == TextProperty && MaskProvider != null && _ignoreTextChanges == false)
-            {
-                if (string.IsNullOrEmpty(Text))
-                {
-                    MaskProvider.Clear();
-                    RefreshText(MaskProvider, CaretIndex);
-                    base.OnPropertyChanged(change);
-                    return;
-                }
-
-                MaskProvider.Set(Text);
-                RefreshText(MaskProvider, CaretIndex);
-            }
-            else if (change.Property == MaskProperty)
+            if (change.Property == MaskProperty)
             {
                 UpdateMaskProvider();
 
@@ -445,5 +429,20 @@ private void RefreshText(MaskedTextProvider? provider, int position)
             }
         }
 
+        /// <inheritdoc />
+        protected override string? CoerceText(string? text)
+        {
+            if (!_ignoreTextChanges && MaskProvider is { } maskProvider)
+            {
+                if (string.IsNullOrEmpty(text))
+                    maskProvider.Clear();
+                else
+                    maskProvider.Set(text);
+
+                text = maskProvider.ToDisplayString();
+            }
+
+            return base.CoerceText(text);
+        }
     }
 }
diff --git a/src/Avalonia.Controls/TextBox.cs b/src/Avalonia.Controls/TextBox.cs
index aa8c239d27e..458040ee7d4 100644
--- a/src/Avalonia.Controls/TextBox.cs
+++ b/src/Avalonia.Controls/TextBox.cs
@@ -568,19 +568,29 @@ public string? Text
         }
 
         private static string? CoerceText(AvaloniaObject sender, string? value)
-        {
-            var textBox = (TextBox)sender;
+            => ((TextBox)sender).CoerceText(value);
 
+        /// <summary>
+        /// Coerces the current text.
+        /// </summary>
+        /// <param name="value">The initial text.</param>
+        /// <returns>A coerced text.</returns>
+        /// <remarks>
+        /// This method also manages the internal undo/redo state whenever the text changes:
+        /// if overridden, ensure that the base is called or undo/redo won't work correctly.
+        /// </remarks>
+        protected virtual string? CoerceText(string? value)
+        {
             // Before #9490, snapshot here was done AFTER text change - this doesn't make sense
-            // since intial state would never be no text and you'd always have to make a text 
+            // since initial state would never be no text and you'd always have to make a text
             // change before undo would be available
             // The undo/redo stacks were also cleared at this point, which also doesn't make sense
             // as it is still valid to want to undo a programmatic text set
             // So we snapshot text now BEFORE the change so we can always revert
             // Also don't need to check IsUndoEnabled here, that's done in SnapshotUndoRedo
-            if (!textBox._isUndoingRedoing)
+            if (!_isUndoingRedoing)
             {
-                textBox.SnapshotUndoRedo();
+                SnapshotUndoRedo();
             }
 
             return value;
diff --git a/tests/Avalonia.Controls.UnitTests/MaskedTextBoxTests.cs b/tests/Avalonia.Controls.UnitTests/MaskedTextBoxTests.cs
index 56f131b410a..e467048e1d1 100644
--- a/tests/Avalonia.Controls.UnitTests/MaskedTextBoxTests.cs
+++ b/tests/Avalonia.Controls.UnitTests/MaskedTextBoxTests.cs
@@ -884,7 +884,42 @@ public void Keys_Allow_Undo(Key key, KeyModifiers modifiers)
 
                 RaiseKeyEvent(target, key, modifiers);
                 RaiseKeyEvent(target, Key.Z, KeyModifiers.Control); // undo
-                Assert.True(target.Text == "0123");
+                Assert.Equal("0123", target.Text);
+            }
+        }
+
+        [Fact]
+        public void Invalid_Text_Is_Coerced_Without_Raising_Intermediate_Change()
+        {
+            using (Start())
+            {
+                var target = new MaskedTextBox
+                {
+                    Template = CreateTemplate()
+                };
+
+                var impl = CreateMockTopLevelImpl();
+                var topLevel = new TestTopLevel(impl.Object) {
+                    Template = CreateTopLevelTemplate(),
+                    Content = target
+                };
+                topLevel.ApplyTemplate();
+                topLevel.LayoutManager.ExecuteInitialLayoutPass();
+
+                var texts = new List<string>();
+
+                target.PropertyChanged += (_, e) =>
+                {
+                    if (e.Property == TextBox.TextProperty)
+                        texts.Add(e.GetNewValue<string>());
+                };
+
+                target.Mask = "000";
+
+                target.Text = "123";
+                target.Text = "abc";
+
+                Assert.Equal(["___", "123"], texts);
             }
         }