Skip to content
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

TextBox SelectAll Input Chinese crash #7304

Open
q812143836 opened this issue Nov 18, 2022 · 10 comments
Open

TextBox SelectAll Input Chinese crash #7304

q812143836 opened this issue Nov 18, 2022 · 10 comments
Milestone

Comments

@q812143836
Copy link

  • .NET Core Version: .NET6
  • Windows version: (Windows10)
  • Does the bug reproduce also in WPF for .NET Framework 4.8?: Yes

Problem description:TextBox SelectAll Input Chinese crash

    <Grid>
        <TextBox
            Name="textEdit"
            AcceptsReturn="False"
            TextWrapping="NoWrap" />
    </Grid>
      public MainWindow()
        {
            InitializeComponent();
            //In actual business, sometimes I need to assign a string with a newline character
            textEdit.Text = "testasdfasdfgasd阿斯顿发斯蒂芬\r\n撒旦法师打发斯蒂芬\r\n阿斯顿发送到发送到\r\n";
        }

When there are multiple lines of strings, when using the Chinese input method to input, the selected text will crash directly
I know setting AcceptsReturn="True" UndoLimit!="0" will fix this
But I need to use Enter event, so it doesn't work for me
So how should I solve this problem?

WPFTextBoxBug

@Symbai
Copy link
Contributor

Symbai commented Nov 19, 2022

Don't understand what you're doing with Invariant.cs in your GIF. But I am also not using WPF source code for reproduction. I created a normal .NET 6(.0.111) WPF application with your repro steps, run it in debug mode and selected all text, no crash at all.

Sorry, my bad.

@q812143836
Copy link
Author

You need to select all the text

And then you type it in with the Chinese input method and it crashes

Your video presentation does not include input in the Select All state

@q812143836
Copy link
Author

WPFTextBoxBug2

In the demo above I used English input method and Chinese input method

Only manual input using the Chinese input method in the selected state will cause a crash
Copy-pasting doesn't cause this problem

@miloush
Copy link
Contributor

miloush commented Nov 19, 2022

This is this assert failing (for any IME as long as the selection contains new line):

text = this.TextEditor._FilterText(composition.CompositionText, range, /*filterMaxLength:*/false);
// A change in length should not happen other than for MaxLength filtering during finalization since we cover those
// cases and reject input if necessary when the IME edits the document in the first place.
Invariant.Assert(text.Length == composition.CompositionText.Length);

The filter trims the text at first line break if AcceptsReturn is false:

if (!this.AcceptsReturn)
{
int endOfFirstLine = textData.IndexOf(Environment.NewLine, StringComparison.Ordinal);

Because the selection including new lines will become part of the CompositionText, the assumption described in the comment is violated.

@miloush
Copy link
Contributor

miloush commented Nov 19, 2022

The documentation for AcceptsReturn says:

true if pressing the ENTER key inserts a new line at the current cursor position; otherwise, the ENTER key is ignored.

This is clearly wrong either way. First, it assumes that ENTER key will result into a new line in the text, which might not be the case for IME. Second, it also causes pasted text to be trimmed after the first new line, which normally has nothing to do with ENTER key. It does not prevent new lines in the Text property though, because you can set it directly. Changing either of these two behaviors would be a breaking change.

Filtering during composition update is problematic. First it is rude to mess with the IME by coercing its output in the middle of the composition. If the filtering is desired, it should happen when the composition is finished. Second, the IME is already done by this point, the framework is only replaying the composition events that had happened in the storage, so the IME cannot deal with the coercions even if it wanted to. And finally, it destabilizes the whole replay stack because the recorded text pointers will no longer be valid (you will run into that if you simply remove the assert) and fixing it up would be a lot of work that only the IME might be qualified to do.

I am not a big fan of all this filtering to begin with, but we can't really remove it at this stage. I think the right thing to do here is to only do the filtering at the end of the composition, i.e. replace line 1981 above with text = composition.CompositionText.

(Note also that there is CharacterCasing property on the TextBox that is enforced by filtering. This uses the current input language and Unicode does not guarantee that such casing changes will not change the number of characters in the string, so this is a similar bug waiting to happen. Also note that the documentation for this property has a remark "This property does not affect characters that are added programmatically" which would be fitting for AcceptsReturn property too. And the AcceptsTab replaces tabs with spaces, which might have been a better design for AcceptsReturn too.)

@singhashish-wpf singhashish-wpf added 📭 waiting-author-feedback To request more information from author. and removed Untriaged Requires WPF team triage labels Nov 21, 2022
@ghost ghost removed the 📭 waiting-author-feedback To request more information from author. label Nov 25, 2022
@q812143836
Copy link
Author

 private void textBox_PreviewKeyDown(object sender, KeyEventArgs e)
    {
        if (this.textBox.SelectedText == this.textBox.Text)
        {
            this.textBox.Text = string.Empty;
        }
    }

I used this method to solve the crash problem as stupid as it was

@pchaurasia14 pchaurasia14 added the 📭 waiting-author-feedback To request more information from author. label Jan 20, 2023
@pchaurasia14
Copy link
Contributor

@q812143836 - Are we good to close this due to the availability of above workaround?

@q812143836
Copy link
Author

@q812143836 - 由于上述解决方法的可用性,我们是否可以关闭它?

@q812143836 - Are we good to close this due to the availability of above workaround?

YES

@ghost ghost removed the 📭 waiting-author-feedback To request more information from author. label Jan 20, 2023
@miloush
Copy link
Contributor

miloush commented Jan 20, 2023

@pchaurasia14 Good for @q812143836 that it works for his scenario, but I don't think it is an acceptable workaround. As a trivial example, selecting all text and pressing keys to copy it into clipboard will delete the text instead.

I suggested a way to fix the crash which is indeed a bug, i.e. filter after the composition is done. I can submit PR if there is an agreement.

@pchaurasia14
Copy link
Contributor

@miloush - I agree it is not ideal and we'll be happy to review a PR that fixes the issue.

@pchaurasia14 pchaurasia14 reopened this Jan 20, 2023
@karelz karelz added this to the Future milestone Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants