<Swing Dev> [9] Review request for 8057791: Selection in JList is drawn with wrong colors in Nimbus L&F
Alexander Potochkin
alexander.potochkin at oracle.com
Wed Jun 22 14:56:21 UTC 2016
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
>>>
>>
>>
>
More information about the swing-dev
mailing list