<Swing Dev> [9] Review request for 8057791: Selection in JList is drawn with wrong colors in Nimbus L&F
Anton Litvinov
anton.litvinov at oracle.com
Tue Jun 21 17:55:48 UTC 2016
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
>>
>
>
More information about the swing-dev
mailing list