<Swing Dev> RFR: 8249674: Redo: Nimbus JTree renderer properties persist across L&F changes

[Philip Race]

We've been trying to get away from using bug ids in path names or the names
of the tests themselves.

-phil

On 7/29/20, 4:53 AM, Prasanta Sadhukhan wrote:
>
>
> On 29-Jul-20 1:25 PM, Tejpal Rebari wrote:
>> Hi Prasanta,
>>
>>> On 27-Jul-2020, at 2:21 PM, Prasanta Sadhukhan 
>>> <prasanta.sadhukhan at oracle.com 
>>> <mailto:prasanta.sadhukhan at oracle.com>> wrote:
>>>
>>> Fix looks ok to me. But I think the test is more appropriate if it 
>>> is placed in javax/swing/plaf/nimbus directory. Also, the test can 
>>> be enhanced a bit to not fail at the first attempt as we are testing 
>>> 8 different properties, so I think it will be good if you will store 
>>> the results in a string for each property that fails, and finally at 
>>> the end throw failure with stored string.
>>>
>> I was bit confused about where the test should be placed because it 
>> also checks for other LAFs.
>> But for other LAF test was passing earlier also so keeping the test 
>> in javax/swing/plaf/nimbus.
>>
> We normally place the test in the area where the fix is being made.
>> Made changes in the test to throw failure with stored string.
>>
>> Updated webrev : 
>> http://cr.openjdk.java.net/~trebari/swing/8249674/webrev01/ 
>> <http://cr.openjdk.java.net/%7Etrebari/swing/8249674/webrev01/>
>
> Looks ok but please use "," instead of "/t"
>
> and replace "/t" with ":" if "failedkeys" is null. No need of another 
> webrev for me.
>
> Regards
> Prasanta
>>
>> Thanks
>> Tejpal
>>
>>
>>> Regards
>>> Prasanta
>>> On 24-Jul-20 2:36 PM, Tejpal Rebari wrote:
>>>> Hi All,
>>>> Please review the following fix for jdk16.
>>>>
>>>> Bug : https://bugs.openjdk.java.net/browse/JDK-8249674
>>>> Webrev : 
>>>> http://cr.openjdk.java.net/~trebari/swing/8249674/webrev00/ 
>>>> <http://cr.openjdk.java.net/%7Etrebari/swing/8249674/webrev00/>
>>>>
>>>> This is a modified fix for 
>>>> https://bugs.openjdk.java.net/browse/JDK-8041701
>>>> which caused some internal test to fail.
>>>>
>>>> Issue : UI properties “Tree.leafIcon”, “Tree.closedIcon”, 
>>>> “Tree.openIcon”,
>>>> “Tree.selectionForeground” ..etc are supposed to be instances of 
>>>> UIResource
>>>> if they are defined by LAF.
>>>>
>>>> In Nimbus LAF all the above properties are defined , but none of 
>>>> them implement
>>>> UIResource, this causes them to persists when LAF is changed.
>>>> This leaves artefacts when switching from nimbus to other LAFs.
>>>>
>>>> Fix :  Fix is to make use of UIResource for the 
>>>> abovementioned uiProperties.
>>>> Making NimbuIcon implements the UIResource was working fine earlier
>>>> so keeping the change same.
>>>>
>>>> The second part of the earlier fix "class DerivedColor extends 
>>>> Color implements UIResource"
>>>> caused the internal test failure,
>>>>
>>>> So changed this fix to keep the DerivedColor class same and for the 
>>>> uiProperties
>>>> Tree.selectionForeground” ..etc  use the UIResource
>>>> class that is inside the DerivedColor. These changes were made in 
>>>> skin.laf file.
>>>>
>>>> Testing : Tested on all three platform, and all internal tests.Link 
>>>> is in JBS.
>>>> In test also added the part which was the reason of revert of 
>>>> earlier fix.
>>>> The test fails with the fix of 8041701 and passes with the current fix.
>>>>
>>>>
>>>> Regards
>>>> Tejpal
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/swing-dev/attachments/20200801/3bcbc9d9/attachment.htm>