-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix Entry select all text on refocus not working on WinUI #35383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
2628170
eff5421
d09285f
7c8bd36
98eec79
0b16696
0fbb78b
503f3a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -315,6 +315,29 @@ string ITextInput.Text | |
| set => SetValue(TextProperty, value, SetterSpecificity.FromHandler); | ||
| } | ||
|
|
||
| private protected override void OnBindablePropertySet(BindableProperty property, object original, object value, bool changed, bool willFirePropertyChanged) | ||
| { | ||
| base.OnBindablePropertySet(property, original, value, changed, willFirePropertyChanged); | ||
|
|
||
| // When the same CursorPosition or SelectionLength value is re-set, the bindable property | ||
| // system detects no change (changed=false) and does not fire PropertyChanged, so the | ||
| // handler mapper is never invoked. We force a handler update here so the native control | ||
| // always receives the selection state. This is required on WinUI where the native TextBox | ||
| // resets the caret position on each focus event. | ||
| if (!changed) | ||
| { | ||
| switch (property.PropertyName) | ||
| { | ||
| case nameof(CursorPosition): | ||
| Handler?.UpdateValue(nameof(CursorPosition)); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [warning] Cross-platform re-entry risk This shared |
||
| break; | ||
| case nameof(SelectionLength): | ||
| Handler?.UpdateValue(nameof(SelectionLength)); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private protected override string GetDebuggerDisplay() | ||
| { | ||
| var debugText = DebuggerDisplayHelpers.GetDebugText(nameof(Text), Text); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, 23329, "Entry select all text on refocus does not work on WinUI", PlatformAffected.UWP)] | ||
| public class Issue23329 : ContentPage | ||
| { | ||
| readonly Entry _textBox; | ||
|
|
||
| public Issue23329() | ||
| { | ||
| _textBox = new Entry | ||
| { | ||
| AutomationId = "TextBox", | ||
| WidthRequest = 300 | ||
| }; | ||
| _textBox.Focused += OnTextBoxFocused; | ||
|
|
||
| var otherElement = new Button | ||
| { | ||
| AutomationId = "OtherElement", | ||
| Text = "Other", | ||
| WidthRequest = 128 | ||
| }; | ||
|
|
||
| Content = new VerticalStackLayout | ||
| { | ||
| Children = { _textBox, otherElement } | ||
| }; | ||
| } | ||
|
|
||
| void OnTextBoxFocused(object sender, FocusEventArgs e) | ||
| { | ||
| if (_textBox.Text != null) | ||
| { | ||
| _textBox.CursorPosition = 0; | ||
| _textBox.SelectionLength = _textBox.Text.Length; | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| #if WINDOWS | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
|
|
||
| public class Issue23329 : _IssuesUITest | ||
| { | ||
| public Issue23329(TestDevice device) | ||
| : base(device) | ||
| { } | ||
|
|
||
| public override string Issue => "Entry select all text on refocus does not work on WinUI"; | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.Entry)] | ||
| public void EntrySelectAllOnRefocusReplacesText() | ||
| { | ||
| App.WaitForElement("TextBox"); | ||
| App.Click("TextBox"); | ||
| App.EnterText("TextBox", "Hello"); | ||
|
|
||
| // Blur and refocus twice. The second refocus is where the bug manifests: | ||
| // the Focused handler re-applies the same CursorPosition / SelectionLength | ||
| // values, so the bindable property change tracking would skip propagation | ||
| // to the native TextBox unless the InputView / TextBoxExtensions fix forces | ||
| // a handler update for unchanged values. | ||
| App.Click("OtherElement"); | ||
| App.Click("TextBox"); | ||
| App.Click("OtherElement"); | ||
| App.Click("TextBox"); | ||
|
|
||
| // If select-all worked on the second refocus, typing 'X' replaces the | ||
| // entire selected text, so the Entry ends up containing just "X". If the | ||
| // bug is still present, 'X' is inserted at the native caret position | ||
| // (wherever the click landed in "Hello"), producing something like "HelloX". | ||
| App.EnterText("TextBox", "X"); | ||
|
|
||
| var text = App.FindElement("TextBox").GetText(); | ||
| Assert.That(text, Is.EqualTo("X")); | ||
| } | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[moderate] Cross-Platform Consistency / Regression Prevention - This shared
InputViewoverride changes same-valueCursorPosition/SelectionLengthpropagation for everyEntry/Editorhandler, not just WinUI. The regression and bug are Windows-specific, but Android/iOS/Tizen now receive extra mapper updates when app code re-sets the same selection values, and those handlers have platform-specific focus/timing behavior. Please either scope the forced same-value update to Windows (for example by moving this override into a Windows-specific partial/guard) or add non-Windows coverage proving same-value selection updates do not move or corrupt caret/selection state.