RFR: 8235491: Tree/TableView: implementation of isSelected(int) violates contract [v6]

[Kevin Rushforth]

On Wed, 3 Aug 2022 23:28:24 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> 1. reword SelectionModel.isSelected(int) javadoc, removing incorrect statement "Is functionally equivalent to calling <code>getSelectedIndices().contains(index)</code>." 
>> 2. reimplement TableView.TableViewSelectionModel.isSelected(int) method to return true when at least one cell in *any* column is selected on the given row (was: *all* columns)
>> 3. change selectRowWhenInSingleCellSelectionMode() and selectRowWhenInSingleCellSelectionMode2() in TableViewSelectionModelImplTest to reflect new reality.
>> 
>> NOTE: proposed change alters semantics of isSelected(int) method (in the right direction, in my opinion).
>
> Andy Goryachev 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 11 additional commits since the last revision:
> 
>  - 8235491: whitespace
>  - 8235491: additional tests
>  - Merge remote-tracking branch 'origin/master' into 8235491.isselected
>  - 8235491: javadoc
>  - 8235491: tree table view
>  - Merge remote-tracking branch 'origin/master' into 8235491.isselected
>  - 8235491: review comments
>  - 8235491: whitespace
>  - 8235491: javadoc
>  - 8235491: 2022
>  - ... and 1 more: https://git.openjdk.org/jfx/compare/240a9eba...ad3c70b9

> a bit confused about the csr - shouldn't that be focused entirely on SelectionModel.isSelected)? That's where we clarify the contract - all changes to TableXXSelectionModels are implementation changes, fixing their contract violation. So I would expect these not to be mentioned at all in the csr.

The specification section of the CSR should (and does) list only the spec change to `SelectionModel.isSelected`. Since there could be an impact to applications that rely on the current behavior, it also seems reasonable to list those in the Solution section (as well as the Compatibility concerns section). I think that the Summary should start by listing the spec change to `SelectionModel.isSelected`, but it seems useful to also say that the implementation of `TableXXSelectionModel` is changing as a result.

> BTW: isSelected is not a convenience method - that's already changed here, all mentions of its being so should be removed from the csr as well, IMO :)

Agreed.

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

PR: https://git.openjdk.org/jfx/pull/839