<Swing Dev> [9] Review request for JDK-8152981 Double icons with JMenuItem setHorizontalTextPosition on Win 10

[Alexandr Scherbatiy]

The fix looks good to me.

Thanks,
Alexandr.

On 5/16/2016 6:49 PM, Sergey Bylokhov wrote:
> Looks fine.
>
> On 11.05.16 11:40, Rajeev Chamyal wrote:
>> Hello Sergey,
>>
>> Please review the updated webrev.
>>
>> http://cr.openjdk.java.net/~rchamyal/8152981/webrev.02/
>>
>>   -  The empty catch block "catch (Exception e) {}, it will be better 
>> to re-throw an exception.
>> Updated the catch code to throw exception.
>>
>>   - "frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE)" can cause 
>> a test failure in some jtreg modes(see JDK-8154365)
>> Removed
>>
>>   - Is it necessary to make this test "win only", can it cover other 
>> look and feels and platforms?
>> This issue is with Windows look and feel only. For other LAF and 
>> platforms it works fine.
>>
>> Regards,
>> Rajeev Chamyal
>>
>>
>> -----Original Message-----
>> From: Sergey Bylokhov
>> Sent: 05 May 2016 18:54
>> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev at openjdk.java.net
>> Subject: Re: <Swing Dev> [9] Review request for JDK-8152981 Double 
>> icons with JMenuItem setHorizontalTextPosition on Win 10
>>
>> The code change looks fine, a few notes about the test:
>>   -  The empty catch block "catch (Exception e) {}, it will be better 
>> to re-throw an exception.
>>   - "frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE)" can cause 
>> a test failure in some jtreg modes(see JDK-8154365)
>>   - Is it necessary to make this test "win only", can it cover other 
>> look and feels and platforms?
>>
>> On 05.05.16 15:36, Rajeev Chamyal wrote:
>>> Hello Sergey,
>>>
>>> Please review the updated webrev.
>>> http://cr.openjdk.java.net/~rchamyal/8152981/webrev.01/
>>>
>>> Update: Added the check Icon install code in installDefaults to a 
>>> private method. Calling same method on propertychange.
>>>
>>> Regards,
>>> Rajeev Chamyal
>>>
>>> -----Original Message-----
>>> From: Sergey Bylokhov
>>> Sent: 04 May 2016 19:47
>>> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev at openjdk.java.net
>>> Subject: Re: <Swing Dev> [9] Review request for JDK-8152981 Double
>>> icons with JMenuItem setHorizontalTextPosition on Win 10
>>>
>>> Hi, Rajeev.
>>> Is it necessary to reinstall UI for the component from the UI 
>>> itself, probably it will be possible to reconfigure the current one?
>>>
>>> On 03.05.16 15:08, Rajeev Chamyal wrote:
>>>> Hello All,
>>>>
>>>>
>>>>
>>>> Please review the below webrev.
>>>>
>>>>
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8152981
>>>>
>>>> Webrev : http://cr.openjdk.java.net/~rchamyal/8152981/webrev.00/
>>>>
>>>>
>>>>
>>>> Issue : Setting horizontal text position on a menuItem with icon
>>>> results in Icon appearing twice on menuItem.
>>>>
>>>>
>>>>
>>>> Cause: When HorizontalTextPosition is set to LEFT/CENTER on MenuItem
>>>> with an Icon, MenuItemUI defaults are not updated to set correct
>>>> columnlayout .
>>>>
>>>> Which results in setting of check Icon on menuItem and 2 icons are
>>>> shown on MenuItem.
>>>>
>>>>
>>>>
>>>> Fix : installing the UI defaults again if HorizontalTextPosition is
>>>> set on MenuItem.
>>>>
>>>>
>>>>
>>>> Regards,
>>>>
>>>> Rajeev Chamyal
>>>>
>>>
>>>
>>> -- 
>>> Best regards, Sergey.
>>>
>>
>>
>> -- 
>> Best regards, Sergey.
>>
>
>