RFR: 8176270: Adding ChangeListener to TextField.selectedTextProperty causes StringOutOfBoundsException [v7]

Kevin Rushforth kcr at openjdk.java.net
Thu Jul 9 20:41:52 UTC 2020


On Thu, 2 Jul 2020 05:29:05 GMT, Robert Lichtenberger <rlichten at openjdk.org> wrote:

>> This PR fixes JDK-8176270 by clamping the end index of the selected text to the length of the text.
>
> Robert Lichtenberger has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8176270: Adding ChangeListener to TextField.selectedTextProperty causes StringOutOfBoundsException
>   
>   Clamping is no longer needed, removed it.

The fix looks good. I left one question and one comment on the tests.

I believe this is safe enough and desired for 15, so can you please retarget this PR to the `jfx15` branch?

modules/javafx.controls/src/test/java/test/javafx/scene/control/TextFieldTest.java line 475:

> 474:         assertEquals(4, txtField.getSelection().getStart());
> 475:         assertEquals(4, txtField.getSelection().getStart());
> 476:     }

Did you mean to check `getStart()` twice?

modules/javafx.controls/src/test/java/test/javafx/scene/control/TextInputControlTest.java line 1924:

> 1923:         textInput.selectionProperty().addListener((__, ___, selection) -> selectionLog.append("|" +
> selection.getStart() + "," + selection.getEnd())); 1924:         textInput.textProperty().addListener((__, ___,
> text) -> textLog.append("|" + text)); 1925:

The pattern of using multiple underscores for unused variables isn't one we use. I recommend using something more
common like `obs` and `o` (or similar).

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

PR: https://git.openjdk.java.net/jfx/pull/138


More information about the openjfx-dev mailing list