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

[Tejpal Rebari]

Gentle Reminder.

> On 04-Aug-2020, at 11:46 AM, Tejpal Rebari <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/~trebari/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 <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 <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 above mentioned 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/20200810/ab750309/attachment.htm>