Re: RFR: 6278172: TextComponent.getSelectedText() throws StringIndexOutOfBoundsException [v2]
On Fri, 8 Jan 2021 00:32:32 GMT, Sergey Bylokhov wrote: > The new call to select() method should take care of that since the user as > well can call it passing wrong parameters. Thank you for clarification. Looks good. - PR: https://git.openjdk.java.net/jdk/pull/1104
Re: RFR: 6278172: TextComponent.getSelectedText() throws StringIndexOutOfBoundsException [v2]
On Thu, 7 Jan 2021 20:11:31 GMT, Alexey Ivanov wrote: > I wonder how native text components behave: is the selection preserved when > text is replaced? On windows, the selection resets, on macOS and Linux it is preserved. But it works that way because we implemented it in a such way, actually did not pay attention to such use-case. > Do I get it right that if the new text is longer than the initial one, then > only the portion will be left selected. For the case: 1. the user sets the short text 2. then selects the longer distance 3. then sets the longer text nothing is changed. The select() method at step 2 will trim passed parameters based on the short text at step 1. So only the portion of the text will be selected. > Won't it cause the issue if the scenario where the selectionStart is past the > length of the new text. I mean the only "field" portion of "text field" is > selected, and then the text in component is replaced with "text", thus the > selectionStart points 1 character past the end of the new text. The new call to select() method should take care of that since the user as well can call it passing wrong parameters. - PR: https://git.openjdk.java.net/jdk/pull/1104
Re: RFR: 6278172: TextComponent.getSelectedText() throws StringIndexOutOfBoundsException [v2]
On Thu, 7 Jan 2021 20:10:07 GMT, Alexey Ivanov wrote: >> Sergey Bylokhov has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains three additional >> commits since the last revision: >> >> - Merge branch 'master' into JDK-6278172 >> - Merge branch 'master' into JDK-6278172 >> - Initial fix > > Marked as reviewed by aivanov (Reviewer). I wonder how native text components behave: is the selection preserved when text is replaced? - PR: https://git.openjdk.java.net/jdk/pull/1104
Re: RFR: 6278172: TextComponent.getSelectedText() throws StringIndexOutOfBoundsException [v2]
On Thu, 7 Jan 2021 03:42:24 GMT, Sergey Bylokhov wrote: >> The text components are implements as wrappers over the "native" peers. Most >> of the functionality is provided by the peers not wrappers like >> TextArea/TextField. The current bug occurs in the TextArea/TextField when >> the native peer was created yet. >> >> The steps to reproduce: >> 1. Sets the long text to the component >> 2. Select all text in the component >> 3. Sets the short text to the component >> 4. Request the selected text -> BOOM >> >> The bug on step 4 occurred because we request a long substring of the short >> text. To eliminate an exception we have two choices: >> 1. Preserve the selection in the setText(), but limit it by the length of >> the current text. >> 2. Resets the selection. >> >> I tried both solutions, the second caused some TCK tests to fail, so I >> selected the first one. > > Sergey Bylokhov has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Merge branch 'master' into JDK-6278172 > - Merge branch 'master' into JDK-6278172 > - Initial fix Marked as reviewed by aivanov (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1104
Re: RFR: 6278172: TextComponent.getSelectedText() throws StringIndexOutOfBoundsException [v2]
> The text components are implements as wrappers over the "native" peers. Most > of the functionality is provided by the peers not wrappers like > TextArea/TextField. The current bug occurs in the TextArea/TextField when the > native peer was created yet. > > The steps to reproduce: > 1. Sets the long text to the component > 2. Select all text in the component > 3. Sets the short text to the component > 4. Request the selected text -> BOOM > > The bug on step 4 occurred because we request a long substring of the short > text. To eliminate an exception we have two choices: > 1. Preserve the selection in the setText(), but limit it by the length of > the current text. > 2. Resets the selection. > > I tried both solutions, the second caused some TCK tests to fail, so I > selected the first one. Sergey Bylokhov has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Merge branch 'master' into JDK-6278172 - Merge branch 'master' into JDK-6278172 - Initial fix - Changes: - all: https://git.openjdk.java.net/jdk/pull/1104/files - new: https://git.openjdk.java.net/jdk/pull/1104/files/a414f2df..5c99c3d4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=1104&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=1104&range=00-01 Stats: 360278 lines in 4004 files changed: 225151 ins; 88575 del; 46552 mod Patch: https://git.openjdk.java.net/jdk/pull/1104.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/1104/head:pull/1104 PR: https://git.openjdk.java.net/jdk/pull/1104
