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