On Wed, 21 Feb 2024 10:01:37 GMT, Karthik P K <k...@openjdk.org> wrote:

>> In the `getHitInfo()` method of PrismTextLayout, RTL node orientation 
>> conditions were not considered, hence hit test values such as character 
>> index and insertion index values were incorrect.
>> 
>> Added checks for RTL orientation of nodes and  fixed the issue in 
>> `getHitInfo()` to calculate correct hit test values.
>> 
>> Added system tests to validate the changes.
>
> 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/98fd3ec3...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.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1501710956

Reply via email to