OK.

On 18-Feb-20 3:58 PM, Pankaj Bansal wrote:
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)?




Reply via email to