On Sat, 24 Feb 2024 23:00:43 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> Karthik P K has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 13 additional >> commits since the last revision: >> >> - Merge branch 'master' into rtl_hittest_issue >> - Code refactoring >> - Review comments >> - Fix emoji issue >> - Inline node issue fix >> - Code review changes >> - Fix issue with multiline text >> - Fix issue with RTL text within LTR text >> - Review changes >> - Fix multiline text insertion index calculation issue >> - ... and 3 more: https://git.openjdk.org/jfx/compare/05c327e9...e3812732 > > modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java > line 606: > >> 604: boolean addLtrIdx = run.getTextSpan().getText().length() != >> run.length; >> 605: if (r.getStart() != curRunStart && !r.isLinebreak() && >> addLtrIdx >> 606: && r.getTextSpan().getText().equals(text)) { > > I'm concerned about this check: `r.getTextSpan().getText().equals(text)` -- > it seems to me that it either must be irrelevant, or it if is relevant, what > if I have many `Text` nodes in a `TextFlow` that happen to have the same > text? Let's say I have an English text, where I give each word its own `Text` > node in a `TextFlow`. There would be many duplicates... so I find it hard to > believe this check could accomplish anything. You are right. It fails when there are repeated text nodes. I will look into this ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1502392096