<Swing Dev> [9] Review request for 8057791: Selection in JList is drawn with wrong colors in Nimbus L&F

[Anton Litvinov]

Hello Sergey and Semyon,

Sergey, thank you for review of this fix. In the 1st version of the fix 
I decided to return "null", because at least one implementation of 
"javax.swing.plaf.synth.SynthStyle" which is 
"sun.swing.plaf.synth.DefaultSynthStyle" returns "null" from its 
implementation of "getColorForState(SynthContext, ColorType)", but while 
I worked on the fix, I did not read the comment in "NimbusStyle.java" 
which you quoted. Yes, I agree that it is incorrect to return "null" 
from the method "NimbusStyle.getColorForState".

Adding support of "Selected", "Disabled+Selected" states to 
"List.cellRenderer" component in 
"src/java.desktop/share/classes/javax/swing/plaf/nimbus/skin.laf" file 
was considered before the review request of the 1st fix version, but it 
was defined that in "SwingSet2" demo selected items in the list were 
still drawn with wrong colors. However today I defined the reason of 
this issue in "SwingSet2" demo, and it is caused by the fact that in the 
demo a custom extension of "javax.swing.DefaultListCellRenderer" class 
is used instead of the default 
"javax.swing.plaf.synth.SynthListUI.SynthListCellRenderer" which is 
capable of setting "SynthConstants.SELECTED" bit mask to its state 
through the call sequence 
"SynthListCellRenderer.getListCellRendererComponent" --> 
"SynthLookAndFeel.setSelectedUI".

The 2nd version of the fix was created. Could you please review the 2nd 
version of the fix.

Webrev 01: http://cr.openjdk.java.net/~alitvinov/8057791/jdk9/webrev.01

In the new fix version the regression test stayed unchanged. The new fix 
version adds support of "Selected", "Disabled+Selected" states to 
"List.cellRenderer" component in 
"src/java.desktop/share/classes/javax/swing/plaf/nimbus/skin.laf" to 
make "NimbusStyle.getColorForState" method return correct colors for 
"SynthListCellRenderer" instances used in "JList" in Nimbus L&F.

The following 2 sets of regression tests were executed on Windows platform:
1. Automatic regression tests from open and closed sets located in 
"javax/swing/JList", "javax/swing/plaf/nimbus" directories.
2. JCK tests:
     api/javax_swing/CellRendererPane
     api/javax_swing/DefaultCellEditor
     api/javax_swing/DefaultListCellRenderer
     api/javax_swing/DefaultListModel
     api/javax_swing/DefaultListSelectionModel
     api/javax_swing/JList
     api/javax_swing/plaf/ListUI
     api/javax_swing/plaf/nimbus
     api/javax_swing/plaf/synth/SynthListUI

Thank you,
Anton

On 6/20/2016 3:13 PM, Sergey Bylokhov wrote:
> Hi, Anton.
> I am a little bit worried about the change in 
> NimbusStyle.getColorForState(). Before the fix we intentionally never 
> return null from this method, but return DEFAULT_COLOR which has this 
> javadoc:
>     /**
>      * <p>The Color to return from getColorForState if it would 
> otherwise have
>      * returned null.</p>
>      *
>      * <p>Returning null from getColorForState is a very bad thing, as 
> it causes
>      * the AWT peer for the component to install a SystemColor, which 
> is not a
>      * UIResource. As a result, if <code>null</code> is returned from
>      * getColorForState, then thereafter the color is not updated for 
> other
>      * states or on LAF changes or updates. This DEFAULT_COLOR is used to
>      * ensure that a ColorUIResource is always returned from
>      * getColorForState.</p>
>      */
>
> And after the fix this method will return null for all ListCellRenderer.
>
> Can you please clarify why the change states of "List.cellRenderer" in 
> plaf/nimbus/skin.laf does not solve the problem?
>
>> On 6/13/2016 12:12 AM, Anton Litvinov wrote:
>>> Hello,
>>>
>>> Could you please review the following fix for the bug.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8057791
>>> Webrev: http://cr.openjdk.java.net/~alitvinov/8057791/jdk9/webrev.00
>>>
>>> The bug is the fact that selected elements in "javax.swing.JList"
>>> component are drawn with wrong background and foreground colors,
>>> though "JList.getSelectionForeground()",
>>> "JList.getSelectionBackground()" methods return the correct color
>>> values for the tested JList instance.
>>>
>>> Wrong colors are returned in run time from the method
>>> "javax.swing.plaf.synth.SynthStyle.getColor" for
>>> "javax.swing.plaf.synth.SynthListUI.SynthListCellRenderer" instance,
>>> because it can be only in 1 state "SynthConstants.ENABLED" and cannot
>>> be in "SynthConstants.SELECTED" state, therefore the call from this
>>> method to
>>> "javax.swing.plaf.nimbus.NimbusStyle.getColorForState(SynthContext,
>>> ColorType)" method always returns some wrong default L&F foreground,
>>> background colors which are observed in this bug.
>>>
>>> All automatic regression tests from open and closed sets located in
>>> "javax/swing/JList", "javax/swing/plaf/nimbus" directories were run on
>>> MS Windows 7 OS during verification of the fix.
>>>
>>> Thank you,
>>> Anton
>>
>
>