<Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout flickers when label text shows "..." and is updated

Wed Nov 15 18:53:36 UTC 2017

On 11/15/2017 09:29 AM, Sergey Bylokhov wrote:

> On 15/11/2017 08:58, Semyon Sadetsky wrote:
>> Hi Krishna& Sergey,
>>
>> The provided reg test passes on Ubuntu Linux before the fix.
>
> I have checked it on my Ubuntu and it always does not pass. If it pass 
> in your case then you should cooperate with Krishna and check what 
> happened.
>
>>
>> Yet, I have a question about the assumption I get from the proposed 
>> solution that any component  placed into GridBagLayout must have its 
>> getPrefferredSize() equals to its getMinimumSize() result. Do we have 
>> this assumption documented somewhere?
>
> There are no such restrictions that getMinimumSize() must return exact 
> value of getPrefferredSize(). It should return some reasonable value 
> instead of null, which can be considered as 1x1 by some layout 
> managers. In most cases the PrefferredSize is used for this purpose.
This doesn't correspond to what I see if add

menu.setMinimumSize(new Dimension(2,2));

After that the problem described in the bug comes back.
>
>>
>> --Semyon
>>
>>
>> On 11/15/2017 08:24 AM, Sergey Bylokhov wrote:
>>> Looks fine.
>>>
>>> On 15/11/2017 07:26, Krishna Addepalli wrote:
>>>> Hi Sergey,
>>>>
>>>> Per our conversation, I have added the volatile keyword to the 
>>>> Boolean flag. Here is the updated webrev: 
>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev04/
>>>>
>>>> Thanks,
>>>> Krishna
>>>>
>>>> -----Original Message-----
>>>> From: Krishna Addepalli
>>>> Sent: Wednesday, November 15, 2017 6:07 PM
>>>> To: Sergey Bylokhov <sergey.bylokhov at oracle.com>; 
>>>> swing-dev at openjdk.java.net
>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout 
>>>> flickers when label text shows "..." and is updated
>>>>
>>>> Hi Sergey,
>>>>
>>>> I have modified the code as per your requests, and here is the 
>>>> updated webrev: 
>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev03/
>>>>
>>>> However, I have a couple questions regarding the fix:
>>>> 1. Why was it considered good to fix it in BasicMenuUI rather than 
>>>> in BasicMenuUIItem?
>>>> 2. While using Robot.waitForIdle(), I came across the function 
>>>> "Suntoolkit.flushPendingEvents" function. But this is a private 
>>>> function and applications cannot use this. Is it expected that the 
>>>> applications should import the robot module to flush the events, 
>>>> considering that mostly it is used for testing purposes? If this is 
>>>> the case, then "Suntoolkit.flushPendingEvents" becomes a valuable 
>>>> tool, given that Swing rendering happens in an EDT that is separate 
>>>> from MainThread.
>>>>
>>>> Thanks,
>>>> Krishna
>>>>
>>>> -----Original Message-----
>>>> From: Sergey Bylokhov
>>>> Sent: Wednesday, November 15, 2017 2:23 AM
>>>> To: Krishna Addepalli <krishna.addepalli at oracle.com>; 
>>>> swing-dev at openjdk.java.net
>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout 
>>>> flickers when label text shows "..." and is updated
>>>>
>>>> Hi, Krishna.
>>>> Small comments: I think that this code
>>>> "(JMenu)menuItem).isTopLevelMenu() == true"
>>>> can be simplified to
>>>> (JMenu)menuItem).isTopLevelMenu().
>>>>
>>>> In the test you create and dispose JFrame and other components 
>>>> outside of EDT thread. I suggest to use invokeAndWait() which will 
>>>> work synchronously. Instead of sleep() you van use 
>>>> Robot.waitForIdle();
>>>>
>>>>
>>>> On 12/11/2017 22:45, Krishna Addepalli wrote:
>>>>> Hi Sergey,
>>>>>
>>>>> Gentle reminder! Could you review the fix?
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>> -----Original Message-----
>>>>> From: Krishna Addepalli
>>>>> Sent: Wednesday, November 8, 2017 8:22 PM
>>>>> To: Sergey Bylokhov <sergey.bylokhov at oracle.com>;
>>>>> swing-dev at openjdk.java.net
>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>> flickers when label text shows "..." and is updated
>>>>>
>>>>> Hi Sergey,
>>>>>
>>>>> Are the changes fine now?
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>> -----Original Message-----
>>>>> From: Krishna Addepalli
>>>>> Sent: Monday, November 6, 2017 9:02 PM
>>>>> To: Sergey Bylokhov <sergey.bylokhov at oracle.com>;
>>>>> swing-dev at openjdk.java.net
>>>>> Subject: RE: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>> flickers when label text shows "..." and is updated
>>>>>
>>>>> Hi Sergey,
>>>>>
>>>>> As per your recommendation I have done the following changes:
>>>>> 1. Moved the fix to BasicMenuUI.
>>>>> 2. Corrected the test to:
>>>>>     a. Access the UI elements only in the EDT.
>>>>>     b. Made sure that the main thread doesn't exit till the test 
>>>>> is complete.
>>>>>
>>>>> Here is the updated webrev:
>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev02/
>>>>>
>>>>> However, I was wondering, why it is good to fix in BasicMenuUI? 
>>>>> Also, while writing the testcase, I couldnot find any 
>>>>> functionality like "flush" on the event queue, which will execute 
>>>>> all the pending events. Is it by design?
>>>>>
>>>>> Thanks,
>>>>> Krishna
>>>>>
>>>>> -----Original Message-----
>>>>> From: Sergey Bylokhov
>>>>> Sent: Friday, November 3, 2017 11:24 PM
>>>>> To: Krishna Addepalli <krishna.addepalli at oracle.com>;
>>>>> swing-dev at openjdk.java.net
>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>> flickets when label text shows "..." and is updated
>>>>>
>>>>> Hi, Krishna.
>>>>> A few notes:
>>>>>     - Looks like the bug can be reproduced in JMenu only? Then it 
>>>>> will
>>>>> be good to fix it in BasicMenuUI, moreover it is already 
>>>>> implemented a
>>>>> getMaximumSize() in a similar way as in your fix.
>>>>>     - In the test some of the Swing components are accessed on 
>>>>> non-EDT
>>>>> thread. Also note that when you start the Thread you will exit
>>>>> invokeAndWait() and it is possible that the test will end before the
>>>>> Thread complete(jtreg will kill the test when the main thread is
>>>>> completed)
>>>>>
>>>>> On 31/10/2017 05:00, Krishna Addepalli wrote:
>>>>>> Hi Sergey,
>>>>>>
>>>>>> Thanks for pointing that out. Fixed the test case and created a new
>>>>>> webrev here: 
>>>>>> http://cr.openjdk.java.net/~kaddepalli/8178430/webrev01/
>>>>>>
>>>>>> Now, the testcase throws an exception and also after a few 
>>>>>> trials, closes on its own.
>>>>>>
>>>>>> Thanks,
>>>>>> Krishna
>>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Sergey Bylokhov
>>>>>> Sent: Tuesday, October 31, 2017 1:13 AM
>>>>>> To: Krishna Addepalli <krishna.addepalli at oracle.com>;
>>>>>> swing-dev at openjdk.java.net
>>>>>> Subject: Re: <Swing Dev> [10][JDK-8178430] JMenu in GridBagLayout
>>>>>> flickets when label text shows "..." and is updated
>>>>>>
>>>>>> Hi, Krishna.
>>>>>> Can you please clarify in what situation the test will stop 
>>>>>> working(it does not have any assertions)?
>>>>>>
>>>>>> On 27/10/2017 02:45, Krishna Addepalli wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> Please review the fix for bug:
>>>>>>>
>>>>>>>
>>>>>>> Bug: JDK-8178430: https://bugs.openjdk.java.net/browse/JDK-8178430
>>>>>>>
>>>>>>> Webrev: http://cr.openjdk.java.net/~kaddepalli/8178430/webrev00/
>>>>>>>
>>>>>>> Summary:
>>>>>>>
>>>>>>> The issue is when the label text width is more than the
>>>>>>> container(JPanel) width, the container tries to render with minimum
>>>>>>> width for all the components. In such case, the JMenuItem, which is
>>>>>>> added to the JMenuBar also returns its height dimension as 1 (the
>>>>>>> default minimum). The test case alternates between the short text
>>>>>>> and long text on the label, and it gives a flickering effect of 
>>>>>>> the Menu.
>>>>>>> The fix is to return preferred size from JMenuItem, if its 
>>>>>>> parent is
>>>>>>> a JMenuBar, since JMenuBar is added to the top level window.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Krishna
>>>>>>>
>>>>>>
>>>>>>
>>>>>> -- 
>>>>>> Best regards, Sergey.
>>>>>>
>>>>>
>>>>>
>>>>> -- 
>>>>> Best regards, Sergey.
>>>>>
>>>>
>>>>
>>>> -- 
>>>> Best regards, Sergey.
>>>>
>>>
>>>
>>
>
>