<Swing Dev> RFR: 8271315: Redo: Nimbus JTree renderer properties persist across L&F changes

[Alexey Ivanov]

On Thu, 19 Aug 2021 06:13:39 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

> This is another attempt of fixing JDK-8249674 which created a regression JDK-8266510 for which it was backed out.
> This fix reinstates the UIResource properties  for "Tree.leafIcon", "Tree.closedIcon",  "Tree.openIcon", "Tree.selectionForeground",
>  "Tree.textForeground", "Tree.selectionBackground", "Tree.textBackground", "Tree.selectionBorderColor" for NimbusLookAndFeel.
> 
> The regression which was "when a JTree node is selected, the text should be painted using the selected text color (white), matching the color of the expansion control. Instead, it is painted using the standard color (black)"  is also fixed by this fix, although I could not find any mention of this behaviour in the spec that selected text color should match the expanded icon color
> but it's a long standing behaviour, so catered to it.
> It was happening because SynthLabelUI#paint when it tries to paint textForeground for cell label, it calls
> 
> g.setColor(context.getStyle().getColor(context,
>                     ColorType.TEXT_FOREGROUND));
> 
> 
> which then calls SynthStyle#getColor where even though cell renderer correctly gets the foreground color, it gets overridden by getColorForState()  because the color is a UIResource and there's no corresponding color defined for that state for Tree.CellRenderer so it uses default "black" color for "text" as defined in skin.laf
> 
> else if (type == ColorType.TEXT_FOREGROUND) {
>                 color = c.getForeground();
>             }
>         }
> 
>         if (color == null || color instanceof UIResource) {
>             // Then use what we've locally defined
>             color = getColorForState(context, type);
>         }
> 
> 
> Proposed fix is to check if current foregroundColor for DefaultTreeCellRenderer is UIResouce, then use that color itself.

Marked as reviewed by aivanov (Reviewer).

test/jdk/javax/swing/plaf/nimbus/NimbusPropertiesDoNotImplUIResource.java line 45:

> 43:             "Tree.textBackground", "Tree.selectionBorderColor"};
> 44: 
> 45:     private static String failedKeys = null;

This is unnecessary, you initialise it to `null` before the loop over the properties.

test/jdk/javax/swing/plaf/nimbus/NimbusPropertiesDoNotImplUIResource.java line 58:

> 56:                     verifyProperty(propertyKey);
> 57:                 }
> 58:                 if(failedKeys != null) {

Suggestion:

                if (failedKeys != null) {

And in other places too, please.

test/jdk/javax/swing/plaf/nimbus/NimbusPropertiesDoNotImplUIResource.java line 63:

> 61:                             + LF.getClassName());
> 62:                 }
> 63:             } catch(UnsupportedLookAndFeelException e) {

Suggestion:

            } catch (UnsupportedLookAndFeelException e) {

test/jdk/javax/swing/plaf/nimbus/NimbusPropertiesDoNotImplUIResource.java line 70:

> 68: 
> 69:         //Check that the both uiResource option true and false are working for
> 70:         //getDerivedColor method of NimbusLookAndFeel

Suggestion:

        // Check that both uiResource options true and false work for
        // getDerivedColor method of NimbusLookAndFeel

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

PR: https://git.openjdk.java.net/jdk/pull/5178