From 94f458da0e178fc19d7f52db45b426ece6a553c5 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen Date: Sat, 25 Jul 2026 07:03:02 +1000 Subject: [PATCH] fix(TextBox): clear undo history on external text updates (#21839) * test(TextBox): reproduce stale undo history after binding update TwoWay-bound TextBox instances retain prior user-edit snapshots when the binding source replaces the text, allowing undo to write values from a previous model context back into the current source. Cover the external replacement and the normal TwoWay source echo separately so a fix cannot clear valid user undo history after each edit. * fix(TextBox): reset undo history for external text updates Binding and direct property updates currently enter the same undo timeline as user edits, so a reused TwoWay-bound TextBox can write text from a previous model back into the current source. Track TextBox-initiated mutations through synchronous source notifications, preserve those edits in the undo timeline, and establish a fresh baseline for all external updates. MaskedTextBox routes its own edits through the same boundary so its existing undo behavior remains intact. * fix(TextBox): distinguish edits from internal synchronization in undo history Replace the boolean text-mutation flag with a three-state TextMutationKind (ExternalReplacement / Edit / InternalSynchronization) so MaskedTextBox's prompt-character reformatting on focus/blur and mask-provider refreshes no longer create undo entries, while actual keyboard edits remain undoable. --- src/Avalonia.Controls/MaskedTextBox.cs | 19 ++- src/Avalonia.Controls/TextBox.cs | 91 ++++++++++--- .../MaskedTextBoxTests.cs | 121 ++++++++++++++++++ .../TextBoxTests.cs | 67 ++++++++++ 4 files changed, 273 insertions(+), 25 deletions(-) diff --git a/src/Avalonia.Controls/MaskedTextBox.cs b/src/Avalonia.Controls/MaskedTextBox.cs index 22c05e7159..756eb31b12 100644 --- a/src/Avalonia.Controls/MaskedTextBox.cs +++ b/src/Avalonia.Controls/MaskedTextBox.cs @@ -190,7 +190,7 @@ namespace Avalonia.Controls { if (HidePromptOnLeave == true && MaskProvider != null) { - SetCurrentValue(TextProperty, MaskProvider.ToDisplayString()); + SetTextFromInternalSynchronization(MaskProvider.ToDisplayString()); } base.OnGotFocus(e); } @@ -241,7 +241,7 @@ namespace Avalonia.Controls } } - SetCurrentValue(TextProperty, MaskProvider.ToDisplayString()); + SetTextFromEdit(MaskProvider.ToDisplayString()); e.Handled = true; return; } @@ -291,7 +291,7 @@ namespace Avalonia.Controls { if (HidePromptOnLeave && MaskProvider != null) { - SetCurrentValue(TextProperty, MaskProvider.ToString(!HidePromptOnLeave, true)); + SetTextFromInternalSynchronization(MaskProvider.ToString(!HidePromptOnLeave, true)); } base.OnLostFocus(e); } @@ -306,7 +306,7 @@ namespace Avalonia.Controls { MaskProvider.Set(Text); } - RefreshText(MaskProvider, 0); + RefreshText(MaskProvider, 0, isEdit: false); } if (change.Property == MaskProperty) { @@ -426,11 +426,18 @@ namespace Avalonia.Controls return startPosition; } - private void RefreshText(MaskedTextProvider? provider, int position) + private void RefreshText(MaskedTextProvider? provider, int position, bool isEdit = true) { if (provider != null) { - SetCurrentValue(TextProperty, provider.ToDisplayString()); + if (isEdit) + { + SetTextFromEdit(provider.ToDisplayString()); + } + else + { + SetTextFromInternalSynchronization(provider.ToDisplayString()); + } SetCurrentValue(CaretIndexProperty, position); } } diff --git a/src/Avalonia.Controls/TextBox.cs b/src/Avalonia.Controls/TextBox.cs index 0f2e3d51ce..408e837043 100644 --- a/src/Avalonia.Controls/TextBox.cs +++ b/src/Avalonia.Controls/TextBox.cs @@ -362,11 +362,21 @@ namespace Avalonia.Controls public override int GetHashCode() => Text?.GetHashCode() ?? 0; } + private enum TextMutationKind + { + ExternalReplacement, + Edit, + InternalSynchronization, + } + private TextPresenter? _presenter; private ScrollViewer? _scrollViewer; private readonly TextBoxTextInputMethodClient _imClient = new(); private readonly UndoRedoHelper _undoRedoHelper; private bool _isUndoingRedoing; + private TextMutationKind _textMutationKind; + // Coercion runs before the new value is committed, so a snapshot taken there would capture the old text. + private bool _needsUndoRedoSnapshotAfterTextChange; private bool _canCut; private bool _canCopy; private bool _canPaste; @@ -641,16 +651,29 @@ namespace Avalonia.Controls /// protected virtual string? CoerceText(string? value) { - // Before #9490, snapshot here was done AFTER text change - this doesn't make sense - // 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 (!_isUndoingRedoing) { - SnapshotUndoRedo(); + switch (_textMutationKind) + { + case TextMutationKind.Edit: + SnapshotUndoRedo(); + + if (!_undoRedoHelper.CanUndo && + !string.Equals(Text, value, StringComparison.Ordinal)) + { + _needsUndoRedoSnapshotAfterTextChange = true; + } + break; + + case TextMutationKind.InternalSynchronization: + break; + + case TextMutationKind.ExternalReplacement: + default: + ClearUndoRedo(); + _needsUndoRedoSnapshotAfterTextChange = true; + break; + } } return value; @@ -878,9 +901,7 @@ namespace Avalonia.Controls // from docs at // https://docs.microsoft.com/en-us/dotnet/api/system.windows.controls.primitives.textboxbase.isundoenabled: // "Setting UndoLimit clears the undo queue." - _undoRedoHelper.Clear(); - _selectedTextChangesMadeSinceLastUndoSnapshot = 0; - _hasDoneSnapshotOnce = false; + ClearUndoRedo(); } /// @@ -1032,6 +1053,12 @@ namespace Avalonia.Controls if (change.Property == TextProperty) { + if (_needsUndoRedoSnapshotAfterTextChange) + { + _needsUndoRedoSnapshotAfterTextChange = false; + SnapshotUndoRedo(); + } + CoerceValue(CaretIndexProperty); CoerceValue(SelectionStartProperty); CoerceValue(SelectionEndProperty); @@ -1078,9 +1105,7 @@ namespace Avalonia.Controls // "Setting this property to false clears the undo stack. // Therefore, if you disable undo and then re-enable it, undo commands still do not work // because the undo stack was emptied when you disabled undo." - _undoRedoHelper.Clear(); - _selectedTextChangesMadeSinceLastUndoSnapshot = 0; - _hasDoneSnapshotOnce = false; + ClearUndoRedo(); } } @@ -1206,7 +1231,7 @@ namespace Avalonia.Controls var text = StringBuilderCache.GetStringAndRelease(textBuilder); - SetCurrentValue(TextProperty, text); + SetTextFromEdit(text); ClearSelection(); @@ -1611,7 +1636,7 @@ namespace Avalonia.Controls sb.Append(text); sb.Remove(start, end - start); - SetCurrentValue(TextProperty, StringBuilderCache.GetStringAndRelease(sb)); + SetTextFromEdit(StringBuilderCache.GetStringAndRelease(sb)); SetCurrentValue(CaretIndexProperty, start); @@ -1650,7 +1675,7 @@ namespace Avalonia.Controls sb.Append(text); sb.Remove(start, end - start); - SetCurrentValue(TextProperty, StringBuilderCache.GetStringAndRelease(sb)); + SetTextFromEdit(StringBuilderCache.GetStringAndRelease(sb)); } } @@ -2139,7 +2164,7 @@ namespace Avalonia.Controls /// /// Clears the text in the TextBox /// - public void Clear() => SetCurrentValue(TextProperty, string.Empty); + public void Clear() => SetTextFromEdit(string.Empty); private void MoveHorizontal(int direction, bool wholeWord, bool isSelecting, bool moveCaretPosition) { @@ -2396,7 +2421,7 @@ namespace Avalonia.Controls textBuilder.Append(text); textBuilder.Remove(start, end - start); - SetCurrentValue(TextProperty, StringBuilderCache.GetStringAndRelease(textBuilder)); + SetTextFromEdit(StringBuilderCache.GetStringAndRelease(textBuilder)); _presenter?.MoveCaretToTextPosition(start); @@ -2434,6 +2459,27 @@ namespace Avalonia.Controls return text.Substring(start, end - start); } + internal void SetTextFromEdit(string? value) => SetTextCore(value, TextMutationKind.Edit); + + internal void SetTextFromInternalSynchronization(string? value) => + SetTextCore(value, TextMutationKind.InternalSynchronization); + + private void SetTextCore(string? value, TextMutationKind mutationKind) + { + // Stays set through the synchronous TwoWay source echo so it isn't mistaken for an external replacement. + var previousMutationKind = _textMutationKind; + _textMutationKind = mutationKind; + + try + { + SetCurrentValue(TextProperty, value); + } + finally + { + _textMutationKind = previousMutationKind; + } + } + /// /// Returns the sum of any vertical whitespace added between the and in the control template. /// @@ -2561,6 +2607,13 @@ namespace Avalonia.Controls } } + private void ClearUndoRedo() + { + _undoRedoHelper.Clear(); + _selectedTextChangesMadeSinceLastUndoSnapshot = 0; + _hasDoneSnapshotOnce = false; + } + /// /// Undoes the first action in the undo stack /// diff --git a/tests/Avalonia.Controls.UnitTests/MaskedTextBoxTests.cs b/tests/Avalonia.Controls.UnitTests/MaskedTextBoxTests.cs index cddac813f8..74d025920b 100644 --- a/tests/Avalonia.Controls.UnitTests/MaskedTextBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/MaskedTextBoxTests.cs @@ -921,6 +921,127 @@ namespace Avalonia.Controls.UnitTests } } + [Fact] + public void Focusing_And_Unfocusing_Does_Not_Create_Undo_Operation() + { + using (Start(FocusServices)) + { + var target = new MaskedTextBox + { + Template = CreateTemplate(), + Mask = "000", + HidePromptOnLeave = true, + Text = "123" + }; + + var other = new MaskedTextBox { Template = CreateTemplate() }; + + var sp = new StackPanel(); + sp.Children.Add(target); + sp.Children.Add(other); + + target.ApplyTemplate(); + other.ApplyTemplate(); + + var root = new TestRoot() { Child = sp }; + + target.Focus(); + other.Focus(); + + Assert.False(target.CanUndo); + } + } + + [Fact] + public void Masked_Edit_Remains_Undoable() + { + using (Start(FocusServices)) + { + var target = new MaskedTextBox + { + Template = CreateTemplate(), + Mask = "000", + Text = "123" + }; + + target.ApplyTemplate(); + + var root = new TestRoot() { Child = target }; + + target.Focus(); + target.CaretIndex = 3; + + RaiseKeyEvent(target, Key.Back, KeyModifiers.None); + + Assert.Equal("12_", target.Text); + Assert.True(target.CanUndo); + + target.Undo(); + + Assert.Equal("123", target.Text); + } + } + + [Fact] + public void External_Replacement_After_Masked_Edit_Clears_Undo_History() + { + using (Start(FocusServices)) + { + var target = new MaskedTextBox + { + Template = CreateTemplate(), + Mask = "000", + Text = "123" + }; + + target.ApplyTemplate(); + + var root = new TestRoot() { Child = target }; + + target.Focus(); + target.CaretIndex = 3; + + RaiseKeyEvent(target, Key.Back, KeyModifiers.None); + + Assert.True(target.CanUndo); + + target.Text = "456"; + + Assert.False(target.CanUndo); + Assert.False(target.CanRedo); + } + } + + [Fact] + public void Clear_And_SelectedText_Replacement_Remain_Undoable() + { + using (Start(FocusServices)) + { + var target = new MaskedTextBox + { + Template = CreateTemplate(), + Mask = "000", + Text = "123" + }; + + target.ApplyTemplate(); + + var root = new TestRoot() { Child = target }; + + target.Focus(); + + target.Clear(); + Assert.True(target.CanUndo); + + target.Undo(); + target.SelectionStart = 0; + target.SelectionEnd = 1; + target.SelectedText = "9"; + + Assert.True(target.CanUndo); + } + } + private static TestServices FocusServices => TestServices.MockThreadingInterface.With( keyboardDevice: () => new KeyboardDevice(), keyboardNavigation: () => new KeyboardNavigationHandler(), diff --git a/tests/Avalonia.Controls.UnitTests/TextBoxTests.cs b/tests/Avalonia.Controls.UnitTests/TextBoxTests.cs index 1d7570c2a1..39dd5efa1d 100644 --- a/tests/Avalonia.Controls.UnitTests/TextBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/TextBoxTests.cs @@ -1406,6 +1406,73 @@ namespace Avalonia.Controls.UnitTests } } + [Fact] + public void Binding_Source_Change_Clears_Undo_History() + { + using (UnitTestApplication.Start(Services)) + { + var source = new Class1 { Bar = "initial" }; + var textBox = new TextBox + { + Template = CreateTemplate(), + DataContext = source, + }; + + textBox.Bind(TextBox.TextProperty, new Binding(nameof(Class1.Bar)) + { + Mode = BindingMode.TwoWay, + }); + textBox.Measure(Size.Infinity); + textBox.CaretIndex = textBox.Text!.Length; + + RaiseTextEvent(textBox, " edit"); + RaiseKeyEvent(textBox, Key.Space, KeyModifiers.None); + RaiseTextEvent(textBox, " more"); + + Assert.Equal("initial edit more", source.Bar); + Assert.True(textBox.CanUndo); + + source.Bar = "replacement"; + + Assert.Equal("replacement", textBox.Text); + Assert.False(textBox.CanUndo); + Assert.False(textBox.CanRedo); + } + } + + [Fact] + public void TwoWay_Binding_Source_Echo_Does_Not_Clear_Undo_History() + { + using (UnitTestApplication.Start(Services)) + { + var source = new Class1 { Bar = "initial" }; + var textBox = new TextBox + { + Template = CreateTemplate(), + DataContext = source, + }; + + textBox.Bind(TextBox.TextProperty, new Binding(nameof(Class1.Bar)) + { + Mode = BindingMode.TwoWay, + }); + textBox.Measure(Size.Infinity); + textBox.CaretIndex = textBox.Text!.Length; + + RaiseTextEvent(textBox, " edit"); + RaiseKeyEvent(textBox, Key.Space, KeyModifiers.None); + RaiseTextEvent(textBox, " more"); + + Assert.Equal("initial edit more", source.Bar); + Assert.True(textBox.CanUndo); + + textBox.Undo(); + + Assert.Equal("initial edit", textBox.Text); + Assert.Equal("initial edit", source.Bar); + } + } + [Fact] public void Setting_UndoLimit_Clears_Undo_Redo() {