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

[Tejpal Rebari]

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> 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/20200804/e14b5f01/attachment-0001.htm>