Hello Sergey, <<So the problem is that we need to reinstall all UI on "horizontalTextPosition" event?(what exact property is missing?) No, this is not the issue. The issue is that the updateIcon method is not supposed to be called in SynthLookAndFeel. SynthMenuItemUI class has overridden installDefaults of BasicMenuItemUI class and it never calls the code in updateCheckIcon even when that code was part of BasicMenuItemUI installDefaults. WindowsMenuItemUI has not overridden installDefauls, so this code is called for WindowsLookAndFeel. The issue is that during the fix of [1], the changes were made in code which is called for all LookAndFeels, though the issue is only in WindowsLookAndFeel. The fix should have been specific to WindowsLookAndFeel.
<< 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. webrev: http://cr.openjdk.java.net/~pbansal/8216329/webrev01/ [1] https://bugs.openjdk.java.net/browse/JDK-8152981 Regards, Pankaj -----Original Message----- From: Sergey Bylokhov Sent: Monday, January 27, 2020 1:15 PM To: Pankaj Bansal; swing-dev@openjdk.java.net Subject: Re: <Swing Dev> [15] RFR JDK-8216329: Cannot resize CheckBoxItemMenu in Synth L&F with setHorizontalTextPosition On 1/26/20 10:33 pm, Pankaj Bansal wrote: > Hello Sergey, > > << The updateCheckIcon() was a private method in the public class from the > javax.swing.plaf.basic package you cannot simply make it protected. > The updateCheckIcon function was made from installDefaults function while > fixing [1], so that it can be called from multiple places [2]. > installDefaults is a protected function only, so I have not exposed any code > which was not exposed earlier. So I think it should not be an issue to do > this. 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. > > << Can you please provide some more details why the call to updateCheckIcon() > break the resize? > In the fix, we are not reinstalling the full UI, but are changing it > partially. If we reinstall it fully as done in first iteration of review for > [1], this works fine[3]. So the problem is that we need to reinstall all UI on "horizontalTextPosition" event?(what exact property is missing?) > > [1] https://bugs.openjdk.java.net/browse/JDK-8152981 > [2] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/5fb24aaf6945 > [3] http://cr.openjdk.java.net/~rchamyal/8152981/webrev.00/ > > > Regards, > Pankaj > > -----Original Message----- > From: Sergey Bylokhov > Sent: Friday, January 24, 2020 5:55 AM > To: Pankaj Bansal; swing-dev@openjdk.java.net > Subject: Re: <Swing Dev> [15] RFR JDK-8216329: Cannot resize > CheckBoxItemMenu in Synth L&F with setHorizontalTextPosition > > Hi, Pankaj. > > The updateCheckIcon() was a private method in the public class from the > javax.swing.plaf.basic package you cannot simply make it protected. > > Can you please provide some more details why the call to updateCheckIcon() > break the resize? > > > On 1/23/20 3:05 am, Pankaj Bansal wrote: >> Hi All, >> >> Please review the following fix for jdk15. >> >> >> Bug: >> >> https://bugs.openjdk.java.net/browse/JDK-8216329 >> >> webrev: >> >> http://cr.openjdk.java.net/~pbansal/8216329/webrev00/ >> >> Issue: >> >> The JCheckBoxMenuItem is not being resized properly when >> setHorizontalTextPosition is called on it. This results in some text getting >> truncated from the end. The issue is specific to Synth L&F. >> >> Cause: >> >> The present bug is a regression of [1]. >> >> Fix: >> >> The problem in [1] was specific to Windows L&F. but the fix was done in >> BasicMenuItemUI which is shared by all the L&F. The current fix moves the >> fix done for [1] to Windows L&F specific code. This fixes the current issue. >> I have run the tests that was added for [1]. All works fine. >> >> [1] https://bugs.openjdk.java.net/browse/JDK-8152981 >> >> >> Regards, >> Pankaj Bansal >> > > > -- > Best regards, Sergey. > -- Best regards, Sergey.
