Gentle Reminder.
> On 04-Aug-2020, at 11:46 AM, Tejpal Rebari <tejpal.reb...@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.r...@oracle.com
>> <mailto:philip.r...@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.sadhuk...@oracle.com <mailto: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.
>>>>
>>> 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
>>>>
>