On Wed, 7 Feb 2024 10:43:12 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 incrementally with one additional > commit since the last revision: > > Fix emoji issue Looks good with minor comments. @hjohn this might impact your work in #1236 - would you like to take a look? modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java line 459: > 457: break; > 458: } > 459: if (i - 1 >= 0) { `if (i > 0)` ? modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java line 531: > 529: for (int i = runs.length - 1; i > runIdx; i--) { > 530: TextRun r = runs[i]; > 531: boolean addLtrIdx = > run.getTextSpan().getText().length() != run.length; Q: we should never get an NPE here, correct? ------------- Marked as reviewed by angorya (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/1323#pullrequestreview-1865964546 PR Comment: https://git.openjdk.org/jfx/pull/1323#issuecomment-1934698730 PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1483395969 PR Review Comment: https://git.openjdk.org/jfx/pull/1323#discussion_r1483400064