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

Wed Nov 15 16:58:33 UTC 2017

Hi Krishna& Sergey,

The provided reg test passes on Ubuntu Linux before the fix.

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?

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