On Thu, 2 Jul 2020 05:29:05 GMT, Robert Lichtenberger <[email protected]>
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
