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

[Sergey Bylokhov]

+1

On 22.06.16 17:56, Alexander Potochkin wrote:
>
> Hello Anton
>
> I like the latest version of this fix!
> It looks good to me.
>
> Thanks
> alexp
>
> On 6/21/2016 20:55, Anton Litvinov wrote:
>> 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
>>>>
>>>
>>>
>>
>


-- 
Best regards, Sergey.