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

[Philip Race]

Ok. +1

-phil.

On 8/10/20, 8:34 AM, Tejpal Rebari wrote:
> Gentle Reminder.
>
>> On 04-Aug-2020, at 11:46 AM, Tejpal Rebari <tejpal.rebari at oracle.com 
>> <mailto:tejpal.rebari at oracle.com>> wrote:
>>
>> Hi Phil,
>> I have updated the path of the test.
>> Updated webrev : 
>> http://cr.openjdk.java.net/~trebari/swing/8249674/webrev02/ 
>> <http://cr.openjdk.java.net/%7Etrebari/swing/8249674/webrev02/>
>>
>> Thanks
>> Tejpal
>>
>>> On 01-Aug-2020, at 9:41 PM, Philip Race <philip.race at oracle.com 
>>> <mailto:philip.race at oracle.com>> wrote:
>>>
>>> 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/20200813/dc940ff1/attachment-0001.htm>