On Thu, 19 Aug 2021 06:13:39 GMT, Prasanta Sadhukhan <psadhuk...@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
