Bug: Incorrect Caret Position After Undo in `CodeArea`

[Symeon94]

I've encountered an unexpected bug in the undo behavior of the CodeArea. It seems that when pressing Enter after selecting text, and then performing an undo (CTRL+Z), the caret is not restored to its original position. Instead, it ends up one character before the end of the original selection.

Steps to Reproduce

1. In a CodeArea, type any text — for example: Something,
2. Select a portion of the text — e.g., select "eth" in Som[eth]ing,
3. Press Enter — the result will be Som\ning with the caret positioned at the start of the second line,
4. Press CTRL+Z to undo — the text reverts to Something, but the caret is now placed after the letter 't' instead of its original position and there is no selection.

Expected Behavior

The undo operation should restore both:

• The original selection range
• The caret position exactly as it was before pressing Enter

Additional Notes

• Selection direction matters: selecting from 'e' to 'h' vs. 'h' to 'e' should not affect the final caret position after undo.
• The caret should consistently return to its pre-Enter location regardless of selection direction.

Update: I suspect the issue is in the UndoManager, so it might be a bug to be reported there instead, I will look for the root cause.

[Symeon94]

From what I have seen when I undo, the UndoManager has the position, the removed and the added text. But it does not contain the previous caret position (in PlainTextChange).

It seems to me though that there are two issues here:

1. The caret is not at the right position after undo (which can be fixed independantly of knowing the previous caret position)
2. Restoring the caret at the exact position it was before

[Symeon94]

After some research, it seems like the caret position is moved after the redo in RedoUtils method applyMultiRichTextChange by calling moveToChange.

In the example I gave the change is (start: 3, end:4, inserted: "eth", removed: "\n"), so logically speaking it should do:

int pos = chg.getPosition(); // 3
int len = chg.getNetLength(); // 2 (see below)
if ( len > 0 ) pos += len; // pos = 3 + 2 = 5

area.moveTo( Math.min( pos, area.getLength() ) ); // Move to min(5, ?)

The thing is, I debugged the code and chg.getNetLength() is equal to 2. Because it is removing \n and adding eth. That is how the cursor is moved to the fifth character in "Something".

public int getNetLength() {
    return this.insertedLength() - this.removedLength();
}

This is where the bug comes from.

[Symeon94]

I will add some Unit tests for all cases, for me there are 4:

1. Add some text
2. Delete some text with "delete" key
3. Delete some text with "backspace" key
4. Replace some text

Type	From	To	Position	Removed	Added	Net length	Caret after undo
Add	"Hat"	"Heat"	1	-	e	-1	1 (ok)
Delete	"Heat"	"Hat"	1	e	-	1	2 (not ok)
Backspace	"Heat"	"Hat"	1	e	-	1	2 (ok)
Replace	"Slate"	"Site"	1	la	i	1	2 (not ok)

Note that for the addition it does not add the length due to the if ( len > 0 )

[Symeon94]

I'm actually wrong when stating we can divide the problem. I could easily fix the replace issue but the delete needs the information about the previous position of the caret.
Delete with "backspace" or "delete" for the same letter produce different results, and there is no way to know where the caret must be after the undo without having this information saved (see my example above, we remove the same letter and the position/removed/added is exactly the same).

I'll see if I can easily add the caret position in the undo record, if not we can already push the easy fix for the replace.

[Jugen]

Here is some relevant code in CaretNode, just in case this is helpful and you haven't seen it:

RichTextFX/richtextfx/src/main/java/org/fxmisc/richtext/CaretNode.java

Lines 161 to 194 in 40fc429

	int finalPosition = getPosition();
	for (PlainTextChange plainTextChange : list) {
	int netLength = plainTextChange.getNetLength();
	if (netLength != 0) {
	int indexOfChange = plainTextChange.getPosition();
	// in case of a replacement: "hello there" -> "hi."
	int endOfChange = indexOfChange + Math.abs(netLength);
	/*
	"->" means add (positive) netLength to position
	"<-" means add (negative) netLength to position
	"x" means don't update position
	"+c" means caret was included in the deleted portion of content
	"-c" means caret was not included in the deleted portion of content
	Before/At/After means indexOfChange "<" / "==" / ">" position
	| Before +c | Before -c | At | After
	-------+---------------+-----------+----+------
	Add | N/A | -> | -> | x
	Delete | indexOfChange | <- | x | x
	*/
	if (indexOfChange == finalPosition && netLength > 0) {
	finalPosition = finalPosition + netLength;
	} else if (indexOfChange < finalPosition) {
	finalPosition = finalPosition < endOfChange
	? indexOfChange
	: finalPosition + netLength;
	}
	}
	}
	if (finalPosition != getPosition()) {
	moveTo(finalPosition);
	}

[Symeon94]

Thanks for that tip, I'll have a look. In the meantime I have extended the UT to include more cases. There is one more wrong thing I have seen. If you do a selection, normally you would expect that after the undo action you would get back that selection (meaning that if you select "Som[eth]ing" you want the selection back after the undo).

So that would be a third problem.

For the UT, I made sure to prepare them to pass against the old code before doing a change, so we are sure of the previous behaviour.

[Symeon94]

@Jugen I looked at that code which seems to define how the caret is repositioned after the change. But that is not relevant in that case, as the caret positioning is not in question. The problem is that some information are not recorded in the undo records, making it impossible to properly undo.

[Jugen]

I'm confused, I thought that's the whole problem is that the caret is positioned incorrectly after an undo. So I thought the code in CaretNode may be helpful in the calculation as it's different from in UndoUitls. For instance, it seems to take the current caret position into account. BTW there's similar code for selection in SelectionImpl.

[Symeon94]

You are correct about the issue. The issue is that we are missing information, I can fix the undo issue for the replace by doing:

private static void moveToChange( GenericStyledArea area, TextChange chg )
{
    int pos = chg.getInsertionEnd();
    area.moveTo( Math.min( pos, area.getLength() ) );
}

But that does not fix the problem of caret position for pressing the "delete" key.

The code in CaretNode is reevaluating what it calls finalPosition which I assume to be the last position of the text. For this it has all the needed information. Let me illustrate: for word "Something" if caret is at 4 ("Some|thing"), and:

• You press "delete", it removes "t" (len=1) at position 4,
• You press "backspace", it removes "e" (len = 1) at position 3.

It doesn't need to know where the caret was in that case, it knows the position and the length of the removed content.

When doing "undo", the information "e" at position 3 could either be:

• A "delete" when the caret was at position 3
• A "backspace" when the caret was at position 4

I hope I'm clear enough. Let me know if you think I'm missing something there.

My plan is to try to add the selection information in the TextChange and see if that can be done with minimal changes.