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> 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 >>>
