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




Reply via email to