Hi Prasanta,

> On 27-Jul-2020, at 2:21 PM, Prasanta Sadhukhan 
> <prasanta.sadhuk...@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.

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/~trebari/swing/8249674/webrev01/>

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/~trebari/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

Reply via email to