Hello Prasanta, Thanks for the review. I don’t think this field needs to be accessed outside the class windowsMenuItemUI. So we should keep it private only. Please let me know if you think otherwise, else I will be pushing this as it is.
Regards, Pankaj -----Original Message----- From: Prasanta Sadhukhan Sent: Tuesday, February 18, 2020 3:40 PM To: Pankaj Bansal <[email protected]>; [email protected] Subject: Re: <Swing Dev> [15] RFR JDK-8216329: Cannot resize CheckBoxItemMenu in Synth L&F with setHorizontalTextPosition Looks ok to me. Only thing I have some doubt is "propertyChangeListener" object is "private" in windowsMenuItemUI whereas it was "protected" in BasicMenuItemUI. I guess it should be protected too. Regards Prasanta On 08-Feb-20 4:14 AM, Sergey Bylokhov wrote: > Looks fine. > > On 2/5/20 5:00 am, Pankaj Bansal wrote: >> Hello Sergey, >> >> << ok. >> <<Do we need to remove the listener added to the menuItem? >> <<I guess it will be added every time we change L&F to the windows >> and will never be removed. >> >> >> Yes, it should be done. >> Webrev: http://cr.openjdk.java.net/~pbansal/8216329/webrev02/ >> >> Regards, >> Pankaj >> >> -----Original Message----- >> From: Sergey Bylokhov >> Sent: Thursday, January 30, 2020 5:59 AM >> To: Pankaj Bansal <[email protected]>; >> [email protected] >> Subject: Re: <Swing Dev> [15] RFR JDK-8216329: Cannot resize >> CheckBoxItemMenu in Synth L&F with setHorizontalTextPosition >> >> On 1/29/20 2:25 am, Pankaj Bansal wrote: >>> One more point, I am able to reproduce the current issue with Synth >>> LookAndFeel in all platforms without fix and it works fine with the >>> fix. >> >> ok. >> >> Do we need to remove the listener added to the menuItem? >> I guess it will be added every time we change L&F to the windows and >> will never be removed. >> >> >>> >>> Regards, >>> Pankaj >>> >>> -----Original Message----- >>> From: Pankaj Bansal >>> Sent: Wednesday, January 29, 2020 3:19 PM >>> To: Sergey Bylokhov; [email protected] >>> Subject: Re: <Swing Dev> [15] RFR JDK-8216329: Cannot resize >>> CheckBoxItemMenu in Synth L&F with setHorizontalTextPosition >>> >>> Hello Sergey, >>> >>> << Can you please double check that it is not possible to reproduce >>> JDK-8152981 even if the test is modified in some way? >>> <<For example if some other "basic" L&F will be used(Motif, Aqua)? >>> I changed the test in JDK-8152981 to run on all installed >>> LookAndFeels on windows, linux and Mac after removing the windows >>> only condition. The tests passes on all platforms with all >>> LookAndFeels with the current fix. >>> I can check in this change in JDK-8152981 test along with the >>> current fix if needed, though I feel it is not required as the issue >>> was originally only in WindowsLookAndFeel. >>> >>> Regards, >>> Pankaj Bansal >>> >>> -----Original Message----- >>> From: Sergey Bylokhov >>> Sent: Wednesday, January 29, 2020 1:17 PM >>> To: Pankaj Bansal; [email protected] >>> Subject: Re: <Swing Dev> [15] RFR JDK-8216329: Cannot resize >>> CheckBoxItemMenu in Synth L&F with setHorizontalTextPosition >>> >>> On 1/28/20 4:33 pm, Sergey Bylokhov wrote: >>>> On 1/27/20 7:15 am, Pankaj Bansal wrote: >>>>> << It is not a big issue, but for such a fix we will need a proper >>>>> specification and CSR, it is like adding a new method to the >>>>> public class. It is preferable to try to fix it in some other way >>>>> first. >>>>> I did not realize earlier that this can be done by making changes >>>>> in WindowsMenuItemUI without calling the updateCheckIcon by moving >>>>> the code in updateCheckIcon method in WindowsMenuItemUI class. I >>>>> have made the changes for the same and all works fine. Also, I >>>>> have removed the updateCheckIcon method from BasicMenuItemUI class >>>>> as it is not needed. >>>> >>>> Can you please double check that it is not possible to reproduce >>>> JDK-8152981 even if the test is modified in some way? >>> >>> For example if some other "basic" L&F will be used(Motif, Aqua)? >>> >>> >> >> > >
