RFR: 8282290: TextField Cursor Position one off
Marius Hanl
mhanl at openjdk.org
Wed Nov 15 19:49:43 UTC 2023
On Wed, 15 Nov 2023 19:02:09 GMT, Andy Goryachev <angorya at openjdk.org> wrote:
>> 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());
>> }
>
> this might be problematic, as it uses undocumented aspect of Text/Flow.caretShape(). the current implementation returns either a single line, or two lines (moveto,lineto,moveto,lineto) in the case of split/dual caret. But this may not be true in the future! If we add a directional indicator to the caret this logic will break.
>
> What we may need is to invent a new API possibly that gives us more information about the caret - whether it's a single or split one, or whether it has a directionality indicator, information about the adjacent text segments (rtl,ltr), etc.
>
> We can keep this logic for now, to be fixed once (and if) we add the more detailed caret API.
Yes, right now it is a bit magic. At least the first if can be simplified still, as `!= 4` will also handle `== 0`.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1287#discussion_r1394717069
More information about the openjfx-dev
mailing list