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