RFR: 8176270: Adding ChangeListener to TextField.selectedTextProperty causes StringOutOfBoundsException

[Jeanette Winzenburg]

On Fri, 7 Feb 2020 18:04:35 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> This is a WIP PR. Requesting for comments.
>> 
>> I could not replicate the test given at https://bugs.openjdk.java.net/browse/JDK-8176270 without TestFX. I nevertheless let my try to replicate the @Test things in here.
>> 
>> The fix is just a wild guess without really understanding the side effects of `.addListener`.
> 
> modules/javafx.controls/src/main/java/javafx/scene/control/TextInputControl.java line 168:
> 
>> 167:                 // Ensure that the last character to get is within the bounds of the txt string
>> 168:                 if (end >= start + length) end = length-1;
>> 169:                 // In case the start is after the whole txt, nothing valid is selected. Thus, return the default.
> 
> First, I think the existing test is simply wrong: Why should the `start` value matter when checking whether the `end` value needs to be clamped -- it's an `end` point not a number of characters. I think the entire problem might be the fact that it is comparing against `start + length`. Second, the value to be clamped to was correct before your change. The `substring` method to which `end` is passed is documented as subtracting 1.
> 
> I think the test should be something like:
> 
>     if (end > length) end = length;

on second thought: correct clamping or not, the start/end indices of selection are invalid whenever text was removed/added at an index before the selection - they are in coordinates of the _old_ text, not the new. If they point to an index > new textLength, incorrect clamping will throw. If they are both < new textLength, it'll fire an intermediate changeEvent with incorrect selection, the total sequence beining
  oldSelectedText -> incorrectly-shifted-selectedText 
  incorrectly-shifted-selectedText -> empty

The correct change notification would be, because at the end of the text change, the selection is cleared,
   oldSelectedText -> empty

To me, it looks like using a binding here is not the best of options.

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

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