RFR: 8306083: Text.hitTest is incorrect when more than one Text node in TextFlow [v7]

[Phil Race]

On Mon, 21 Aug 2023 18:25:08 GMT, Karthik P K <kpk at openjdk.org> wrote:

>> The text run selected in `PrismTextLayout::getHitInfo()` method for character index calculation was not correct when hitTest was invoked for Text node in a TextFlow with more than one Text child. Hence wrong character index value was calculated.
>> 
>> Since only x, y coordinates were available in the above mentioned method, sending the text as a parameter to this method is necessary so as to know if the text run selected for character index calculation is correct. Along with this change modified the `PrismTextLayout::getHitInfo()` method to calculate the correct character index.
>> 
>> Added tests to validate the changes.
>
> Karthik P K has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comments

Changes requested by prr (Reviewer).

modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java line 760:

> 758:         int lineCount = getLineCount();
> 759:         while (index < lineCount) {
> 760:             if (text != null) {

why aren't you using "textFound" as in if "(!textFound) {"

modules/javafx.graphics/src/main/java/com/sun/javafx/text/PrismTextLayout.java line 762:

> 760:             if (text != null) {
> 761:                 for (TextRun r : lines[index].runs) {
> 762:                     if (r.getTextSpan() == null || r.getTextSpan().getText().equals(text)) {

I am less familiar with this code than I'd like to be, but some things seem odd to me.
(1) If you are looking for a particular match of text, what if there are 2 lines or spans that have the exact same text ?
(2) What is that the logic behind this line ? At least currently you only pass in text for a Text node.
Can you break down what exactly it is doing ?

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

PR Review: https://git.openjdk.org/jfx/pull/1157#pullrequestreview-1636193932
PR Review Comment: https://git.openjdk.org/jfx/pull/1157#discussion_r1331981945
PR Review Comment: https://git.openjdk.org/jfx/pull/1157#discussion_r1331987391