RFR: 8330462: StringIndexOutOfBoundException when typing anything into TextField [v24]
Oliver Kopp
duke at openjdk.org
Mon Apr 29 16:08:14 UTC 2024
On Mon, 29 Apr 2024 12:35:19 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:
>> Oliver Kopp has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Revert using utility method
>
> modules/javafx.graphics/src/main/java/com/sun/glass/ui/win/WinTextRangeProvider.java line 124:
>
>> 122: // In case there was an overflow, return the maximum end index
>> 123: if (res < 0) return maxEndIndex;
>> 124: return res;
>
> In this class,
> - The index **start** and **end** are guaranteed to be +ve values such that, `0 <= start <= end <= length` (length is the total length of text in the text component). : please check the method `validateRange()`
> - So we only need to validate that the variable `length` in this helper method is valid i.e.
> 1. `length > 0` and
> 2. `length <= end - start`
>
>
> /**
> * In the context of substrings, this is a helper method to get the end offset index
> * from the start index for a given length of a substring.
> *
> * @param startIndex The start index of the range in the string. Needs to be 0 or more (not checked in the code).
> * @param length The requested length of a substring in the range when starting from "start".
> * Invalid values are treated as full length.
> * @param endIndex The end index of the range in the string. Needs to be equal or greater than startIndex
> * (not checked in the code).
> */
> static int getEndOffsetIndex(int startIndex, int length, int endIndex) {
> if (length < 0 || (endIndex - startIndex) <= length) {
> return endIndex;
> }
> return startIndex + length;
> }
>
> - With this change
> 1. The start and end index values are always positive and hence negative value tests should be removed.
> 2. The name of method is changed so, the Shim class would need a change too.
> 3. The fix could be as simple as below one line correction in the ternary statement in the GetText() method, but then we won't be able to add the test. So keeping the helper method would be good idea.
> `int endOffset = (length < 0 || (endIndex - startIndex) <= length) ? end : start + maxLength;`
Smalltalk: I could go back to my first commit https://github.com/openjdk/jfx/commit/e81712ca12035e6607c9a31379fa0180be8be7fd to be consistent with the other style in the class 🤣🤣🤣. Only joking, the new code is more readable to Java newcomers IMHO. 😅✌️
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1442#discussion_r1583344489
More information about the openjfx-dev
mailing list