Re: [9] Review request for JDK-8152981 Double icons with JMenuItem setHorizontalTextPosition on Win 10

2016-05-23 Thread 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@openjdk.java.net
Subject: Re:  [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@openjdk.java.net
Subject: Re:  [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.








Re: [9] Review request for JDK-8152981 Double icons with JMenuItem setHorizontalTextPosition on Win 10

2016-05-16 Thread Sergey Bylokhov

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@openjdk.java.net
Subject: Re:  [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@openjdk.java.net
Subject: Re:  [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.




--
Best regards, Sergey.


Re: [9] Review request for JDK-8152981 Double icons with JMenuItem setHorizontalTextPosition on Win 10

2016-05-11 Thread Rajeev Chamyal
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@openjdk.java.net
Subject: Re:  [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@openjdk.java.net
> Subject: Re:  [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.


Re: [9] Review request for JDK-8152981 Double icons with JMenuItem setHorizontalTextPosition on Win 10

2016-05-05 Thread Sergey Bylokhov

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@openjdk.java.net
Subject: Re:  [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.


Re: [9] Review request for JDK-8152981 Double icons with JMenuItem setHorizontalTextPosition on Win 10

2016-05-05 Thread Rajeev Chamyal
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@openjdk.java.net
Subject: Re:  [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.


Re: [9] Review request for JDK-8152981 Double icons with JMenuItem setHorizontalTextPosition on Win 10

2016-05-04 Thread Sergey Bylokhov

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.


Re: [9] Review request for JDK-8152981 Double icons with JMenuItem setHorizontalTextPosition on Win 10

2016-05-04 Thread Alexander Scherbatiy


  The fix looks good to me.

  Thanks,
  Alexandr,

On 5/3/2016 3:08 PM, 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





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

2016-05-03 Thread Rajeev Chamyal
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