+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.
