On Wed, 15 Nov 2023 19:02:09 GMT, Andy Goryachev <ango...@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
