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

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Wed Nov 15 16:24:31 UTC 2017


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


-- 
Best regards, Sergey.



More information about the swing-dev mailing list