TextEditor caret placement (this one is a bug)

[code]This is a sample that demonstrates a bug
in the caret display.

Paste it into a TextEditor with font size 18.
Use the arrow keys to move the caret
to right before the hash: # then mark this
text by keeping the shift key down and
using the arrow down key until the caret
is #
right after the hash in the above line.
That is, the hash signs are the first and
last chars in the highlight.

Then type the letter a.

The caret erroneously gets placed on the
next line, right before “right after”.

Then type the letter b.

Now, things are as expected. It seems the internal logic of the TextEditor works ok, but the caret occasionally appears on a wrong line when highlighted text is replaced by something else.

Now, type Ctrl+Z to undo the last experiment.
Then highlight the dollarized paragraph
so that the dollar signs are the first and
last character. Use Ctrl+C to copy to the
clipboard.

Then highlight the hashed sentence as
before, including both hashes. Press
Ctrl+V to replace it with the dollarized
paragraph in the clipboard.

On my system, the caret now appears
4 lines lower than it should. Still it seems
the insertion/deletion logic is sound, only
the caret display is wrong, and only until
the next keypress.[/code]

I assume you’re on windows, - but yes same behaviour on Mac here.

And here is a fix:

[code]void TextEditor::insertTextAtCursor (String newText)
{
if (allowedCharacters.isNotEmpty())
newText = newText.retainCharacters (allowedCharacters);

if (! isMultiLine())
    newText = newText.replaceCharacters (T("\r\n"), T("  "));
else
    newText = newText.replace (T("\r\n"), T("\n"));

const int newCaretPos = selectionStart + newText.length();
const int insertIndex = selectionStart;

remove (selectionStart, selectionEnd,
        &undoManager,
        selectionStart /* instead of newCaretPos */
		);

if (maxTextLength > 0)
    newText = newText.substring (0, maxTextLength - getTotalNumChars());

if (newText.isNotEmpty())
    insert (newText,
            insertIndex,
            currentFont,
            findColour (textColourId),
            &undoManager,
            newCaretPos);

textChanged();

}[/code]
The use of newCaretPos in the remove() call is ill-defined; if newText is longer than the text after the insertion point (which isn’t an unusual situation), the caret index doesn’t exist.

Frankly, I don’t understand how this can cause the false caret display since the caret is set in the insert() call anyway, and the sample I posted doesn’t produce such undefined indexes, but the fix works as far as I can tell.

On 2nd thought, I think I know what’s going on. The remove() call places the caret newText.length chars after the insertion point, calculating by whatever text is there. The insert() call leaves it there because the index is unchanged, but since the text has changed, the coordinates aren’t right.

I guess newCaretPos is used in the remove() call to avoid scrolling back in cases where the anchor point is out of sight; this may happen with the fix I suggested.

So perhaps [code] remove (selectionStart, selectionEnd,
&undoManager,
newCaretPos - 1 /* instead of newCaretPos */
);

[/code] is a better choice - to ensure a recalculation is triggered in the insert() call.

well spotted - that’s a pretty subtle one, to say the least! I like your solution as a simple fix, I’ll check something in to sort that out shortly.