On Tue, 16 Jun 2020 09:03:35 GMT, Robert Lichtenberger <rlich...@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 > > Move replaceSelectionAtEndWithListener test to general test class. > Add a more general test for selection/text properties and replace/undo/redo > operations. Changing from using bindings to using listeners seems reasonable as long as there can't be a memory leak as a result (it should be fine). As I note inline below, I think the clamping is still wrong. Maybe you can't ever hit a case where it matters now, but it should still be fixed. modules/javafx.controls/src/main/java/javafx/scene/control/TextInputControl.java line 186: > 185: int length = txt.length(); > 186: if (end > start + length) end = length; > 187: if (start > length-1) start = end = 0; As I mentioned in [this comment in PR #73](https://github.com/openjdk/jfx/pull/73#discussion_r376528686), I think this test is wrong. The value of `start` has no impact on whether `end` is out of range. So... shouldn't this simply be `if (end > length) end = length;` ? ------------- PR: https://git.openjdk.java.net/jfx/pull/138
