> The bug happens when List.select(int) is used in multiple selection mode. It 
> only appears when the List is added to a Frame, because in that case, the 
> selection is handled by the platform peer, and the internal logic in the List 
> class is skipped.
> 
> To fix this, the code now uses 
> [addSelectionInterval](https://github.com/openjdk/jdk/blob/910bb68e5191f830ff6f3dff5753e4e5f6214a7b/src/java.desktop/share/classes/javax/swing/ListSelectionModel.java#L93)
>  instead of setSelectedIndex. This method works correctly for multiple 
> selection mode when using JList as the delegate on macOS.
> 
> I added DeselectionUnitTest and SelectionUnitTest tests for some selection 
> and deselection cases. At first, it was one big test, but I split it because 
> it got too large.
> 
> **Notes on invalid index handling**
> According to the java.awt.List 
> [spec](https://github.com/openjdk/jdk/blob/0e5655e6680762a99b5aecb58369b880ea913565/src/java.desktop/share/classes/java/awt/List.java#L573),
>  using invalid indexes is undefined behavior. For now, I have decided not to 
> validate these cases fully in this patch, but I did add a check in the 
> `LWListPeer.select()` to handle them more safely.
> 
> The previously used setSelectedIndex method 
> [ignored](https://github.com/openjdk/jdk/blob/0e5655e6680762a99b5aecb58369b880ea913565/src/java.desktop/share/classes/javax/swing/JList.java#L2229)
>  indexes greater than the list size, unlike addSelectionInterval. So I added 
> an explicit check to skip all indexes larger than the list size.
> 
> Later, I discovered that passing a negative index not only causes an 
> exception (which is acceptable, since it's undefined behavior), but also 
> leaves the peer in an inconsistent state. This happens because 
> setSkipStateChangedEvent(false) at the end of LWListPeer.select() is not 
> called if an exception is thrown.
> 
> To prevent this, I added a check to skip all negative values as well. As a 
> result, the peer now cleanly ignores all out-of-range indexes.
> 
> **Description of how invalid indexes are handled on other platforms:**
>  - The shared code in java.awt.List stores elements and selection separately, 
> so it accepts invalid indexes. This can cause exceptions if the selection and 
> data become out of sync.
>  - On Windows, all invalid indexes are ignored, except for the special value 
> -1, which is used to select or deselect all elements. This happens because 
> the indexes are passed to the win api without validation.
>  - XAWT uses the same logic as the shared code, so it can throw the same 
> exceptions if the data...

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 six additional 
commits since the last revision:

 - Merge branch 'openjdk:master' into JDK-8369327
 - Update ProblemList.txt
 - Update SelectInvalid.java
 - Validate "index >= 0 && index < model.getSize()"
 - Create SelectInvalid.java
 - 8369327: On macOS List may loses selection when added to Frame

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/27682/files
  - new: https://git.openjdk.org/jdk/pull/27682/files/3e3543ef..e639b90e

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=27682&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=27682&range=00-01

  Stats: 101136 lines in 1595 files changed: 57105 ins; 34314 del; 9717 mod
  Patch: https://git.openjdk.org/jdk/pull/27682.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/27682/head:pull/27682

PR: https://git.openjdk.org/jdk/pull/27682

Reply via email to