On Tue, 14 Nov 2023 10:24:20 GMT, Karthik P K <k...@openjdk.org> wrote:
> The change listener on caretPositionProperty() was not getting invoked on
> replacing the content with same text as there was no change in caret
> position. Since the textProperty invalidation sets the forward bias to true
> by default, incorrect caret position was calculated when the same text was
> replaced after clicking on the trailing edge of last character or empty space
> in the TextField.
>
> Since caretShapeProperty invalidation listener gets invoked without changing
> the caret position, updating the caretBiasProperty on this listener solves
> the issue.
>
> Since the caret position value will be same when the caret is present after
> the last character or before the last character, it can not be validated
> using unit test.
> The fix can be validated using MonkeyTester.
> Steps to select TextField option in Monkey Tester.
>
> - Open the MonkeyTester app and select TextField from the left option pane.
> - Select Short from Text selection option and click on the TextField to bring
> it into focus.
> - Select all using cmd(ctrl) + a and copy using cmd(ctrl) + c
> - Click on empty space in the TextField after the present content.
> - Select all again using the keyboard shortcut and paste using cmd(ctrl) + v
> - The caret should be displayed after the last character. Without the fix
> caret gets displayed before the last character.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/TextFieldSkin.java
line 248:
> 246: textNode.caretShapeProperty().addListener(observable -> {
> 247:
> caretPath.getElements().setAll(textNode.caretShapeProperty().get());
> 248: if (caretPath.getElements().size() == 0 ||
> caretPath.getElements().size() != 4) {
I think this can be simplified to:
if (caretPath.getElements().size() != 4) {
// The caret pos is invalid.
updateTextNodeCaretPos(control.getCaretPosition());
} else {
caretWidth = Math.round(caretPath.getLayoutBounds().getWidth());
}
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1287#discussion_r1394603088
