On Tue, 18 Mar 2025 05:24:39 GMT, Abhishek Kumar <abhis...@openjdk.org> wrote:

> If there are no rows then most probably `selectAll` will select `column 
> headers`.. right?
> 
> I don't understand why do we want to select all columns if there are no rows 
> or vice-versa ? Does it return any useful details?
> 
> Few changes can be done in the fix and test code (if fix is applicable)

Spec says "Selects all rows, columns, and cells in the table." so it should 
mean we should select all available cells in the "table" and not header, which 
is separately handled via JTableHeader..

> src/java.desktop/share/classes/javax/swing/JTable.java line 2199:
> 
>> 2197:             SwingUtilities2.setLeadAnchorWithoutSelection(selModel, 
>> oldLead, oldAnchor);
>> 2198: 
>> 2199:             selModel.setValueIsAdjusting(false);
> 
> Recently added code is a subset of the code block in `if block`. Duplicate 
> code can be refactored with helper methods.

Duplication removed

> test/jdk/javax/swing/JTable/TestTableSelectAll.java line 43:
> 
>> 41: 
>> 42:         // TableModel with no rows, but 10 columns
>> 43:         DefaultTableModel data = new DefaultTableModel(0, 10);
> 
> Proposed fix tries to handle two cases:
> 1. rowCount > 0 and columnCount = 0
> 2. columnCount > 0 and rowCount = 0
> 
> but test code covers only second case (rowCount = 0).
> 
> Test should be extended to verify both cases.

Extended..

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

PR Comment: https://git.openjdk.org/jdk/pull/24025#issuecomment-2731758311
PR Review Comment: https://git.openjdk.org/jdk/pull/24025#discussion_r2000259177
PR Review Comment: https://git.openjdk.org/jdk/pull/24025#discussion_r2000259518

Reply via email to