On Wed, 12 Nov 2025 07:56:33 GMT, Prasanta Sadhukhan <[email protected]> 
wrote:

> `JTree.updateUI` calculate item sizes from the cell renderer, but it doesn't 
> call `updateUI `on the cell renderer until afterwards. This leads to 
> incorrect size calculation, which is observed when we switch from one L&F to 
> another where it is seen that all tree items are slightly too cramped, with 
> too little space between rows and text are abbreviated.
> Fix is to ensure `JTree.updateUI` update the cell renderer before updating 
> the UI. 
> CI testing is ok..
> 
> Before fix
> <img width="142" height="146" alt="image" 
> src="https://github.com/user-attachments/assets/95d43e47-122a-4ca4-8a3f-f4bf5c1d3f43";
>  />
> 
> After fix
> <img width="145" height="125" alt="image" 
> src="https://github.com/user-attachments/assets/068b5988-119c-4b99-be27-1ef5d52a28a2";
>  />

The change itself looks straight-forward and clean. Testing looks OK. Left some 
minor suggestions.

test/jdk/javax/swing/JTree/JTreeUpdateTest.java line 27:

> 25:  * @test
> 26:  * @bug 8042054
> 27:  * @summary JTree.updateUI sould use updated item size information

Suggestion:

 * @summary JTree.updateUI should use updated item size information

test/jdk/javax/swing/JTree/JTreeUpdateTest.java line 49:

> 47:         and JTree items are cramped with little space between rows
> 48:         then press Fail.
> 49:         If the left JTree is identical with right JTree, press Pass.

Suggestion:

        A frame with two identical JTrees is shown.
        If the left JTree's text is abbreviated and JTree items are
        cramped with little space between rows then press Fail.
        If the left JTree is identical to the right JTree, press Pass.


Looks a little strange with where the line breaks are. Some are long and some 
are short. And phrasing.

test/jdk/javax/swing/JTree/JTreeUpdateTest.java line 62:

> 60:     }
> 61: 
> 62:     static JFrame createUI() {

Suggestion:

    private static JFrame createUI() {

Explicitly set to public or private.

test/jdk/javax/swing/JTree/JTreeUpdateTest.java line 89:

> 87:                 SwingUtilities.updateComponentTreeUI(tree2);
> 88:             }
> 89:         });

Suggestion:

        SwingUtilities.invokeLater(() -> {
            setLaf("javax.swing.plaf.nimbus.NimbusLookAndFeel");
            SwingUtilities.updateComponentTreeUI(frame);
            SwingUtilities.updateComponentTreeUI(tree2);
        });

Can replace with lambda expression.

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

PR Review: https://git.openjdk.org/jdk/pull/28258#pullrequestreview-3457190833
PR Review Comment: https://git.openjdk.org/jdk/pull/28258#discussion_r2521286133
PR Review Comment: https://git.openjdk.org/jdk/pull/28258#discussion_r2521291600
PR Review Comment: https://git.openjdk.org/jdk/pull/28258#discussion_r2521308239
PR Review Comment: https://git.openjdk.org/jdk/pull/28258#discussion_r2521297975

Reply via email to