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

[Prasanta Sadhukhan]

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/

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/
>>>
>>> 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/20200729/63614cf4/attachment-0001.htm>