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

Thu Nov 16 01:21:05 UTC 2017

On 15/11/2017 10:53, Semyon Sadetsky wrote:
>> 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.

This will not be a bug, if the application thinks that 2x2 or 200x200 
are reasonable values for maximum/minimum/preferred sizes and will set 
it, then these values will be used.

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

-- 
Best regards, Sergey.