Re: [9] RFR JDK-8168657: [PIT] Still, on Windows test always fails: java/awt/SplashScreen/MultiResolutionSplash/unix/UnixMultiResolutionSplashTest.java

[Rajeev Chamyal]

Looks good to me.

Regards,
Rajeev Chamyal

-Original Message-
From: Prasanta Sadhukhan 
Sent: 26 October 2016 12:54
To: Rajeev Chamyal; Alexandr Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] RFR JDK-8168657: [PIT] Still, on Windows test 
always fails: 
java/awt/SplashScreen/MultiResolutionSplash/unix/UnixMultiResolutionSplashTest.java

Please review this.

diff -r aae3690e53e3
test/java/awt/SplashScreen/MultiResolutionSplash/unix/UnixMultiResolutionSplashTest.java
---
a/test/java/awt/SplashScreen/MultiResolutionSplash/unix/UnixMultiResolutionSplashTest.java
Thu Oct 20 14:21:46 2016 +0300
+++ 
b/test/java/awt/SplashScreen/MultiResolutionSplash/unix/UnixMultiResolutionSplashTest.java
Wed Oct 26 12:53:21 2016 +0530
@@ -44,9 +44,11 @@
  import javax.imageio.ImageIO;

  /**
- * @test @bug 8145174 8151787
+ * @test
+ * @bug 8145174 8151787 8168657
   * @summary HiDPI splash screen support on Linux
   * @modules java.desktop/sun.java2d
+ * @requires (os.family == "linux")
   * @run main UnixMultiResolutionSplashTest
   */
  public class UnixMultiResolutionSplashTest {

Regards
Prasanta
On 10/26/2016 12:49 PM, Prasanta Sadhukhan wrote:
> Hi All,
>
> Please review a simple fix for splasscreen testissue where we need to 
> restrict this test from running on linux as this test uses linux 
> enviroment variable GDK_SCALE so restricting to run only on linux.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8168657
>
> diff -r aae3690e53e3
> test/java/awt/SplashScreen/MultiResolutionSplash/unix/UnixMultiResolut
> ionSplashTest.java
> ---
> a/test/java/awt/SplashScreen/MultiResolutionSplash/unix/UnixMultiResol
> utionSplashTest.java
> Thu Oct 20 14:21:46 2016 +0300
> +++ 
> b/test/java/awt/SplashScreen/MultiResolutionSplash/unix/UnixMultiResol
> utionSplashTest.java
> Wed Oct 26 12:46:56 2016 +0530
> @@ -47,6 +47,7 @@
>   * @test @bug 8145174 8151787
>   * @summary HiDPI splash screen support on Linux
>   * @modules java.desktop/sun.java2d
> + * @requires (os.family == "linux")
>   * @run main UnixMultiResolutionSplashTest
>   */
>  public class UnixMultiResolutionSplashTest {
>
>
> Regards
> Prasanta

Re: [9] Review request for JDK-8150176 [hidpi] wrong resolution variant of multi-res. image is used for TrayIcon

[Rajeev Chamyal]

Hello Sergey,

I have removed the html file.
http://cr.openjdk.java.net/~rchamyal/8150176/webrev.03/ 

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 14 September 2016 23:39
To: Alexandr Scherbatiy; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8150176 [hidpi] wrong 
resolution variant of multi-res. image is used for TrayIcon

Is "MultiResolutionTrayIconTest.html" should be removed?

On 14.09.16 17:44, Alexandr Scherbatiy wrote:
> The fix looks good to me.
>
> Thanks,
> Alexandr.
>
> On 9/14/2016 12:01 PM, Rajeev Chamyal wrote:
>>
>> Hello Alexandr,
>>
>>
>>
>> Thanks for the review.
>>
>> Please review the webrev updated as per review comments.
>>
>> http://cr.openjdk.java.net/~rchamyal/8150176/webrev.03/
>> <http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.03/>
>>
>>
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>>
>>
>> *From:*Alexandr Scherbatiy
>> *Sent:* 12 September 2016 20:07
>> *To:* Rajeev Chamyal; swing-dev@openjdk.java.net; Sergey Bylokhov
>> *Subject:* Re:  [9] Review request for JDK-8150176 [hidpi] 
>> wrong resolution variant of multi-res. image is used for TrayIcon
>>
>>
>>
>> On 9/9/2016 3:06 PM, Rajeev Chamyal wrote:
>>
>> Hello Alexandr,
>>
>>
>>
>> Please review the updated webrev.
>>
>> http://cr.openjdk.java.net/~rchamyal/8150176/webrev.02/
>> <http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.02/>
>>
>> WTrayIconPeer.java
>> +gr.drawImage(image, 0, 0, (autosize ? w :
>> image.getWidth(observer)),
>> + (autosize ? h :
>> image.getHeight(observer)), observer);
>>
>> The w and h are scaled. It looks like the image.getWidth(observer) 
>> and
>> image.getHeight(observer) also should be scaled in the same way.
>>
>>  MultiResolutionTrayIconTest.java
>> +latch.await();
>>
>> It is better to add a timeout here.
>>
>> Thanks,
>> Alexandr.
>>
>>
>>
>>
>>
>>
>> Update: Updated webrev is fixing the issue in windows only.
>>
>> We have a separate bug linux and it will be fixed through a
>> separate webrev.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8154551
>>
>>
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>>
>>
>> *From:*Alexandr Scherbatiy
>> *Sent:* 14 June 2016 15:21
>> *To:* Rajeev Chamyal; swing-dev@openjdk.java.net
>> <mailto:swing-dev@openjdk.java.net>; Sergey Bylokhov
>> *Subject:* Re:  [9] Review request for JDK-8150176
>> [hidpi] wrong resolution variant of multi-res. image is used for
>> TrayIcon
>>
>>
>>
>> On 6/13/2016 3:18 PM, Rajeev Chamyal wrote:
>>
>>
>> Hello Alexandr,
>>
>>
>>
>> Thanks for the review. I have updated the webrev as per review
>> comments.
>>
>> http://cr.openjdk.java.net/~rchamyal/8150176/webrev.01/
>> <http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.01/>
>>
>> I tried drawing the image directly to paint graphics without
>> buffered image and it was getting cropped.
>>
>>
>> Did you paint it using non scaled width and height?
>>   g.drawImage(image, 0, 0, curW, curH, null);
>> Is the g transform already scaled?
>>
>> XTrayIconPeer:
>>
>> 606 if (scaleX > 1.0 && scaleY > 1.0) {
>>
>> 607 resolutionVariant =
>> ((MultiResolutionImage) image).
>>
>>
>>It is better to change the condition to "image instanceof
>> MultiResolutionImage". It is necessary to not get CCE for non
>> multi-resolution image and the multi-resolution image should
>> return the best resolution variant for any scale.
>>
>> 618         gr.drawImage(resolutionVariant, 0, 0,
>>
>> 619 curW, curH, observer);
>>
>>   The width and height should be scaled here to draw to whole
>> buffered image.
>>
>> WTrayIconPeer:
>>
>>  133     BufferedImage bufImage = new
>> BufferedImage(TRAY_ICON_WIDTH, TRAY_ICON_HEIGHT,
>>  134
>> BufferedImage.TYPE_INT_ARGB);
>>
>>  The size for the buffer

Re: [9] Review request for JDK-8150176 [hidpi] wrong resolution variant of multi-res. image is used for TrayIcon

[Rajeev Chamyal]

Hello Alexandr,

 

Thanks for the review.

Please review the webrev updated as per review comments.

http://cr.openjdk.java.net/~rchamyal/8150176/webrev.03/ 

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 12 September 2016 20:07
To: Rajeev Chamyal; swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  [9] Review request for JDK-8150176 [hidpi] wrong 
resolution variant of multi-res. image is used for TrayIcon

 

On 9/9/2016 3:06 PM, Rajeev Chamyal wrote:



Hello Alexandr,

 

Please review the updated webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.02/"http://cr.openjdk.java.net/~rchamyal/8150176/webrev.02/

WTrayIconPeer.java
+gr.drawImage(image, 0, 0, (autosize ? w : 
image.getWidth(observer)),
+ (autosize ? h : image.getHeight(observer)), 
observer);

The w and h are scaled. It looks like the image.getWidth(observer) and 
image.getHeight(observer) also should be scaled in the same way.

 MultiResolutionTrayIconTest.java
+latch.await();

It is better to add a timeout here.

Thanks,
Alexandr.




 

 

Update: Updated webrev is fixing the issue in windows only.

We have a separate bug linux and it will be fixed through a separate webrev.

https://bugs.openjdk.java.net/browse/JDK-8154551

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 14 June 2016 15:21
To: Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  [9] Review request for JDK-8150176 [hidpi] wrong 
resolution variant of multi-res. image is used for TrayIcon

 

On 6/13/2016 3:18 PM, Rajeev Chamyal wrote:




Hello Alexandr,

 

Thanks for the review. I have updated the webrev as per review comments.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.01/"http://cr.openjdk.java.net/~rchamyal/8150176/webrev.01/

I tried drawing the image directly to paint graphics without buffered image and 
it was getting cropped.


Did you paint it using non scaled width and height?
  g.drawImage(image, 0, 0, curW, curH, null);
Is the g transform already scaled?

XTrayIconPeer:

 606 if (scaleX > 1.0 && scaleY > 1.0) {
 607 resolutionVariant = ((MultiResolutionImage) image).


   It is better to change the condition to "image instanceof 
MultiResolutionImage". It is necessary to not get CCE for non multi-resolution 
image and the multi-resolution image should return the best resolution variant 
for any scale.

 618 gr.drawImage(resolutionVariant, 0, 0,
 619 curW, curH, observer);

  The width and height should be scaled here to draw to whole buffered image.

WTrayIconPeer:

 133 BufferedImage bufImage = new BufferedImage(TRAY_ICON_WIDTH, 
TRAY_ICON_HEIGHT,
 134
BufferedImage.TYPE_INT_ARGB);

 The size for the buffered image should be scaled in the same was as for 
XTrayIconPeer.
 It may require to update the native code as well to set proper high-resolution 
icon. 

Thanks,
Alexandr.






 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 09 June 2016 20:43
To: Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  [9] Review request for JDK-8150176 [hidpi] wrong 
resolution variant of multi-res. image is used for TrayIcon

 

On 6/9/2016 11:55 AM, Rajeev Chamyal wrote:





Hello All,

 

Please review the following fix.

Bug: https://bugs.openjdk.java.net/browse/JDK-8150176 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8150176/webrev.00/
 

 

Issue: Wrong resolution variant image is used in Tray Icon.

Fix : Applying the device transform to graphics object to select the correct 
image.

The image could be cropped on Linux because the high resolution icon which 
size is bigger that the original image is drawn to the buffered image with 
un-scaled size curW x CurH. 
  It is better to get a resolution variant from the multi-resolution image, 
draw it to a buffered image with the same scaled size and then draw the 
buffered image to the paint graphics using original size:
---
Image resolutionVariant = ((MultiResolutionImage) 
image).getResolutionVariant(scaleX * curW, scaleY * curH);
BufferedImage bufImage = new BufferedImage(scaleX * curW, scaleY * curH, 
BufferedImage.TYPE_INT_ARGB);
// ...
gr.drawImage(image, 0, 0, scaleX * curW, scaleY * curH, observer);
// ...
g.drawImage(bufImage, 0, 0, curW, curH, observer); // non scaled width and 
height
---

  By the way, is the buffered image necessary in this case? Is it possible to 
draw the image directly to the paint graphics?
---
 g.drawImage(image, 0, 0, curW, curH, null);
---

Thanks,
Alexandr.
  

 

Regards,

Rajeev Chamyal

 

 

 

 

Re: [9] Review request for JDK-8150176 [hidpi] wrong resolution variant of multi-res. image is used for TrayIcon

[Rajeev Chamyal]

Hello Alexandr,

 

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8150176/webrev.02/

 

 

Update: Updated webrev is fixing the issue in windows only.

We have a separate bug linux and it will be fixed through a separate webrev.

https://bugs.openjdk.java.net/browse/JDK-8154551

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 14 June 2016 15:21
To: Rajeev Chamyal; swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  [9] Review request for JDK-8150176 [hidpi] wrong 
resolution variant of multi-res. image is used for TrayIcon

 

On 6/13/2016 3:18 PM, Rajeev Chamyal wrote:



Hello Alexandr,

 

Thanks for the review. I have updated the webrev as per review comments.

http://cr.openjdk.java.net/~rchamyal/8150176/webrev.01/

I tried drawing the image directly to paint graphics without buffered image and 
it was getting cropped.


Did you paint it using non scaled width and height?
  g.drawImage(image, 0, 0, curW, curH, null);
Is the g transform already scaled?

XTrayIconPeer:

 606 if (scaleX > 1.0 && scaleY > 1.0) {
 607 resolutionVariant = ((MultiResolutionImage) image).


   It is better to change the condition to "image instanceof 
MultiResolutionImage". It is necessary to not get CCE for non multi-resolution 
image and the multi-resolution image should return the best resolution variant 
for any scale.

 618 gr.drawImage(resolutionVariant, 0, 0,
 619 curW, curH, observer);

  The width and height should be scaled here to draw to whole buffered image.

WTrayIconPeer:

 133 BufferedImage bufImage = new BufferedImage(TRAY_ICON_WIDTH, 
TRAY_ICON_HEIGHT,
 134
BufferedImage.TYPE_INT_ARGB);

 The size for the buffered image should be scaled in the same was as for 
XTrayIconPeer.
 It may require to update the native code as well to set proper high-resolution 
icon. 

Thanks,
Alexandr.





 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 09 June 2016 20:43
To: Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  [9] Review request for JDK-8150176 [hidpi] wrong 
resolution variant of multi-res. image is used for TrayIcon

 

On 6/9/2016 11:55 AM, Rajeev Chamyal wrote:




Hello All,

 

Please review the following fix.

Bug: https://bugs.openjdk.java.net/browse/JDK-8150176 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8150176/webrev.00/
 

 

Issue: Wrong resolution variant image is used in Tray Icon.

Fix : Applying the device transform to graphics object to select the correct 
image.

The image could be cropped on Linux because the high resolution icon which 
size is bigger that the original image is drawn to the buffered image with 
un-scaled size curW x CurH. 
  It is better to get a resolution variant from the multi-resolution image, 
draw it to a buffered image with the same scaled size and then draw the 
buffered image to the paint graphics using original size:
---
Image resolutionVariant = ((MultiResolutionImage) 
image).getResolutionVariant(scaleX * curW, scaleY * curH);
BufferedImage bufImage = new BufferedImage(scaleX * curW, scaleY * curH, 
BufferedImage.TYPE_INT_ARGB);
// ...
gr.drawImage(image, 0, 0, scaleX * curW, scaleY * curH, observer);
// ...
g.drawImage(bufImage, 0, 0, curW, curH, observer); // non scaled width and 
height
---

  By the way, is the buffered image necessary in this case? Is it possible to 
draw the image directly to the paint graphics?
---
 g.drawImage(image, 0, 0, curW, curH, null);
---

Thanks,
Alexandr.
  

 

Regards,

Rajeev Chamyal

 

 

 

Re: 8163274: [TEST_BUG][macosx] apparent regression: javax/swing/JColorChooser/Test7194184.java

[Rajeev Chamyal]

Looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 08 September 2016 11:00
To: Rajeev Chamyal
Cc: Alexandr Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  8163274: [TEST_BUG][macosx] apparent regression: 
javax/swing/JColorChooser/Test7194184.java

 

A gentle reminder, please review code changes as indicated in new webrev.

 

With Regards,

Avik Niyogi

 

On 07-Sep-2016, at 9:50 pm, Alexandr Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

The fix looks good to me.

Thanks,
Alexandr.

On 9/7/2016 9:02 AM, Avik Niyogi wrote:

Hi All, 

Kindly review the updated fix for JDK9 with new inputs incorporated.

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8163274/webrev.01/"http://cr.openjdk.java.net/~aniyogi/8163274/webrev.01/

 

With Regards,

Avik Niyogi

 

On 07-Sep-2016, at 11:03 am, Rajeev Chamyal mailto:rajeev.cham...@oracle.com"rajeev.cham...@oracle.com> wrote:

 

Looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 06 September 2016 16:16
To: Avik Niyogi; Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re:  8163274: [TEST_BUG][macosx] apparent regression: 
javax/swing/JColorChooser/Test7194184.java

 

The fix looks good to me.

Thanks,
Alexandr.

On 9/6/2016 9:12 AM, Avik Niyogi wrote:

Hi All,

 

Kindly review the fix for JDK9.  

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8163274

 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8163274/webrev.00/"http://cr.openjdk.java.net/~aniyogi/8163274/webrev.00/

 

Issue: This test javax/swing/JColorChooser/Test7194184.java throws exception 
due to event delay.

 

Cause:  Delay for event idling was not added.

 

Fix: Appropriate changes for autoWaitForIdle() were added.

 

With Regards,

Avik Niyogi

 

 

 

 

Re: 8163274: [TEST_BUG][macosx] apparent regression: javax/swing/JColorChooser/Test7194184.java

[Rajeev Chamyal]

Looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 06 September 2016 16:16
To: Avik Niyogi; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re:  8163274: [TEST_BUG][macosx] apparent regression: 
javax/swing/JColorChooser/Test7194184.java

 

The fix looks good to me.

Thanks,
Alexandr.

On 9/6/2016 9:12 AM, Avik Niyogi wrote:

Hi All,

 

Kindly review the fix for JDK9.  

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8163274

 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8163274/webrev.00/"http://cr.openjdk.java.net/~aniyogi/8163274/webrev.00/

 

Issue: This test javax/swing/JColorChooser/Test7194184.java throws exception 
due to event delay.

 

Cause:  Delay for event idling was not added.

 

Fix: Appropriate changes for autoWaitForIdle() were added.

 

With Regards,

Avik Niyogi

 

Re: 8163161: [PIT][TEST_BUG] increase timeout in javax/swing/plaf/nimbus/8057791/bug8057791.java

[Rajeev Chamyal]

Looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 24 August 2016 14:14
To: Rajeev Chamyal
Cc: Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  8163161: [PIT][TEST_BUG] increase timeout in 
javax/swing/plaf/nimbus/8057791/bug8057791.java

 

Hi All,

A gentle reminder, please review my code change.

 

With Regards,

Avik Niyogi

On 23-Aug-2016, at 2:28 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

The fix looks good to me.

Thanks,
Alexandr.

On 23/08/16 10:38, Avik Niyogi wrote:

Hi All,

 

Kindly review the fix for JDK9.  

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8163161

 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8163161/webrev.00/"http://cr.openjdk.java.net/~aniyogi/8163161/webrev.00/

 

Issue: This test javax/swing/plaf/nimbus/8057791/bug8057791.java times out

 

Cause:  No timeout delay was provided so it would fail on other systems.

 

Fix: Appropriate changes for timeout were added.

 

With Regards,

Avik Niyogi

 

 

Re: Swing Dev>[9] Review Request JDK-8163160 [PIT][TEST_BUG] Some issues in java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

[Rajeev Chamyal]

Hello Yuri,

Thanks for +1.
I have changed the Color to green.

Regards,
Rajeev Chamyal

-Original Message-
From: Yuri Nesterenko 
Sent: 19 August 2016 17:38
To: Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy; 
swing-dev@openjdk.java.net
Subject: Re:  Swing Dev>[9] Review Request JDK-8163160 
[PIT][TEST_BUG] Some issues in 
java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

Indeed. I tried it myself, and it is yellow for me, too...
but, on a snapshot and with my nose to the screen:-) I approve the test as it 
is but some other color perhaps could be more visible. I know the RED is taken 
but maybe green or orange? Anyway, +1.

-yan

On 08/19/2016 02:37 PM, Rajeev Chamyal wrote:
> Hello Yuri,
>
> I have attached a snapshot of button in JBS. The button icon border is Yellow.
> https://bugs.openjdk.java.net/browse/JDK-8163160
>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Yuri Nesterenko
> Sent: 19 August 2016 16:38
> To: Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy; 
> swing-dev@openjdk.java.net
> Subject: Re:  Swing Dev>[9] Review Request JDK-8163160 
> [PIT][TEST_BUG] Some issues in 
> java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java
>
> Hi Rajeev,
>
> I tried this version of the test on two Ubuntu 16.04 systems with Unity and a 
> promoted b132.
> Now, the Launcher icon is blue with yellow border around it -- but the icon 
> on the frame with label "Test" is blue with gray border. I think you should 
> either change instructions even more or look into that issue (if there's an 
> issue).
>
> Thank you,
> -yan
>
> On 08/19/2016 08:06 AM, Rajeev Chamyal wrote:
>> Hello Yuri,
>>
>> Can you please review below webrev.
>> Webrev: http://cr.openjdk.java.net/~rchamyal/8163160/webrev.00/
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -Original Message-
>> From: Rajeev Chamyal
>> Sent: 16 August 2016 18:45
>> To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net
>> Subject: Re:  Swing Dev>[9] Review Request JDK-8163160 
>> [PIT][TEST_BUG] Some issues in 
>> java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java
>>
>> Hello Sergey,
>>
>> Thanks for the review. In the bug its reported that border of button is grey.
>> Instead of button border icon border should be checked. I have updated test 
>> instructions for this.
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -Original Message-
>> From: Sergey Bylokhov
>> Sent: 16 August 2016 18:28
>> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
>> Subject: Re: Swing Dev>[9] Review Request JDK-8163160 [PIT][TEST_BUG] 
>> Some issues in 
>> java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java
>>
>> On 16.08.16 15:57, Sergey Bylokhov wrote:
>>> The changes looks fine, there are a notice, is it a known issues?
>>
>> there are a notice about different background of the buttons.
>>
>>>
>>> On 16.08.16 15:01, Rajeev Chamyal wrote:
>>>> Hello All,
>>>>
>>>>
>>>>
>>>> Please review the following webrev.
>>>>
>>>>
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~rchamyal/8163160/webrev.00/
>>>>
>>>> Bug : https://bugs.openjdk.java.net/browse/JDK-8163160
>>>>
>>>> Issue : manual tag was missing in test.
>>>>
>>>>
>>>>
>>>> Regards,
>>>>
>>>> Rajeev Chamyal
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>

Re: Swing Dev>[9] Review Request JDK-8163160 [PIT][TEST_BUG] Some issues in java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

[Rajeev Chamyal]

Hello Yuri,

I have attached a snapshot of button in JBS. The button icon border is Yellow.
https://bugs.openjdk.java.net/browse/JDK-8163160 

Regards,
Rajeev Chamyal

-Original Message-
From: Yuri Nesterenko 
Sent: 19 August 2016 16:38
To: Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy; 
swing-dev@openjdk.java.net
Subject: Re:  Swing Dev>[9] Review Request JDK-8163160 
[PIT][TEST_BUG] Some issues in 
java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

Hi Rajeev,

I tried this version of the test on two Ubuntu 16.04 systems with Unity and a 
promoted b132.
Now, the Launcher icon is blue with yellow border around it -- but the icon on 
the frame with label "Test" is blue with gray border. I think you should either 
change instructions even more or look into that issue (if there's an issue).

Thank you,
-yan

On 08/19/2016 08:06 AM, Rajeev Chamyal wrote:
> Hello Yuri,
>
> Can you please review below webrev.
> Webrev: http://cr.openjdk.java.net/~rchamyal/8163160/webrev.00/
>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Rajeev Chamyal
> Sent: 16 August 2016 18:45
> To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net
> Subject: Re:  Swing Dev>[9] Review Request JDK-8163160 
> [PIT][TEST_BUG] Some issues in 
> java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java
>
> Hello Sergey,
>
> Thanks for the review. In the bug its reported that border of button is grey.
> Instead of button border icon border should be checked. I have updated test 
> instructions for this.
>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Sergey Bylokhov
> Sent: 16 August 2016 18:28
> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
> Subject: Re: Swing Dev>[9] Review Request JDK-8163160 [PIT][TEST_BUG] 
> Some issues in 
> java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java
>
> On 16.08.16 15:57, Sergey Bylokhov wrote:
>> The changes looks fine, there are a notice, is it a known issues?
>
> there are a notice about different background of the buttons.
>
>>
>> On 16.08.16 15:01, Rajeev Chamyal wrote:
>>> Hello All,
>>>
>>>
>>>
>>> Please review the following webrev.
>>>
>>>
>>>
>>> Webrev: http://cr.openjdk.java.net/~rchamyal/8163160/webrev.00/
>>>
>>> Bug : https://bugs.openjdk.java.net/browse/JDK-8163160
>>>
>>> Issue : manual tag was missing in test.
>>>
>>>
>>>
>>> Regards,
>>>
>>> Rajeev Chamyal
>>>
>>>
>>>
>>
>>
>
>

Re: Swing Dev>[9] Review Request JDK-8163160 [PIT][TEST_BUG] Some issues in java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

[Rajeev Chamyal]

Hello Yuri,

Can you please review below webrev.
Webrev: http://cr.openjdk.java.net/~rchamyal/8163160/webrev.00/

Regards,
Rajeev Chamyal

-Original Message-
From: Rajeev Chamyal 
Sent: 16 August 2016 18:45
To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  Swing Dev>[9] Review Request JDK-8163160 
[PIT][TEST_BUG] Some issues in 
java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

Hello Sergey,

Thanks for the review. In the bug its reported that border of button is grey. 
Instead of button border icon border should be checked. I have updated test 
instructions for this.

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 16 August 2016 18:28
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re: Swing Dev>[9] Review Request JDK-8163160 [PIT][TEST_BUG] Some 
issues in java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

On 16.08.16 15:57, Sergey Bylokhov wrote:
> The changes looks fine, there are a notice, is it a known issues?

there are a notice about different background of the buttons.

>
> On 16.08.16 15:01, Rajeev Chamyal wrote:
>> Hello All,
>>
>>
>>
>> Please review the following webrev.
>>
>>
>>
>> Webrev: http://cr.openjdk.java.net/~rchamyal/8163160/webrev.00/
>>
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8163160
>>
>> Issue : manual tag was missing in test.
>>
>>
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>>
>>
>
>


-- 
Best regards, Sergey.

Re: Review Request for 8163261: regression on Linux: java/awt/LightweightDispatcher/LWDispatcherMemoryLeakTest.java

[Rajeev Chamyal]

Hello Amarish,

Can you add this bug id to existing regression test case.

Regards,
Rajeev Chamyal
> On 15-Aug-2016, at 3:13 PM, Ambarish Rapte  wrote:
> 
> Hi,
> Please review fix for JDK9,
> Bug: https://bugs.openjdk.java.net/browse/JDK-8163261 
> <https://bugs.openjdk.java.net/browse/JDK-8163261>
> Webrev: http://cr.openjdk.java.net/~arapte/8163261/webrev.00/ 
> <http://cr.openjdk.java.net/~arapte/8163261/webrev.00/>
>  
> Issue:
> Reference to JButton was not getting collected by GC.
>  
> Cause:
> A strong reference to objects was held by 
> PainterMultiResolutionCachedImage.
> And the image reference was held by HashMap.
>  
> Fix:
> Changing the HashMap to WeakHashMap. Entry to WeakHashMap 
> gets removed after the object has no other strong reference.
>  
>  
>  
> Regards,
> Ambarish

Re: 8163169: [PIT][TEST_BUG] fix to JDK-8161470 doesn't work

[Rajeev Chamyal]

Looks ok to me.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 17 August 2016 12:26
To: Avik Niyogi; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re:  8163169: [PIT][TEST_BUG] fix to JDK-8161470 doesn't 
work

 

The fix looks good to me.

Thanks,
Alexandr.

On 8/17/2016 9:48 AM, Avik Niyogi wrote:

Hi All,

 

Kindly review the fix for JDK9. 

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8163169

 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8163169/webrev.00/"http://cr.openjdk.java.net/~aniyogi/8163169/webrev.00/

 

Issue: The test javax/swing/JRadioButton/FocusTraversal/FocusTraversal.java 
fails on OS X

 for PIT machines even though it works fine for local machine.

 

Cause:  As pointed out, this is an issue with the robot autoDelay.

 

Fix: Appropriate Robot methods were used to fix this with enough delays added 
to work on PIT machines.

 

With Regards,

Avik Niyogi

 

Re: Swing Dev>[9] Review Request JDK-8163160 [PIT][TEST_BUG] Some issues in java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

[Rajeev Chamyal]

Hello Sergey,

Thanks for the review. In the bug its reported that border of button is grey. 
Instead of button border icon border should be checked. I have updated test 
instructions for this.

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 16 August 2016 18:28
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re: Swing Dev>[9] Review Request JDK-8163160 [PIT][TEST_BUG] Some 
issues in java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

On 16.08.16 15:57, Sergey Bylokhov wrote:
> The changes looks fine, there are a notice, is it a known issues?

there are a notice about different background of the buttons.

>
> On 16.08.16 15:01, Rajeev Chamyal wrote:
>> Hello All,
>>
>>
>>
>> Please review the following webrev.
>>
>>
>>
>> Webrev: http://cr.openjdk.java.net/~rchamyal/8163160/webrev.00/
>>
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8163160
>>
>> Issue : manual tag was missing in test.
>>
>>
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>>
>>
>
>


-- 
Best regards, Sergey.

Swing Dev>[9] Review Request JDK-8163160 [PIT][TEST_BUG] Some issues in java/awt/image/multiresolution/MultiResolutionIcon/IconTest.java

[Rajeev Chamyal]

Hello All,

 

Please review the following webrev.

 

Webrev: http://cr.openjdk.java.net/~rchamyal/8163160/webrev.00/ 

Bug : https://bugs.openjdk.java.net/browse/JDK-8163160 

Issue : manual tag was missing in test.

 

Regards,

Rajeev Chamyal

 

Swing Dev>[9] Review Request JDK-8161913 [PIT] java/awt/Window/8159168/SetShapeTest.java mostly fails

[Rajeev Chamyal]

Hello All,

 

Please review the following webrev.

Bug: https://bugs.openjdk.java.net/browse/JDK-8161913 

Webrev : http://cr.openjdk.java.net/~rchamyal/8161913/webrev.00/

 

Issue : Test was failing when run repeatedly.

 

Fix: Added delay in test.

 

Regards,

Rajeev Chamyal

[9] Review Request JDK-8151787 Unify the HiDPI splash screen image naming convention

[Rajeev Chamyal]

Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8151787 

Webrev: http://cr.openjdk.java.net/~rchamyal/8151787/webrev.00/ 

 

Issue: Currently different naming conventions are used for Hidpi image on 
different platforms.

 

With this change the names will be unified across all platforms.

 

For a unscaled image image.ext following naming convention will be followed.

 

Unscaled name: image.ext

 

Supported Scaled Names:

If screen scale is integer number e.g. 2: HYPERLINK 
"mailto:im...@2x.ext"im...@2x.ext

If screen scale is float value like 1.25: HYPERLINK 
"mailto:im...@125pct.ext"im...@125pct.ext

 

Regards,

Rajeev Chamyal

 

 

Re: [9] Review Request JDK-8147648 [hidpi] multiresolution image: wrong resolution variant is used as icon in the Unity panel

[Rajeev Chamyal]

Linked to 8147648, added prefix Hidpi.

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 22 July 2016 15:51
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net; Sergey 
Bylokhov
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 7/22/2016 1:04 PM, Rajeev Chamyal wrote:

Hello Semyon,

 

Below is the bug id.

https://bugs.openjdk.java.net/browse/JDK-8162387

Please add [hidpi] prefix to the title. Also link it to the JDK-8147648.

--Semyon



 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 22 July 2016 15:21
To: Rajeev Chamyal; Alexander Scherbatiy; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 7/22/2016 12:14 PM, Rajeev Chamyal wrote:

Hello Semyon,

 

Your suggestion regarding _NET_WM_ICON requires some investigation and can be 
implemented as separate bug.

Ok. Please create this bug.

--Semyon




Could you please review the webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.03/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.03/

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 21 July 2016 20:42
To: Rajeev Chamyal; Semyon Sadetsky; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 7/21/2016 5:25 PM, Rajeev Chamyal wrote:





Hello Semyon,

 

The resolution variant image returned is based on the implementation of 
BaseMultiResolutionImage::getResolutionVariant API.

Current implementation of BaseMultiResolutionImage::getResolutionVariant 
returns a resolution variant image which has width and height greater than or 
equal to the passed width and height.

  There is a known issue on it:
JDK-8148619 Select the closest resolution variant in 
BaseMultiResolutionImage
  https://bugs.openjdk.java.net/browse/JDK-8148619

  Thanks,
  Alexandr.





In the case you have suggested dimensions of RED and BLUE images are 32 and  80 
respectively.

Width and height passed to getResolutionVariant is 64  i.e. scaled width and 
height of base image(RED) (GDK_SCALE=2) and blue image is getting returned.

The width and height passed to this  API is that of base image not of the spot.

Applications can control this behaviour by overriding this API in derived 
classes.

 

Regards,

Rajeev Chamyal 

 

From: Alexandr Scherbatiy 
Sent: 21 July 2016 15:09
To: Semyon Sadetsky; Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 7/21/2016 11:49 AM, Semyon Sadetsky wrote:






Hello Rajeev,

The taskbar icon is ok now.

I change the resolution variants from the test a bit:

final BaseMultiResolutionImage IMG = new 
BaseMultiResolutionImage(
new BufferedImage[]{generateImage(4, Color.RED), 
generateImage(10, Color.BLUE)});

And the icon I see in the taskbar and in the button is blue. It seems to me the 
first resolution variant (red) is more appropriate in this case because its 
size is closer to the spot. I'm not sure if this is an issue.

I have an extra question to you and Alexander.
Most native apps on Linux set an array of icons with _NET_WM_ICON. Usually they 
are [16x16, 32x32, 64x64].
So, desktop environment may select icon of appropriate size.
In this fix we are preselecting icon of a specific size in the app and send it 
to WM. 
Why not to send array of the resolution variants images and let the desktop 
environment to select the appropriate one, like native apps do?

  This sounds as good idea. MultiResolutionImage has the special method for 
this "List getResolutionVariants()". We do the similar on Mac OS X where 
NSImage with several representations is created from a MultiResolutionImage:
HYPERLINK 
"http://cr.openjdk.java.net/%7Ealexsch/8028212/webrev.01/src/macosx/classes/sun/lwawt/macosx/CImage.java.udiff.html"http://cr.openjdk.java.net/~alexsch/8028212/webrev.01/src/macosx/classes/sun/lwawt/macosx/CImage.java.udiff.html

 It has sense to try the same approach on Linux.

  Thanks,
  Alexandr.








--Semyon

On 19.07.2016 23:26, Rajeev Chamyal wrote:

Hello Semyon,

 

Please review the updated webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.03/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.03/

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 14 July 2016 16:58
To: Rajeev Chamyal; HYPERLINK 
"mailto:sw

Re: [9] Review Request JDK-8147648 [hidpi] multiresolution image: wrong resolution variant is used as icon in the Unity panel

[Rajeev Chamyal]

Hello Semyon,

 

Below is the bug id.

https://bugs.openjdk.java.net/browse/JDK-8162387

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 22 July 2016 15:21
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net; Sergey 
Bylokhov
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 7/22/2016 12:14 PM, Rajeev Chamyal wrote:

Hello Semyon,

 

Your suggestion regarding _NET_WM_ICON requires some investigation and can be 
implemented as separate bug.

Ok. Please create this bug.

--Semyon



Could you please review the webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.03/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.03/

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 21 July 2016 20:42
To: Rajeev Chamyal; Semyon Sadetsky; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 7/21/2016 5:25 PM, Rajeev Chamyal wrote:




Hello Semyon,

 

The resolution variant image returned is based on the implementation of 
BaseMultiResolutionImage::getResolutionVariant API.

Current implementation of BaseMultiResolutionImage::getResolutionVariant 
returns a resolution variant image which has width and height greater than or 
equal to the passed width and height.

  There is a known issue on it:
JDK-8148619 Select the closest resolution variant in 
BaseMultiResolutionImage
  https://bugs.openjdk.java.net/browse/JDK-8148619

  Thanks,
  Alexandr.




In the case you have suggested dimensions of RED and BLUE images are 32 and  80 
respectively.

Width and height passed to getResolutionVariant is 64  i.e. scaled width and 
height of base image(RED) (GDK_SCALE=2) and blue image is getting returned.

The width and height passed to this  API is that of base image not of the spot.

Applications can control this behaviour by overriding this API in derived 
classes.

 

Regards,

Rajeev Chamyal 

 

From: Alexandr Scherbatiy 
Sent: 21 July 2016 15:09
To: Semyon Sadetsky; Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 7/21/2016 11:49 AM, Semyon Sadetsky wrote:





Hello Rajeev,

The taskbar icon is ok now.

I change the resolution variants from the test a bit:

final BaseMultiResolutionImage IMG = new 
BaseMultiResolutionImage(
new BufferedImage[]{generateImage(4, Color.RED), 
generateImage(10, Color.BLUE)});

And the icon I see in the taskbar and in the button is blue. It seems to me the 
first resolution variant (red) is more appropriate in this case because its 
size is closer to the spot. I'm not sure if this is an issue.

I have an extra question to you and Alexander.
Most native apps on Linux set an array of icons with _NET_WM_ICON. Usually they 
are [16x16, 32x32, 64x64].
So, desktop environment may select icon of appropriate size.
In this fix we are preselecting icon of a specific size in the app and send it 
to WM. 
Why not to send array of the resolution variants images and let the desktop 
environment to select the appropriate one, like native apps do?

  This sounds as good idea. MultiResolutionImage has the special method for 
this "List getResolutionVariants()". We do the similar on Mac OS X where 
NSImage with several representations is created from a MultiResolutionImage:
HYPERLINK 
"http://cr.openjdk.java.net/%7Ealexsch/8028212/webrev.01/src/macosx/classes/sun/lwawt/macosx/CImage.java.udiff.html"http://cr.openjdk.java.net/~alexsch/8028212/webrev.01/src/macosx/classes/sun/lwawt/macosx/CImage.java.udiff.html

 It has sense to try the same approach on Linux.

  Thanks,
  Alexandr.







--Semyon

On 19.07.2016 23:26, Rajeev Chamyal wrote:

Hello Semyon,

 

Please review the updated webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.03/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.03/

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 14 July 2016 16:58
To: Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov; 
Alexander Scherbatiy
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

Hi Rajeev,

I have added 1px border to the icon in your test:

private static BufferedImage generateImage(int scale, Color c) {
int x = SZ * scale;
BufferedImage img = new BufferedImage(x, x, BufferedImage.TYPE_INT_RGB);
Graphics g = img.getGraphics();
if

Re: [9] Review Request JDK-8147648 [hidpi] multiresolution image: wrong resolution variant is used as icon in the Unity panel

[Rajeev Chamyal]

Hello Semyon,

 

Your suggestion regarding _NET_WM_ICON requires some investigation and can be 
implemented as separate bug.

Could you please review the webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.03/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.03/

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 21 July 2016 20:42
To: Rajeev Chamyal; Semyon Sadetsky; swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 7/21/2016 5:25 PM, Rajeev Chamyal wrote:



Hello Semyon,

 

The resolution variant image returned is based on the implementation of 
BaseMultiResolutionImage::getResolutionVariant API.

Current implementation of BaseMultiResolutionImage::getResolutionVariant 
returns a resolution variant image which has width and height greater than or 
equal to the passed width and height.

  There is a known issue on it:
JDK-8148619 Select the closest resolution variant in 
BaseMultiResolutionImage
  https://bugs.openjdk.java.net/browse/JDK-8148619

  Thanks,
  Alexandr.



In the case you have suggested dimensions of RED and BLUE images are 32 and  80 
respectively.

Width and height passed to getResolutionVariant is 64  i.e. scaled width and 
height of base image(RED) (GDK_SCALE=2) and blue image is getting returned.

The width and height passed to this  API is that of base image not of the spot.

Applications can control this behaviour by overriding this API in derived 
classes.

 

Regards,

Rajeev Chamyal 

 

From: Alexandr Scherbatiy 
Sent: 21 July 2016 15:09
To: Semyon Sadetsky; Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 7/21/2016 11:49 AM, Semyon Sadetsky wrote:




Hello Rajeev,

The taskbar icon is ok now.

I change the resolution variants from the test a bit:

final BaseMultiResolutionImage IMG = new 
BaseMultiResolutionImage(
new BufferedImage[]{generateImage(4, Color.RED), 
generateImage(10, Color.BLUE)});

And the icon I see in the taskbar and in the button is blue. It seems to me the 
first resolution variant (red) is more appropriate in this case because its 
size is closer to the spot. I'm not sure if this is an issue.

I have an extra question to you and Alexander.
Most native apps on Linux set an array of icons with _NET_WM_ICON. Usually they 
are [16x16, 32x32, 64x64].
So, desktop environment may select icon of appropriate size.
In this fix we are preselecting icon of a specific size in the app and send it 
to WM. 
Why not to send array of the resolution variants images and let the desktop 
environment to select the appropriate one, like native apps do?

  This sounds as good idea. MultiResolutionImage has the special method for 
this "List getResolutionVariants()". We do the similar on Mac OS X where 
NSImage with several representations is created from a MultiResolutionImage:
HYPERLINK 
"http://cr.openjdk.java.net/%7Ealexsch/8028212/webrev.01/src/macosx/classes/sun/lwawt/macosx/CImage.java.udiff.html"http://cr.openjdk.java.net/~alexsch/8028212/webrev.01/src/macosx/classes/sun/lwawt/macosx/CImage.java.udiff.html

 It has sense to try the same approach on Linux.

  Thanks,
  Alexandr.






--Semyon

On 19.07.2016 23:26, Rajeev Chamyal wrote:

Hello Semyon,

 

Please review the updated webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.03/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.03/

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 14 July 2016 16:58
To: Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov; 
Alexander Scherbatiy
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

Hi Rajeev,

I have added 1px border to the icon in your test:

private static BufferedImage generateImage(int scale, Color c) {
int x = SZ * scale;
BufferedImage img = new BufferedImage(x, x, BufferedImage.TYPE_INT_RGB);
Graphics g = img.getGraphics();
if (g != null) {
g.setColor(c);
g.fillRect(0, 0, x, x);
g.setColor(Color.YELLOW);
g.drawRect(0, 0, x-1, x-1);
}
return img;
}

It seems the icon in the taskbar is not correct for UI scale > 1.

By the way, graphics object should be disposed using g.dispose() when it is not 
needed anymore.

--Semyon





On 14.07.2016 10:08, Rajeev Chamyal wrote:

Hello All,

 

Gentle reminder. Please review the updated webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webr

Re: [9] Review Request JDK-8147648 [hidpi] multiresolution image: wrong resolution variant is used as icon in the Unity panel

[Rajeev Chamyal]

Hello Semyon,

 

The resolution variant image returned is based on the implementation of 
BaseMultiResolutionImage::getResolutionVariant API.

Current implementation of BaseMultiResolutionImage::getResolutionVariant 
returns a resolution variant image which has width and height greater than or 
equal to the passed width and height.

In the case you have suggested dimensions of RED and BLUE images are 32 and  80 
respectively.

Width and height passed to getResolutionVariant is 64  i.e. scaled width and 
height of base image(RED) (GDK_SCALE=2) and blue image is getting returned.

The width and height passed to this  API is that of base image not of the spot.

Applications can control this behaviour by overriding this API in derived 
classes.

 

Regards,

Rajeev Chamyal 

 

From: Alexandr Scherbatiy 
Sent: 21 July 2016 15:09
To: Semyon Sadetsky; Rajeev Chamyal; swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 7/21/2016 11:49 AM, Semyon Sadetsky wrote:



Hello Rajeev,

The taskbar icon is ok now.

I change the resolution variants from the test a bit:

final BaseMultiResolutionImage IMG = new 
BaseMultiResolutionImage(
new BufferedImage[]{generateImage(4, Color.RED), 
generateImage(10, Color.BLUE)});

And the icon I see in the taskbar and in the button is blue. It seems to me the 
first resolution variant (red) is more appropriate in this case because its 
size is closer to the spot. I'm not sure if this is an issue.

I have an extra question to you and Alexander.
Most native apps on Linux set an array of icons with _NET_WM_ICON. Usually they 
are [16x16, 32x32, 64x64].
So, desktop environment may select icon of appropriate size.
In this fix we are preselecting icon of a specific size in the app and send it 
to WM. 
Why not to send array of the resolution variants images and let the desktop 
environment to select the appropriate one, like native apps do?

  This sounds as good idea. MultiResolutionImage has the special method for 
this "List getResolutionVariants()". We do the similar on Mac OS X where 
NSImage with several representations is created from a MultiResolutionImage:

http://cr.openjdk.java.net/~alexsch/8028212/webrev.01/src/macosx/classes/sun/lwawt/macosx/CImage.java.udiff.html

 It has sense to try the same approach on Linux.

  Thanks,
  Alexandr.





--Semyon

On 19.07.2016 23:26, Rajeev Chamyal wrote:

Hello Semyon,

 

Please review the updated webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.03/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.03/

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 14 July 2016 16:58
To: Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov; 
Alexander Scherbatiy
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

Hi Rajeev,

I have added 1px border to the icon in your test:

private static BufferedImage generateImage(int scale, Color c) {
int x = SZ * scale;
BufferedImage img = new BufferedImage(x, x, BufferedImage.TYPE_INT_RGB);
Graphics g = img.getGraphics();
if (g != null) {
g.setColor(c);
g.fillRect(0, 0, x, x);
g.setColor(Color.YELLOW);
g.drawRect(0, 0, x-1, x-1);
}
return img;
}

It seems the icon in the taskbar is not correct for UI scale > 1.

By the way, graphics object should be disposed using g.dispose() when it is not 
needed anymore.

--Semyon




On 14.07.2016 10:08, Rajeev Chamyal wrote:

Hello All,

 

Gentle reminder. Please review the updated webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.02/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.02/

 

Update: simplified the test.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 22 June 2016 15:46
To: Rajeev Chamyal; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

The fix looks good to me.

Thanks,
Alexandr.

On 6/22/2016 10:49 AM, Rajeev Chamyal wrote:

Hello Alexandr,

 

Thanks for the review. I have updated webrev as per comments.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.01/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 21 June 2016 17:37
To: Rajeev Chamyal; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wron

Re: Swing Dev>[9] Review Request JDK-8158918 setExtendedState(1) for maximized Frame results in state==7

[Rajeev Chamyal]

Hello Semyon,

 

Following is the new bug.

https://bugs.openjdk.java.net/browse/JDK-8161995 

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 21 July 2016 16:14
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re: Swing Dev>[9] Review Request JDK-8158918 setExtendedState(1) for 
maximized Frame results in state==7

 

On 7/21/2016 1:30 PM, Rajeev Chamyal wrote:

Hello Semyon,

 

I will be creating a new bug for the old issue.

Could you, please, add the JBS link here.

--Semyon



 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 21 July 2016 15:59
To: Rajeev Chamyal; Alexander Scherbatiy; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: Swing Dev>[9] Review Request JDK-8158918 setExtendedState(1) for 
maximized Frame results in state==7

 

Hi Rajeev,

As I understand you have reverted 8037575 fix.

How the 8037575 will be addressed now?

--Semyon

On 7/18/2016 9:03 AM, Rajeev Chamyal wrote:

Hello Alexandr,

 

Please review the updated webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8158918/webrev.01/"http://cr.openjdk.java.net/~rchamyal/8158918/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 14 July 2016 20:54
To: Rajeev Chamyal; Semyon Sadetsky; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: Swing Dev>[9] Review Request JDK-8158918 setExtendedState(1) for 
maximized Frame results in state==7

 

On 7/14/2016 1:18 PM, Rajeev Chamyal wrote:





Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8158918 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8158918/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8158918/webrev.00/

 

Issue: Frame setExtendedState = 1 on a maximized frame is not working.

Cause: Issue is due to ::ShowWindow API call added as part of fix for HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8037575"JDK-8037575

Fix: Removed the ShowWindow call a sepate bug will be created for HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8037575"JDK-8037575

   40 if (frame.getExtendedState() != 1) {
   It is better to use the named frame state constant instead of just 1.
   
  Thanks,
  Alexandr.





 

Regards,

Rajeev Chamyal

 

 

 

Re: [9] Fix for JDK-7096375 : Swing ignores first click after decreasing system's time

[Rajeev Chamyal]

Looks good to me.

Regards,
Rajeev Chamyal

-Original Message-
From: Ajit Ghaisas 
Sent: 21 July 2016 15:13
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: RE:  [9] Fix for JDK-7096375 : Swing ignores first click 
after decreasing system's time

Fixed a typo in comment (from lable to label)

http://cr.openjdk.java.net/~aghaisas/7096375/webrev.04/

Regards,
Ajit

-Original Message-
From: Ajit Ghaisas
Sent: Thursday, July 21, 2016 11:41 AM
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] Fix for JDK-7096375 : Swing ignores first click 
after decreasing system's time

Good catch.
I have corrected the test case. Please review.
http://cr.openjdk.java.net/~aghaisas/7096375/webrev.03/

Regards,
Ajit

-Original Message-----
From: Rajeev Chamyal
Sent: Thursday, July 21, 2016 11:16 AM
To: Ajit Ghaisas; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: RE: [9] Fix for JDK-7096375 : Swing ignores first click after 
decreasing system's time

Hello Ajit,

Frame dispose is called twice in case pass/fail button are pressed.

Regards,
Rajeev Chamyal

-Original Message-
From: Ajit Ghaisas
Sent: 20 July 2016 17:50
To: Alexander Scherbatiy; swing-dev@openjdk.java.net; Rajeev Chamyal
Subject: RE: [9] Fix for JDK-7096375 : Swing ignores first click after 
decreasing system's time

Hi,

Added throwing exception in case create UI or dispose UI fails.
Here is the updated webrev:
http://cr.openjdk.java.net/~aghaisas/7096375/webrev.02/

Regards,
Ajit


-Original Message-
From: Alexandr Scherbatiy
Sent: Wednesday, July 20, 2016 5:13 PM
To: Ajit Ghaisas; swing-dev@openjdk.java.net; Rajeev Chamyal
Subject: Re: [9] Fix for JDK-7096375 : Swing ignores first click after 
decreasing system's time

On 7/20/2016 1:50 PM, Ajit Ghaisas wrote:
> Hi,
>
> I have modified the test as per suggestion.
>
> Please review the updated webrev :
> http://cr.openjdk.java.net/~aghaisas/7096375/webrev.01/
It is better to rethrow the exceptions during UI creation and disposing 
because it also means some unexpected behavior.

   Thanks,
   Alexandr.
>
> Regards,
> Ajit
>
> -Original Message-
> From: Alexandr Scherbatiy
> Sent: Tuesday, July 19, 2016 2:33 PM
> To: Ajit Ghaisas; swing-dev@openjdk.java.net; Rajeev Chamyal
> Subject: Re: [9] Fix for JDK-7096375 : Swing ignores first click after 
> decreasing system's time
>
> On 7/18/2016 3:27 PM, Ajit Ghaisas wrote:
>> Hi,
>>
>> Bug :
>>   https://bugs.openjdk.java.net/browse/JDK-7096375
>>   Swing ignores first click after decreasing system's time.
>>
>> Fix :
>>   BasicButtonListener keeps track of the last time when a button is 
>> pressed.
>>   This is used while discarding mouse press events to handle 
>> multiClickThreshold.
>>   The condition to discard mouse press event is corrected.
>>
>> Webrev :
>>   http://cr.openjdk.java.net/~aghaisas/7096375/webrev.00/
>>
>>   Request you to review.
> The template used for the test is rather old. It is better to use 
> CountDownLatch for the manual test synchronization.
> Could you rewrite the test using the TitledBorderTest as a sample:
> http://hg.openjdk.java.net/jdk9/client/jdk/file/233b59b7ea2f/test/java
> x/swing/LookAndFeel/6439354/TitledBorderTest.java
>
> There can be added two simple changes. The thread creation is not 
> necessary because the runnable can be  directly executed by 
> SwingUtilities.invokeAndWait(). The timeout can be added to the
> latch.await() call.
>
> Thanks,
> Alexandr.
>> Regards,
>> Ajit

Re: Swing Dev>[9] Review Request JDK-8158918 setExtendedState(1) for maximized Frame results in state==7

[Rajeev Chamyal]

Hello Semyon,

 

I will be creating a new bug for the old issue.

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 21 July 2016 15:59
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re: Swing Dev>[9] Review Request JDK-8158918 setExtendedState(1) for 
maximized Frame results in state==7

 

Hi Rajeev,

As I understand you have reverted 8037575 fix.

How the 8037575 will be addressed now?

--Semyon

On 7/18/2016 9:03 AM, Rajeev Chamyal wrote:

Hello Alexandr,

 

Please review the updated webrev.

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8158918/webrev.01/"http://cr.openjdk.java.net/~rchamyal/8158918/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 14 July 2016 20:54
To: Rajeev Chamyal; Semyon Sadetsky; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: Swing Dev>[9] Review Request JDK-8158918 setExtendedState(1) for 
maximized Frame results in state==7

 

On 7/14/2016 1:18 PM, Rajeev Chamyal wrote:




Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8158918 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8158918/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8158918/webrev.00/

 

Issue: Frame setExtendedState = 1 on a maximized frame is not working.

Cause: Issue is due to ::ShowWindow API call added as part of fix for HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8037575"JDK-8037575

Fix: Removed the ShowWindow call a sepate bug will be created for HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8037575"JDK-8037575

   40 if (frame.getExtendedState() != 1) {
   It is better to use the named frame state constant instead of just 1.
   
  Thanks,
  Alexandr.




 

Regards,

Rajeev Chamyal

 

 

Re: [9] Fix for JDK-7096375 : Swing ignores first click after decreasing system's time

[Rajeev Chamyal]

Hello Ajit,

Frame dispose is called twice in case pass/fail button are pressed.

Regards,
Rajeev Chamyal

-Original Message-
From: Ajit Ghaisas 
Sent: 20 July 2016 17:50
To: Alexander Scherbatiy; swing-dev@openjdk.java.net; Rajeev Chamyal
Subject: RE: [9] Fix for JDK-7096375 : Swing ignores first click after 
decreasing system's time

Hi,

Added throwing exception in case create UI or dispose UI fails.
Here is the updated webrev:
http://cr.openjdk.java.net/~aghaisas/7096375/webrev.02/

Regards,
Ajit


-Original Message-
From: Alexandr Scherbatiy
Sent: Wednesday, July 20, 2016 5:13 PM
To: Ajit Ghaisas; swing-dev@openjdk.java.net; Rajeev Chamyal
Subject: Re: [9] Fix for JDK-7096375 : Swing ignores first click after 
decreasing system's time

On 7/20/2016 1:50 PM, Ajit Ghaisas wrote:
> Hi,
>
> I have modified the test as per suggestion.
>
> Please review the updated webrev :
> http://cr.openjdk.java.net/~aghaisas/7096375/webrev.01/
It is better to rethrow the exceptions during UI creation and disposing 
because it also means some unexpected behavior.

   Thanks,
   Alexandr.
>
> Regards,
> Ajit
>
> -Original Message-
> From: Alexandr Scherbatiy
> Sent: Tuesday, July 19, 2016 2:33 PM
> To: Ajit Ghaisas; swing-dev@openjdk.java.net; Rajeev Chamyal
> Subject: Re: [9] Fix for JDK-7096375 : Swing ignores first click after 
> decreasing system's time
>
> On 7/18/2016 3:27 PM, Ajit Ghaisas wrote:
>> Hi,
>>
>> Bug :
>>   https://bugs.openjdk.java.net/browse/JDK-7096375
>>   Swing ignores first click after decreasing system's time.
>>
>> Fix :
>>   BasicButtonListener keeps track of the last time when a button is 
>> pressed.
>>   This is used while discarding mouse press events to handle 
>> multiClickThreshold.
>>   The condition to discard mouse press event is corrected.
>>
>> Webrev :
>>   http://cr.openjdk.java.net/~aghaisas/7096375/webrev.00/
>>
>>   Request you to review.
> The template used for the test is rather old. It is better to use 
> CountDownLatch for the manual test synchronization.
> Could you rewrite the test using the TitledBorderTest as a sample:
> http://hg.openjdk.java.net/jdk9/client/jdk/file/233b59b7ea2f/test/java
> x/swing/LookAndFeel/6439354/TitledBorderTest.java
>
> There can be added two simple changes. The thread creation is not 
> necessary because the runnable can be  directly executed by 
> SwingUtilities.invokeAndWait(). The timeout can be added to the
> latch.await() call.
>
> Thanks,
> Alexandr.
>> Regards,
>> Ajit

Re: 8161470: [TEST_BUG] Failure javax/swing/JRadioButton/FocusTraversal/FocusTraversal.java

[Rajeev Chamyal]

Looks fine to me.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 20 July 2016 11:52
To: Rajeev Chamyal
Cc: Praveen Srivastava; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  8161470: [TEST_BUG] Failure 
javax/swing/JRadioButton/FocusTraversal/FocusTraversal.java

 

Hi Rajeev,

Updated on same webrev due to single line change.

 

On 20-Jul-2016, at 11:41 am, Rajeev Chamyal mailto:rajeev.cham...@oracle.com"rajeev.cham...@oracle.com> wrote:

 

Hello Avik,

 

Line 67 can be removed from test.

 

Regards,

Rajeev Chamyal

From: Avik Niyogi 
Sent: 20 July 2016 11:30
To: Rajeev Chamyal
Cc: Praveen Srivastava; Alexandr Scherbatiy; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re:  8161470: [TEST_BUG] Failure 
javax/swing/JRadioButton/FocusTraversal/FocusTraversal.java

 

A gentle reminder, please review my code changes.

 

With Regards,

Avik Niyogi

On 19-Jul-2016, at 1:31 pm, Alexandr Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 


The fix looks good to me.

Thanks,
Alexandr.

On 7/19/2016 9:45 AM, Avik Niyogi wrote:

Hi All, 

 

Kindly review the fix for JDK9. 

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8161470

 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8161470/webrev.00/"http://cr.openjdk.java.net/~aniyogi/8161470/webrev.00/

 

Issue: This test consistently (12/12) fails on OS X 10.10 and also fails on 
Ubuntu 14.04.

 

Cause:  Due to quirk in robot.delay the UI was not getting appropriate 
behaviour response.

 

Fix: Appropriate Robot methods were used to fix this.

 

With Regards,

Avik Niyogi

 

Re: 8161470: [TEST_BUG] Failure javax/swing/JRadioButton/FocusTraversal/FocusTraversal.java

[Rajeev Chamyal]

Hello Avik,

 

Line 67 can be removed from test.

 

Regards,

Rajeev Chamyal

From: Avik Niyogi 
Sent: 20 July 2016 11:30
To: Rajeev Chamyal
Cc: Praveen Srivastava; Alexandr Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  8161470: [TEST_BUG] Failure 
javax/swing/JRadioButton/FocusTraversal/FocusTraversal.java

 

A gentle reminder, please review my code changes.

 

With Regards,

Avik Niyogi

On 19-Jul-2016, at 1:31 pm, Alexandr Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 


The fix looks good to me.

Thanks,
Alexandr.

On 7/19/2016 9:45 AM, Avik Niyogi wrote:

Hi All, 

 

Kindly review the fix for JDK9. 

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8161470

 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8161470/webrev.00/"http://cr.openjdk.java.net/~aniyogi/8161470/webrev.00/

 

Issue: This test consistently (12/12) fails on OS X 10.10 and also fails on 
Ubuntu 14.04.

 

Cause:  Due to quirk in robot.delay the UI was not getting appropriate 
behaviour response.

 

Fix: Appropriate Robot methods were used to fix this.

 

With Regards,

Avik Niyogi

 

 

Re: [9] Review Request JDK-8147648 [hidpi] multiresolution image: wrong resolution variant is used as icon in the Unity panel

[Rajeev Chamyal]

Hello Semyon,

 

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8147648/webrev.03/

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 14 July 2016 16:58
To: Rajeev Chamyal; swing-dev@openjdk.java.net; Sergey Bylokhov; Alexander 
Scherbatiy
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

Hi Rajeev,

I have added 1px border to the icon in your test:

private static BufferedImage generateImage(int scale, Color c) {
int x = SZ * scale;
BufferedImage img = new BufferedImage(x, x, BufferedImage.TYPE_INT_RGB);
Graphics g = img.getGraphics();
if (g != null) {
g.setColor(c);
g.fillRect(0, 0, x, x);
g.setColor(Color.YELLOW);
g.drawRect(0, 0, x-1, x-1);
}
return img;
}

It seems the icon in the taskbar is not correct for UI scale > 1.

By the way, graphics object should be disposed using g.dispose() when it is not 
needed anymore.

--Semyon



On 14.07.2016 10:08, Rajeev Chamyal wrote:

Hello All,

 

Gentle reminder. Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8147648/webrev.02/

 

Update: simplified the test.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 22 June 2016 15:46
To: Rajeev Chamyal; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

The fix looks good to me.

Thanks,
Alexandr.

On 6/22/2016 10:49 AM, Rajeev Chamyal wrote:

Hello Alexandr,

 

Thanks for the review. I have updated webrev as per comments.

http://cr.openjdk.java.net/~rchamyal/8147648/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 21 June 2016 17:37
To: Rajeev Chamyal; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 6/21/2016 12:16 PM, Rajeev Chamyal wrote:

Hello All,

 

Please review the following webrev.

 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.00/
 

Bug: https://bugs.openjdk.java.net/browse/JDK-8147648 

 

Issue: Wrong resolution variant is used as icon in the Unity panel.

Cause: The screen transforms are not applied to find the correct resolution 
variant image in current implementation.

Fix: Applied the screen transforms to graphics object.


 222 int scaleX = (int)tx.getScaleX();
 223 int scaleY = (int)tx.getScaleY();
 224 DataBufferInt buffer = new DataBufferInt(scaleX * width * scaleY * 
height);

  The fix is in the shared code and the scale factor can have floating point 
value on Windows. (for example 1.5).
  It is better to round the final width and height after scaling them.
 
  Thanks,
  Alexandr.

 

Regards,

Rajeev Chamyal

 

 

 

Re: Review Request JDK-8159168 [hidpi] Window.setShape() works incorrectly on HiDPI

[Rajeev Chamyal]

Hello Sergey,

It works fine on linux.

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 20 July 2016 01:13
To: Rajeev Chamyal; Semyon Sadetsky; Alexander Scherbatiy; 
swing-dev@openjdk.java.net
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

Hi, Rajeev.
Can you please clarify how it works(if it works) on Linux/Solaris?

On 11.07.16 12:08, Rajeev Chamyal wrote:
> Hello Semyon,
>
>
>
> Please review the updated webrev as per review comments.
>
> http://cr.openjdk.java.net/~rchamyal/8159168/webrev.04/
>
>
>
> Regards,
>
> Rajeev Chamyal
>
>
>
> *From:*Semyon Sadetsky
> *Sent:* 11 July 2016 14:29
> *To:* Rajeev Chamyal; Alexander Scherbatiy; 
> swing-dev@openjdk.java.net; Sergey Bylokhov
> *Subject:* Re:  Review Request JDK-8159168 [hidpi]
> Window.setShape() works incorrectly on HiDPI
>
>
>
> On 7/11/2016 11:10 AM, Rajeev Chamyal wrote:
>
> Hello Semyon,
>
>
>
> Thanks for the review. Yes, mouse move is not required I have
> removed it.
>
> Please review the updated webrev.
>
> http://cr.openjdk.java.net/~rchamyal/8159168/webrev.03/
>
> Thanks.
> I also cannot see the reason to wait 1 second in line 59.
> It seems tx variable from line 53 is never used.
>
> --Semyon
>
>
>
> Regards,
>
> Rajeev Chamyal
>
>
>
> *From:*Semyon Sadetsky
> *Sent:* 11 July 2016 12:30
> *To:* Rajeev Chamyal; Alexander Scherbatiy;
> swing-dev@openjdk.java.net <mailto:swing-dev@openjdk.java.net>;
> Sergey Bylokhov
> *Subject:* Re:  Review Request JDK-8159168 [hidpi]
> Window.setShape() works incorrectly on HiDPI
>
>
>
> Hi Rajeev,
>
> The fix itself looks good. I only did not get for what purpose mouse
> pointer is moved in the test?
>
> --Semyon
>
>
>
> On 7/5/2016 9:16 AM, Rajeev Chamyal wrote:
>
> Hello Alexandr,
>
>
>
> Please review updated webrev.
>
> http://cr.openjdk.java.net/~rchamyal/8159168/webrev.02/
>
>
>
> Regards,
>
> Rajeev Chamyal
>
>
>
> *From:*Alexandr Scherbatiy
> *Sent:* 05 July 2016 11:38
>     *To:* Rajeev Chamyal; swing-dev@openjdk.java.net
> <mailto:swing-dev@openjdk.java.net>; Sergey Bylokhov
> *Subject:* Re:  Review Request JDK-8159168 [hidpi]
> Window.setShape() works incorrectly on HiDPI
>
>
>
> On 7/5/2016 8:25 AM, Rajeev Chamyal wrote:
>
>
>
> Hello Alexandr,
>
>
>
> Thanks for the review.
>
> As per windows specification X & Y scale are always equal
> that's why I have put scaleX == scaleY check.
>
> But it may change in future so I have removed this check.
>
>
>
>     http://cr.openjdk.java.net/~rchamyal/8159168/webrev.01/
>
>
> 1135 if (scaleX != 1 && scaleY != 1) {
>
>   It is better to use 'or' operator because the shape should be
> scaled when on of the scales is differ from 1.
>
> Thanks,
> Alexandr.
>
>
>
>
>
>
>     Regards,
>
> Rajeev Chamyal
>
>
>
> *From:*Alexandr Scherbatiy
> *Sent:* 04 July 2016 18:03
> *To:* Rajeev Chamyal; swing-dev@openjdk.java.net
> <mailto:swing-dev@openjdk.java.net>; Sergey Bylokhov
> *Subject:* Re:  Review Request JDK-8159168
> [hidpi] Window.setShape() works incorrectly on HiDPI
>
>
>
> On 7/4/2016 3:09 PM, Rajeev Chamyal wrote:
>
>
>
>
> Hello All,
>
>
>
> Please review the following webrev.
>
>
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8159168
>
> Webrev:
> http://cr.openjdk.java.net/~rchamyal/8159168/webrev.00/
> 
> <http://cr.openjdk.java.net/%7Erchamyal/8159168/webrev.00/>
>
>
>
> Issue: In HiDPI screen shape set through
> window::setShape API is not scaled based on system scale.
>
> Fix:. Updated the WComponentPeer::applyShape to update
> shape based on system scale.
>
> 1131 double scaleX =
> winGraphicsConfig.getDefaultTransform().getScaleX();
> 1132 double scaleY =
> winGraphicsConfig.getDefaultTransform().getScaleY();
>
>  The getDefaultTransform() is call

Re: 8160438: [PIT][macosx] [TEST_BUG] javax/swing/plaf/nimbus/8057791/bug8057791.java fails

[Rajeev Chamyal]

Looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 19 July 2016 12:12
To: Alexandr Scherbatiy
Cc: Rajeev Chamyal; Semyon Sadetsky; Yuri Nesterenko; swing-dev@openjdk.java.net
Subject: Re:  8160438: [PIT][macosx] [TEST_BUG] 
javax/swing/plaf/nimbus/8057791/bug8057791.java fails

 

Hi All,

Another gentle reminder. Please review the changes made.

On 15-Jul-2016, at 2:50 pm, Avik Niyogi mailto:avik.niy...@oracle.com"avik.niy...@oracle.com> wrote:

 

A gentle reminder, please review the changes

 

On 14-Jul-2016, at 8:46 pm, Alexandr Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 


The fix looks good to me.

Thanks,
Alexandr.

On 7/14/2016 9:53 AM, Avik Niyogi wrote:

Hi All, 

Please find my webrev below for review which includes changes as perceived from 
inputs provided.

 

HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8160438/webrev.02/"http://cr.openjdk.java.net/~aniyogi/8160438/webrev.02/

 

With Regards,

Avik Niyogi

On 12-Jul-2016, at 7:12 pm, Alexandr Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

On 7/12/2016 8:24 AM, Avik Niyogi wrote:



Hi All,
A gentle reminder, please review my code changes.

With Regards,
Avik Niyogi




On 08-Jul-2016, at 4:24 pm, Yuri Nesterenko mailto:yuri.nestere...@oracle.com"yuri.nestere...@oracle.com> wrote:

It pass for me if I provide some time to process native events
after the user activity simulation. For instance,
setAutoDelay(50) for robot does that plus waitForIdle()
after every mouseMove(). In this case, the part with that additional
check and a (misleading, I think) comment about the color profiles
may be removed. The test would take much more time though, and
  @run main/timeout=600 bug8057791
line would be necessary.

   Some more comments to the previous ones:
 - runColorTestCase() uses JList and should be run on EDT
 - "if (tryNimbusLookAndFeel()) {" It is supposed that the Nimbus L&F is 
supported on all platforms. May be it is better to fail the test if the Nimbus 
L&F is not set.
 - "if (osName.contains("Mac")) { isMac = true; }" can be simplified to "return 
osName.contains("Mac")"
 - " if (!"".equals(errorString)) {" can be simplified to !errorString.isEmpty()

 Thanks,
 Alexandr.




Thanks,
-yan

On 07/08/2016 04:28 AM, Avik Niyogi wrote:



The test does not pass if mac specific check is not done for
backgroundcolor.
The check is required to get the same values from BufferedImage as they
are the same as found with Digital Color Meter.
This test case fixes that.
Please provide inputs if this fix will get a +1 or if not any positive
inputs to modify the test case will be welcome.

With Regards,
Avik Niyogi



On 07-Jul-2016, at 9:51 pm, Semyon Sadetsky
mailto:semyon.sadet...@oracle.com"semyon.sadet...@oracle.com 
<mailto:semyon.sadet...@oracle.com>> wrote:



On 7/7/2016 6:30 PM, Yuri Nesterenko wrote:



On 07/07/2016 06:15 PM, Semyon Sadetsky wrote:




On 7/7/2016 5:58 PM, Yuri Nesterenko wrote:



On 07/07/2016 05:35 PM, Yuri Nesterenko wrote:



On 07/07/2016 05:04 PM, Semyon Sadetsky wrote:




On 07.07.2016 16:35, Avik Niyogi wrote:



Hi Semyon,

Thank you for the review comment.

In Mac OS X, *System Preferences > Displays > Colors > Display
Profile*  section, the default value is *Color LCD*.
This causes a failure in some test cases which uses robot.The colour
configuration it expects to use is the *Generic RGB Profile*.
That is what "Non-generic display settings" means.

AFAIK there are instruction that tests should be executed using color
profile with no color corrections on OS X. I guess there are many
other
tests that fail with color correction.
If this is a root cause than the bug doesn't need to be fixed.

Well, I filed this bug and I used Generic RGB on all my
test machines. There may be additional precautions in the tests
about that but misconfiguration is not the root case here.
That said, I feel vary about attempts to guess Apple colors

  wary I mean



in non-generic profiles.

Yuri. Do you mean that the non-generic is not a root case?

No: I had Generic RGB everywhere, and it failed for me.
I should say that the new version of the test properly
fails with b120 and pass with current PIT. And colors
are visibly different in the two builds: so the test works OK now.

That contradicts with the description of the fix.
Probably the test does unnecessary care about the non-Generic profiles.

159 if (!foundMatch && isMac()) {
160 //One more chance for Mac due to non-Generic
display setting
161 detectedColor = new Color(img.getRGB(x, y), true);
162 if (detectedColor.equals(colorCheck)) {
163 foundMatch = true;
164

Re: Swing Dev>[9] Review Request JDK-8158918 setExtendedState(1) for maximized Frame results in state==7

[Rajeev Chamyal]

Hello Alexandr,

 

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8158918/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 14 July 2016 20:54
To: Rajeev Chamyal; Semyon Sadetsky; swing-dev@openjdk.java.net
Subject: Re: Swing Dev>[9] Review Request JDK-8158918 setExtendedState(1) for 
maximized Frame results in state==7

 

On 7/14/2016 1:18 PM, Rajeev Chamyal wrote:



Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8158918 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8158918/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8158918/webrev.00/

 

Issue: Frame setExtendedState = 1 on a maximized frame is not working.

Cause: Issue is due to ::ShowWindow API call added as part of fix for HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8037575"JDK-8037575

Fix: Removed the ShowWindow call a sepate bug will be created for HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8037575"JDK-8037575

   40 if (frame.getExtendedState() != 1) {
   It is better to use the named frame state constant instead of just 1.
   
  Thanks,
  Alexandr.



 

Regards,

Rajeev Chamyal

 

Swing Dev>[9] Review Request JDK-8158918 setExtendedState(1) for maximized Frame results in state==7

[Rajeev Chamyal]

Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8158918 

Webrev: http://cr.openjdk.java.net/~rchamyal/8158918/webrev.00/

 

Issue: Frame setExtendedState = 1 on a maximized frame is not working.

Cause: Issue is due to ::ShowWindow API call added as part of fix for HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8037575"JDK-8037575

Fix: Removed the ShowWindow call a sepate bug will be created for HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8037575"JDK-8037575

 

Regards,

Rajeev Chamyal

Re: [9] Review Request JDK-8147648 [hidpi] multiresolution image: wrong resolution variant is used as icon in the Unity panel

[Rajeev Chamyal]

Hello All,

 

Gentle reminder. Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8147648/webrev.02/

 

Update: simplified the test.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 22 June 2016 15:46
To: Rajeev Chamyal; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

The fix looks good to me.

Thanks,
Alexandr.

On 6/22/2016 10:49 AM, Rajeev Chamyal wrote:

Hello Alexandr,

 

Thanks for the review. I have updated webrev as per comments.

http://cr.openjdk.java.net/~rchamyal/8147648/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 21 June 2016 17:37
To: Rajeev Chamyal; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 6/21/2016 12:16 PM, Rajeev Chamyal wrote:

Hello All,

 

Please review the following webrev.

 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.00/
 

Bug: https://bugs.openjdk.java.net/browse/JDK-8147648 

 

Issue: Wrong resolution variant is used as icon in the Unity panel.

Cause: The screen transforms are not applied to find the correct resolution 
variant image in current implementation.

Fix: Applied the screen transforms to graphics object.


 222 int scaleX = (int)tx.getScaleX();
 223 int scaleY = (int)tx.getScaleY();
 224 DataBufferInt buffer = new DataBufferInt(scaleX * width * scaleY * 
height);

  The fix is in the shared code and the scale factor can have floating point 
value on Windows. (for example 1.5).
  It is better to round the final width and height after scaling them.
 
  Thanks,
  Alexandr.

 

Regards,

Rajeev Chamyal

 

 

Re: Review Request JDK-8159168 [hidpi] Window.setShape() works incorrectly on HiDPI

[Rajeev Chamyal]

Hello Semyon,

 

Please review the updated webrev as per review comments.

http://cr.openjdk.java.net/~rchamyal/8159168/webrev.04/ 

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 11 July 2016 14:29
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net; Sergey 
Bylokhov
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

 

On 7/11/2016 11:10 AM, Rajeev Chamyal wrote:

Hello Semyon,

 

Thanks for the review. Yes, mouse move is not required I have removed it.

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8159168/webrev.03/

Thanks. 
I also cannot see the reason to wait 1 second in line 59. 
It seems tx variable from line 53 is never used.

--Semyon



 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 11 July 2016 12:30
To: Rajeev Chamyal; Alexander Scherbatiy; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

 

Hi Rajeev,

The fix itself looks good. I only did not get for what purpose mouse pointer is 
moved in the test?

--Semyon

 

On 7/5/2016 9:16 AM, Rajeev Chamyal wrote:

Hello Alexandr,

 

Please review updated webrev.

http://cr.openjdk.java.net/~rchamyal/8159168/webrev.02/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 05 July 2016 11:38
To: Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

 

On 7/5/2016 8:25 AM, Rajeev Chamyal wrote:





Hello Alexandr,

 

Thanks for the review.

As per windows specification X & Y scale are always equal that's why I have put 
scaleX == scaleY check.

But it may change in future so I have removed this check.

 

http://cr.openjdk.java.net/~rchamyal/8159168/webrev.01/


1135 if (scaleX != 1 && scaleY != 1) {
  
  It is better to use 'or' operator because the shape should be scaled when on 
of the scales is differ from 1.

Thanks,
Alexandr.






 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 04 July 2016 18:03
To: Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

 

On 7/4/2016 3:09 PM, Rajeev Chamyal wrote:






Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8159168 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8159168/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8159168/webrev.00/
 

 

Issue: In HiDPI screen shape set through window::setShape API is not scaled 
based on system scale.

Fix:. Updated the WComponentPeer::applyShape to update shape based on system 
scale. 

1131 double scaleX = 
winGraphicsConfig.getDefaultTransform().getScaleX();
1132 double scaleY = 
winGraphicsConfig.getDefaultTransform().getScaleY();

 The getDefaultTransform() is called twice which leads that AffineTransform 
object is created two times
1133 if (scaleX != 1 && scaleY != 1 && scaleX == scaleY) {

   Is the check scaleX == scaleY really necessary here?

   Is it possible to make the test automated? Just run it with option "@run 
main/othervm -Dsun.java2d.uiScale=2 TestName" and check the area where the 
shape is drawn?

  Thanks,
  Alexandr.






 

Regards,

Rajeev Chamyal

 

 

 

 

 

Re: Review Request JDK-8159168 [hidpi] Window.setShape() works incorrectly on HiDPI

[Rajeev Chamyal]

Hello Semyon,

 

Thanks for the review. Yes, mouse move is not required I have removed it.

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8159168/webrev.03/

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 11 July 2016 12:30
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net; Sergey 
Bylokhov
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

 

Hi Rajeev,

The fix itself looks good. I only did not get for what purpose mouse pointer is 
moved in the test?

--Semyon

 

On 7/5/2016 9:16 AM, Rajeev Chamyal wrote:

Hello Alexandr,

 

Please review updated webrev.

http://cr.openjdk.java.net/~rchamyal/8159168/webrev.02/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 05 July 2016 11:38
To: Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

 

On 7/5/2016 8:25 AM, Rajeev Chamyal wrote:




Hello Alexandr,

 

Thanks for the review.

As per windows specification X & Y scale are always equal that's why I have put 
scaleX == scaleY check.

But it may change in future so I have removed this check.

 

http://cr.openjdk.java.net/~rchamyal/8159168/webrev.01/


1135 if (scaleX != 1 && scaleY != 1) {
  
  It is better to use 'or' operator because the shape should be scaled when on 
of the scales is differ from 1.

Thanks,
Alexandr.





 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 04 July 2016 18:03
To: Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

 

On 7/4/2016 3:09 PM, Rajeev Chamyal wrote:





Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8159168 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8159168/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8159168/webrev.00/
 

 

Issue: In HiDPI screen shape set through window::setShape API is not scaled 
based on system scale.

Fix:. Updated the WComponentPeer::applyShape to update shape based on system 
scale. 

1131 double scaleX = 
winGraphicsConfig.getDefaultTransform().getScaleX();
1132 double scaleY = 
winGraphicsConfig.getDefaultTransform().getScaleY();

 The getDefaultTransform() is called twice which leads that AffineTransform 
object is created two times
1133 if (scaleX != 1 && scaleY != 1 && scaleX == scaleY) {

   Is the check scaleX == scaleY really necessary here?

   Is it possible to make the test automated? Just run it with option "@run 
main/othervm -Dsun.java2d.uiScale=2 TestName" and check the area where the 
shape is drawn?

  Thanks,
  Alexandr.





 

Regards,

Rajeev Chamyal

 

 

 

 

Re: Swing Dev> Review Request JDK-8158205 HiDPI hand cursor broken on Windows

[Rajeev Chamyal]

Hello Alexandr,

 

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8158205/webrev.01/ 

Test was always passing without fix also. I have converted it into manual test.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 05 July 2016 13:29
To: Rajeev Chamyal; swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re: Swing Dev> Review Request JDK-8158205 HiDPI hand cursor broken on 
Windows

 

On 7/5/2016 10:08 AM, Rajeev Chamyal wrote:



Hello All,

 

Please review the following fix.

Bug: https://bugs.openjdk.java.net/browse/JDK-8158205 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8158205/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8158205/webrev.00/

 

Issue: Incorrect cursor type is displayed over frame in Hidpi screen.

 

Cause:  Windows getCursorPos API is returning scaled screen co-ordinates for 
mouse 

as a result the findComponentAt is not able to find the frame below mouse 
pointer and its returning incorrect cursor type. 

 
Fix: Updated awt_Cursor.cpp:: getCursorPos to return scaled down co-ordinates.

 
  - The call to JFrame methods should be done on EDT in the test.
  - It looks like there is no need to use the invokeLater() inside the 
invokeAndWait() call
  - In case the test fails the dispose method is never called.

  Thanks,
  Alexandr.



 
Regards,
Rajeev Chamyal

 

 

 

 

Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create StackOverflowError

[Rajeev Chamyal]

Hello Ajit,

 

The fix looks fine to me.

Regarding test: JTable and JTree tests exceed 80 char limit.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 07 July 2016 15:17
To: Ajit Ghaisas; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

 

The fix looks good to me.

Thanks,
Alexandr.

On 7/7/2016 12:44 PM, Ajit Ghaisas wrote:

Hi,

  

Thanks Alex for pointing out there might be more components showing similar 
behavior.

 

Two more components are identified which may cause this recursion in 
UpdateUI() method - JTree and JTable.

 

Now, total 5 components ( JComboBox, JList, JTree, JTable and JTableHeader) 
are fixed.

 

Please review the updated webrev :

HYPERLINK 
"http://cr.openjdk.java.net/%7Eaghaisas/6567433/webrev.01/"http://cr.openjdk.java.net/~aghaisas/6567433/webrev.01/

 

Regards,

Ajit

 

From: Alexandr Scherbatiy 
Sent: Tuesday, July 05, 2016 5:55 PM
To: Ajit Ghaisas; Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: [9] Fix for JDK-6567433 : JComponent.updateUI() may create 
StackOverflowError

 

On 7/5/2016 2:51 PM, Ajit Ghaisas wrote:




Hi,

 

Bug :

 https://bugs.openjdk.java.net/browse/JDK-6567433

 

Calling updateUI() on JList, JComboBox and JTableHeader can create 
StackOverflowErrors.
For example -

JList.updateUI() invokes updateUI() on its Cellrenderer via 
SwingUtilities.updateComponentTreeUI().
If the cellrenderer is a parent of this JList the method recurses endless 
causing StackOverflowError.

 

 

Fix :

 Added a recursion guard to JComboBox, JList and JTableHeader classes.

 With this fix, UpdateUI() method in these classes does not result in 
recursion.

 

Webrev :

HYPERLINK 
"http://cr.openjdk.java.net/%7Eaghaisas/6567433/webrev.00/"http://cr.openjdk.java.net/~aghaisas/6567433/webrev.00/

 
   Could the same issue affect another Swing components which allow to set a 
cell renderer, like JTree?

  Thanks,
  Alexandr.




 

Request you to review.

 

Regards,

Ajit

 

 

 

Swing Dev> Review Request JDK-8158205 HiDPI hand cursor broken on Windows

[Rajeev Chamyal]

Hello All,

 

Please review the following fix.

Bug: https://bugs.openjdk.java.net/browse/JDK-8158205 

Webrev: http://cr.openjdk.java.net/~rchamyal/8158205/webrev.00/

 

Issue: Incorrect cursor type is displayed over frame in Hidpi screen.

 

Cause:  Windows getCursorPos API is returning scaled screen co-ordinates for 
mouse 

as a result the findComponentAt is not able to find the frame below mouse 
pointer and its returning incorrect cursor type. 

 
Fix: Updated awt_Cursor.cpp:: getCursorPos to return scaled down co-ordinates.
 
Regards,
Rajeev Chamyal

 

 

 

Re: Review Request JDK-8159168 [hidpi] Window.setShape() works incorrectly on HiDPI

[Rajeev Chamyal]

Hello Alexandr,

 

Please review updated webrev.

http://cr.openjdk.java.net/~rchamyal/8159168/webrev.02/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 05 July 2016 11:38
To: Rajeev Chamyal; swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

 

On 7/5/2016 8:25 AM, Rajeev Chamyal wrote:



Hello Alexandr,

 

Thanks for the review.

As per windows specification X & Y scale are always equal that's why I have put 
scaleX == scaleY check.

But it may change in future so I have removed this check.

 

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8159168/webrev.01/"http://cr.openjdk.java.net/~rchamyal/8159168/webrev.01/


1135 if (scaleX != 1 && scaleY != 1) {
  
  It is better to use 'or' operator because the shape should be scaled when on 
of the scales is differ from 1.

Thanks,
Alexandr.




 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 04 July 2016 18:03
To: Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

 

On 7/4/2016 3:09 PM, Rajeev Chamyal wrote:




Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8159168 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8159168/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8159168/webrev.00/
 

 

Issue: In HiDPI screen shape set through window::setShape API is not scaled 
based on system scale.

Fix:. Updated the WComponentPeer::applyShape to update shape based on system 
scale. 

1131 double scaleX = 
winGraphicsConfig.getDefaultTransform().getScaleX();
1132 double scaleY = 
winGraphicsConfig.getDefaultTransform().getScaleY();

 The getDefaultTransform() is called twice which leads that AffineTransform 
object is created two times
1133 if (scaleX != 1 && scaleY != 1 && scaleX == scaleY) {

   Is the check scaleX == scaleY really necessary here?

   Is it possible to make the test automated? Just run it with option "@run 
main/othervm -Dsun.java2d.uiScale=2 TestName" and check the area where the 
shape is drawn?

  Thanks,
  Alexandr.




 

Regards,

Rajeev Chamyal

 

 

 

Re: Review Request JDK-8159168 [hidpi] Window.setShape() works incorrectly on HiDPI

[Rajeev Chamyal]

Hello Alexandr,

 

Thanks for the review.

As per windows specification X & Y scale are always equal that's why I have put 
scaleX == scaleY check.

But it may change in future so I have removed this check.

 

http://cr.openjdk.java.net/~rchamyal/8159168/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 04 July 2016 18:03
To: Rajeev Chamyal; swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  Review Request JDK-8159168 [hidpi] Window.setShape() 
works incorrectly on HiDPI

 

On 7/4/2016 3:09 PM, Rajeev Chamyal wrote:



Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8159168 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8159168/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8159168/webrev.00/
 

 

Issue: In HiDPI screen shape set through window::setShape API is not scaled 
based on system scale.

Fix:. Updated the WComponentPeer::applyShape to update shape based on system 
scale. 

1131 double scaleX = 
winGraphicsConfig.getDefaultTransform().getScaleX();
1132 double scaleY = 
winGraphicsConfig.getDefaultTransform().getScaleY();

 The getDefaultTransform() is called twice which leads that AffineTransform 
object is created two times
1133 if (scaleX != 1 && scaleY != 1 && scaleX == scaleY) {

   Is the check scaleX == scaleY really necessary here?

   Is it possible to make the test automated? Just run it with option "@run 
main/othervm -Dsun.java2d.uiScale=2 TestName" and check the area where the 
shape is drawn?

  Thanks,
  Alexandr.



 

Regards,

Rajeev Chamyal

 

 

Review Request JDK-8159168 [hidpi] Window.setShape() works incorrectly on HiDPI

[Rajeev Chamyal]

Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8159168 

Webrev: http://cr.openjdk.java.net/~rchamyal/8159168/webrev.00/ 

 

Issue: In HiDPI screen shape set through window::setShape API is not scaled 
based on system scale.

Fix:. Updated the WComponentPeer::applyShape to update shape based on system 
scale. 

 

Regards,

Rajeev Chamyal

 

Re: [9] Review Request JDK-8147648 [hidpi] multiresolution image: wrong resolution variant is used as icon in the Unity panel

[Rajeev Chamyal]

Hello Sergey,

 

Could you please review this fix.

http://cr.openjdk.java.net/~rchamyal/8147648/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 22 June 2016 15:46
To: Rajeev Chamyal; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

The fix looks good to me.

Thanks,
Alexandr.

On 6/22/2016 10:49 AM, Rajeev Chamyal wrote:

Hello Alexandr,

 

Thanks for the review. I have updated webrev as per comments.

http://cr.openjdk.java.net/~rchamyal/8147648/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 21 June 2016 17:37
To: Rajeev Chamyal; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 6/21/2016 12:16 PM, Rajeev Chamyal wrote:




Hello All,

 

Please review the following webrev.

 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.00/
 

Bug: https://bugs.openjdk.java.net/browse/JDK-8147648 

 

Issue: Wrong resolution variant is used as icon in the Unity panel.

Cause: The screen transforms are not applied to find the correct resolution 
variant image in current implementation.

Fix: Applied the screen transforms to graphics object.


 222 int scaleX = (int)tx.getScaleX();
 223 int scaleY = (int)tx.getScaleY();
 224 DataBufferInt buffer = new DataBufferInt(scaleX * width * scaleY * 
height);

  The fix is in the shared code and the scale factor can have floating point 
value on Windows. (for example 1.5).
  It is better to round the final width and height after scaling them.
 
  Thanks,
  Alexandr.




 

Regards,

Rajeev Chamyal

 

 

Re: [9] RFR JDK-8159068:The rendering of JTable is broken

[Rajeev Chamyal]

Looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 24 June 2016 17:01
To: Prasanta Sadhukhan; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re:  [9] RFR JDK-8159068:The rendering of JTable is broken

 


The fix looks good to me.

Thanks,
Alexandr.

On 6/24/2016 12:37 PM, Prasanta Sadhukhan wrote:

 

On 6/24/2016 1:41 PM, Alexandr Scherbatiy wrote:

On 6/24/2016 10:04 AM, Prasanta Sadhukhan wrote:



Hi Alexandr,

Thanks for your suggestion. Have modified as per the comment to decide based on 
selected row. HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8081491"8081491 testcase and 
SwingSet2 JTable demo works.
Please review modified webrev:
HYPERLINK 
"http://cr.openjdk.java.net/%7Epsadhukhan/8159068/webrev.01/"http://cr.openjdk.java.net/~psadhukhan/8159068/webrev.01/

    - createUI() also should be called from EDT because it  uses Swing 
components
    - It is better to use CountDownLatch for manual tests synchronization (it 
has special await(...) method with timeout parameter).

Modified testcase to use CountDownLatch
HYPERLINK 
"http://cr.openjdk.java.net/%7Epsadhukhan/8159068/webrev.02/"http://cr.openjdk.java.net/~psadhukhan/8159068/webrev.02/

Regards
Prasanta



  Thanks,
  Alexandr.




Regards
Prasanta

On 6/23/2016 12:41 PM, Alexandr Scherbatiy wrote:

On 6/23/2016 9:53 AM, Prasanta Sadhukhan wrote:



 

On 6/23/2016 12:10 PM, Alexandr Scherbatiy wrote:

On 6/21/2016 2:57 PM, Prasanta Sadhukhan wrote:



 

On 6/21/2016 5:16 PM, Alexandr Scherbatiy wrote:

On 6/21/2016 1:58 PM, Prasanta Sadhukhan wrote:



 

On 6/21/2016 4:14 PM, Alexandr Scherbatiy wrote:

On 6/20/2016 8:10 AM, prasanta sadhukhan wrote:



Gentle reminder for review!!

Regards
Prasanta

On 6/13/2016 4:31 PM, prasanta sadhukhan wrote:

On 6/13/2016 12:51 PM, prasanta sadhukhan wrote:

Hi All, 

Please review a fix for jdk9 where it was seen that if we try to select some 
rows in a JTable, the text painted in the rows goes missing. 

Bug: https://bugs.openjdk.java.net/browse/JDK-8159068 

webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Epsadhukhan/8159068/webrev.00/"http://cr.openjdk.java.net/~psadhukhan/8159068/webrev.00/
 

The issue was rMax value was decremented wrongly so when paintCells() is called 
with wrong rMax, some rows were not printed correctly. 

Fix is to make sure rmax is decremented properly, only when we are trying to 
print whole visible portion of JTable and NOT when some rows are being painted. 


  Could you give two samples how this algorithm work. One sample where a whole 
visible portion of a JTable and another where some rows are being printed. What 
are rMax and  rMin values in both cases and how are they calculated?

If a JTable is of 50 rows and only 35 are being visible in page 1, then 
if whole visible portion of JTable is printed, rMin will be 0 and rMax was 35 
so 36 rows were getting printed so I decrement rMax by 1 to 34 so only 35 will 
be printed (same as shown on console).
When we select some row of JTable as in the case of LostText testcase, rMin 
will be say 6 and rMax will be 9 in which case also, I was decrementing rMax so 
rMin=6, rMax=8 so next row was not getting painted.

   And what are indices of the selected rows?

It will depend on the last selection. At start, rMin = 0 , rMax = last indice, 
say 10 for a JTable of 10 rows
Now, if we select row 5, rMin and rMax both becomes 5 and we decrement rMax so 
rMax becomes less than rMin and paintCell() due to this check 
(int row = rMin; row <= rMax; row++) it does not do 
paintCell(g, cellRect, row, column)
and nothing gets painted.

   It is not clear how the selection interval [0..N] is distinguishable from 
the case where is no selection because rMin should be 0 in both case. May be it 
is better to decrement the rMax depending on are there selected rows or not.

Can you suggest if there any way we can find out if there are any selected rows 
or not?

   There is the method table.getSelectedRowCount(). May be for performance 
reason it is better to check table.getSelectedRow().

  Thanks,
  Alexandr.





Regards
Prasanta




  Thanks,
  Alexandr.




Regards
Prasanta



  Thanks,
  Alexandr.
  




Regards
Prasanta



  Thanks,
  Alexandr.
  




Regarding the regression testcase, I could not make it automated as the failure 
happens on random iteration. 
and also, getting selection background/foreground was giving same values with 
and without the missing text. 

Also, since it is a regression of HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8081491"8081491, it's testcase are 
working fine with this fix and so did SwingSet2 JTable demo.



Regards 
Prasanat 

 

 

 

 

 

 

 

 

 

 

 

 

 

Re: Review Request JDK-8159152 Ctrl+F6, Ctrl+F5 doesn't work for iconified InternalFrame

[Rajeev Chamyal]

Hello Alexandr,

 

Thanks for the review. HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-4878528"JDK-4878528 keyboard focus 
test was failing with current fix.

I have updated the webrev to handle keyboard focus scenario. I have executed 
all the regression tests for JInternalFrame all are passing.

 

http://cr.openjdk.java.net/~rchamyal/8159152/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 21 June 2016 16:48
To: Rajeev Chamyal; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re:  Review Request JDK-8159152 Ctrl+F6, Ctrl+F5 doesn't 
work for iconified InternalFrame

 

On 6/20/2016 5:59 AM, Rajeev Chamyal wrote:



Hello Alexandr,

 

TestJInternalFrameMinimize test passes after this fix.

  Could you also check that the initial issue where the call 
setComponentOrderCheckingEnabled(true/false) has been added to the 
DefaultDesktopManager.iconifyFrame(JInternalFrame) method?
JDK-6325652 Iconified JInternalFrame does not restore when Ctrl+F5 is used
  https://bugs.openjdk.java.net/browse/JDK-6325652

  Thanks,
  Alexandr.




 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 17 June 2016 18:35
To: Rajeev Chamyal; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re:  Review Request JDK-8159152 Ctrl+F6, Ctrl+F5 doesn't 
work for iconified InternalFrame

 

On 6/17/2016 10:09 AM, Rajeev Chamyal wrote:




Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8159152 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8159152/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8159152/webrev.00/

 

Issue: Internal frame cache is not getting updated properly on iconifyFrame if 
there are less than 3 frames on the desktop.

Fix: Updated the iconifyFrame method so that frame cache updates properly on 
internal frame remove and icon addition to desktop.


  Could you check that the test 
test/javax/swing/JInternalFrame/8145060/TestJInternalFrameMinimize.java passes 
after the fix?

  Thanks,
  Alexandr.





 

Regards,

Rajeev Chamyal

 

 

 

 

Re: [9] Review Request JDK-8147648 [hidpi] multiresolution image: wrong resolution variant is used as icon in the Unity panel

[Rajeev Chamyal]

Hello Alexandr,

 

Thanks for the review. I have updated webrev as per comments.

http://cr.openjdk.java.net/~rchamyal/8147648/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 21 June 2016 17:37
To: Rajeev Chamyal; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re: [9] Review Request JDK-8147648 [hidpi] multiresolution 
image: wrong resolution variant is used as icon in the Unity panel

 

On 6/21/2016 12:16 PM, Rajeev Chamyal wrote:



Hello All,

 

Please review the following webrev.

 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147648/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8147648/webrev.00/
 

Bug: https://bugs.openjdk.java.net/browse/JDK-8147648 

 

Issue: Wrong resolution variant is used as icon in the Unity panel.

Cause: The screen transforms are not applied to find the correct resolution 
variant image in current implementation.

Fix: Applied the screen transforms to graphics object.


 222 int scaleX = (int)tx.getScaleX();
 223 int scaleY = (int)tx.getScaleY();
 224 DataBufferInt buffer = new DataBufferInt(scaleX * width * scaleY * 
height);

  The fix is in the shared code and the scale factor can have floating point 
value on Windows. (for example 1.5).
  It is better to round the final width and height after scaling them.
 
  Thanks,
  Alexandr.



 

Regards,

Rajeev Chamyal

 

[9] Review Request JDK-8147648 [hidpi] multiresolution image: wrong resolution variant is used as icon in the Unity panel

[Rajeev Chamyal]

Hello All,

 

Please review the following webrev.

 

Webrev: http://cr.openjdk.java.net/~rchamyal/8147648/webrev.00/ 

Bug: https://bugs.openjdk.java.net/browse/JDK-8147648 

 

Issue: Wrong resolution variant is used as icon in the Unity panel.

Cause: The screen transforms are not applied to find the correct resolution 
variant image in current implementation.

Fix: Applied the screen transforms to graphics object.

 

Regards,

Rajeev Chamyal

Re: Review Request JDK-8159152 Ctrl+F6, Ctrl+F5 doesn't work for iconified InternalFrame

[Rajeev Chamyal]

Hello Alexandr,

 

TestJInternalFrameMinimize test passes after this fix.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 17 June 2016 18:35
To: Rajeev Chamyal; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re:  Review Request JDK-8159152 Ctrl+F6, Ctrl+F5 doesn't 
work for iconified InternalFrame

 

On 6/17/2016 10:09 AM, Rajeev Chamyal wrote:



Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8159152 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8159152/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8159152/webrev.00/

 

Issue: Internal frame cache is not getting updated properly on iconifyFrame if 
there are less than 3 frames on the desktop.

Fix: Updated the iconifyFrame method so that frame cache updates properly on 
internal frame remove and icon addition to desktop.


  Could you check that the test 
test/javax/swing/JInternalFrame/8145060/TestJInternalFrameMinimize.java passes 
after the fix?

  Thanks,
  Alexandr.




 

Regards,

Rajeev Chamyal

 

 

 

Review Request JDK-8159152 Ctrl+F6, Ctrl+F5 doesn't work for iconified InternalFrame

[Rajeev Chamyal]

Hello All,

 

Please review the following webrev.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8159152 

Webrev: http://cr.openjdk.java.net/~rchamyal/8159152/webrev.00/

 

Issue: Internal frame cache is not getting updated properly on iconifyFrame if 
there are less than 3 frames on the desktop.

Fix: Updated the iconifyFrame method so that frame cache updates properly on 
internal frame remove and icon addition to desktop.

 

Regards,

Rajeev Chamyal

 

 

Re: Review Request JDK-8152419 JColorChooser throws Exception

[Rajeev Chamyal]

Looks fine to me.

 

Regards,

Rajeev Chamyal

 

From: Prem Balakrishnan 
Sent: 15 June 2016 14:51
To: Rajeev Chamyal; Alexander Scherbatiy; Sergey Bylokhov; 
swing-dev@openjdk.java.net
Subject: RE:  Review Request JDK-8152419 JColorChooser throws 
Exception

 

Hi Rajeev,

 

Thank you for the review.

Updated patch as per review comment.

http://cr.openjdk.java.net/~pkbalakr/8152419/webrev.03/

 

Regards,
Prem

 

 

From: Rajeev Chamyal 
Sent: Wednesday, June 15, 2016 2:38 PM
To: Alexander Scherbatiy; Prem Balakrishnan; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: RE:  Review Request JDK-8152419 JColorChooser throws 
Exception

 

Hello Prem,

 

testResult variable is accessed in 2 different threads. It should be declared 
volatile.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 10 June 2016 19:53
To: Prem Balakrishnan; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re:  Review Request JDK-8152419 JColorChooser throws 
Exception

 


The fix looks good to me.

Thanks,
Alexandr.

On 6/10/2016 12:39 PM, Prem Balakrishnan wrote:

Hi Alexander,

 

Please review updated patch as per review comments.

http://cr.openjdk.java.net/~pkbalakr/8152419/webrev.02/ 

 

Regards,
Prem

 

From: Alexander Scherbatiy 
Sent: Tuesday, May 31, 2016 4:04 PM
To: Prem Balakrishnan; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: Review Request JDK-8152419 JColorChooser throws Exception

 

On 31/05/16 14:03, Prem Balakrishnan wrote:

Hi Alexander,

 

Please review the updated patch.

http://cr.openjdk.java.net/~pkbalakr/8152419/webrev.01/


Math.max(getWidth() - this.insets.left - this.insets.right, getWidth()) can 
give incorrect result for the case where a component size is 50x50 and insets 
are [10, 10, 10 , 10]. max(50-10-10, 50) = 50 but the expected results is 30.

The correct formula should be max(width-insets.left-insets.right, 
minWidthValue) where minWidthValue is zero or some specified minimal value.

The DiagramComponent.paintComponent() code tries to create an array of size 
width*height and BufferedImage with component size. In this case it may be 
better just to check that the component size minus insets is
greater than zero. If it is less or equal to zero we can just return from the 
paintComponent() method.

Thanks,
Alexandr.



 

 

Regards,

Prem

 

From: Alexander Scherbatiy 
Sent: Monday, May 30, 2016 9:42 PM
To: Prem Balakrishnan; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: Review Request JDK-8152419 JColorChooser throws Exception

 

On 30/05/16 12:39, Prem Balakrishnan wrote:

Hi,

Please review fix for JDK9,

Bug: https://bugs.openjdk.java.net/browse/JDK-8152419 

Webrev: http://cr.openjdk.java.net/~pkbalakr/8152419/webrev.00/ 

 

  

Issue:

JColorChooser throws Exception(NegativeArraySizeException)

 

Fix:

Absolute value is passed while creating array.

   If component size is 10x10 and insets are [30, 30, 30, 30] the absolute 
value of the difference will be  abs(10 - 30 - 30)=50.
   It seems that the right component size should be zero or some minimal values 
which is max(width-insets.left-insets.right, minWidthValue).

  Thanks,
  Alexandr.



 

Regards,
Prem

 

 

 

 

Re: Review Request JDK-8152419 JColorChooser throws Exception

[Rajeev Chamyal]

Hello Prem,

 

testResult variable is accessed in 2 different threads. It should be declared 
volatile.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 10 June 2016 19:53
To: Prem Balakrishnan; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re:  Review Request JDK-8152419 JColorChooser throws 
Exception

 


The fix looks good to me.

Thanks,
Alexandr.

On 6/10/2016 12:39 PM, Prem Balakrishnan wrote:

Hi Alexander,

 

Please review updated patch as per review comments.

http://cr.openjdk.java.net/~pkbalakr/8152419/webrev.02/ 

 

Regards,
Prem

 

From: Alexander Scherbatiy 
Sent: Tuesday, May 31, 2016 4:04 PM
To: Prem Balakrishnan; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: Review Request JDK-8152419 JColorChooser throws Exception

 

On 31/05/16 14:03, Prem Balakrishnan wrote:

Hi Alexander,

 

Please review the updated patch.

http://cr.openjdk.java.net/~pkbalakr/8152419/webrev.01/


Math.max(getWidth() - this.insets.left - this.insets.right, getWidth()) can 
give incorrect result for the case where a component size is 50x50 and insets 
are [10, 10, 10 , 10]. max(50-10-10, 50) = 50 but the expected results is 30.

The correct formula should be max(width-insets.left-insets.right, 
minWidthValue) where minWidthValue is zero or some specified minimal value.

The DiagramComponent.paintComponent() code tries to create an array of size 
width*height and BufferedImage with component size. In this case it may be 
better just to check that the component size minus insets is
greater than zero. If it is less or equal to zero we can just return from the 
paintComponent() method.

Thanks,
Alexandr.





 

 

Regards,

Prem

 

From: Alexander Scherbatiy 
Sent: Monday, May 30, 2016 9:42 PM
To: Prem Balakrishnan; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re: Review Request JDK-8152419 JColorChooser throws Exception

 

On 30/05/16 12:39, Prem Balakrishnan wrote:

Hi,

Please review fix for JDK9,

Bug: https://bugs.openjdk.java.net/browse/JDK-8152419 

Webrev: http://cr.openjdk.java.net/~pkbalakr/8152419/webrev.00/ 

 

  

Issue:

JColorChooser throws Exception(NegativeArraySizeException)

 

Fix:

Absolute value is passed while creating array.

   If component size is 10x10 and insets are [30, 30, 30, 30] the absolute 
value of the difference will be  abs(10 - 30 - 30)=50.
   It seems that the right component size should be zero or some minimal values 
which is max(width-insets.left-insets.right, minWidthValue).

  Thanks,
  Alexandr.





 

Regards,
Prem

 

 

 

 

Re: [9] Review request for JDK-8150176 [hidpi] wrong resolution variant of multi-res. image is used for TrayIcon

[Rajeev Chamyal]

Hello Alexandr,

 

Thanks for the review. I have updated the webrev as per review comments.

http://cr.openjdk.java.net/~rchamyal/8150176/webrev.01/

I tried drawing the image directly to paint graphics without buffered image and 
it was getting cropped.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 09 June 2016 20:43
To: Rajeev Chamyal; swing-dev@openjdk.java.net; Sergey Bylokhov
Subject: Re:  [9] Review request for JDK-8150176 [hidpi] wrong 
resolution variant of multi-res. image is used for TrayIcon

 

On 6/9/2016 11:55 AM, Rajeev Chamyal wrote:



Hello All,

 

Please review the following fix.

Bug: https://bugs.openjdk.java.net/browse/JDK-8150176 

Webrev: HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8150176/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8150176/webrev.00/
 

 

Issue: Wrong resolution variant image is used in Tray Icon.

Fix : Applying the device transform to graphics object to select the correct 
image.

The image could be cropped on Linux because the high resolution icon which 
size is bigger that the original image is drawn to the buffered image with 
un-scaled size curW x CurH. 
  It is better to get a resolution variant from the multi-resolution image, 
draw it to a buffered image with the same scaled size and then draw the 
buffered image to the paint graphics using original size:
---
Image resolutionVariant = ((MultiResolutionImage) 
image).getResolutionVariant(scaleX * curW, scaleY * curH);
BufferedImage bufImage = new BufferedImage(scaleX * curW, scaleY * curH, 
BufferedImage.TYPE_INT_ARGB);
// ...
gr.drawImage(image, 0, 0, scaleX * curW, scaleY * curH, observer);
// ...
g.drawImage(bufImage, 0, 0, curW, curH, observer); // non scaled width and 
height
---

  By the way, is the buffered image necessary in this case? Is it possible to 
draw the image directly to the paint graphics?
---
 g.drawImage(image, 0, 0, curW, curH, null);
---

Thanks,
Alexandr.
  

 

Regards,

Rajeev Chamyal

 

 

Re: Fix for JDK-8065861 : Pressing Esc does not set 'canceled' property of ProgressMonitor

[Rajeev Chamyal]

Looks good to me.

Regards,
Rajeev Chamyal

-Original Message-
From: Alexandr Scherbatiy 
Sent: 10 June 2016 13:06
To: Ajit Ghaisas; Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re: Fix for JDK-8065861 : Pressing Esc does not set 'canceled' 
property of ProgressMonitor


The fix looks good to me.

Thanks,
Alexandr.

On 6/10/2016 8:50 AM, Ajit Ghaisas wrote:
> Hi,
>
>  Thanks Alex for spotting probable exceptions in code changes.
>   I have corrected the code to address them.
>
>   Here is the updated webrev. Request you to review.
>   http://cr.openjdk.java.net/~aghaisas/8065861/webrev.01/
>
> Regards,
> Ajit
> 
>
> -Original Message-
> From: Alexandr Scherbatiy
> Sent: Thursday, June 09, 2016 8:56 PM
> To: Ajit Ghaisas; Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net
> Subject: Re: Fix for JDK-8065861 : Pressing Esc does not set 'canceled' 
> property of ProgressMonitor
>
> On 6/8/2016 5:35 PM, Ajit Ghaisas wrote:
>> Hi,
>>
>> Bug :
>> https://bugs.openjdk.java.net/browse/JDK-8065861
>>
>> Issue :
>> Pressing Esc does not set 'canceled' property of ProgressMonitor
>>
>> Analysis :
>> ProgressMonitor option pane only gets hidden on pressing Escape key.
>> It is not truly canceled as isCanceled() method continues to return false.
>>
>> Fix :
>> On Pressing Escape key JOptionPane.CLOSED_OPTION value is set in JOptionPane 
>> from BasicOptionPaneUI.java.
>> This value is used as a condition in isCanceled() to identify 
>> ProgressMonitor is canceled by pressing Escape key.
>>
>> Please review the webrev:
>> http://cr.openjdk.java.net/~aghaisas/8065861/webrev.00/
>The updated code calls methods from 'v' object before it is checked to 
> the null which can lead to the NPE. The cancelOption[0] also can throw the 
> IOBE before the arrays length checking.
>
> Thanks,
> Alexandr.
>> On review completion, I will raise a CCC request for documentation change.
>>
>> Regards,
>> Ajit

Re: [9] Review request for JDK-8159135 [PIT] javax/swing/JMenuItem/8152981/MenuItemIconTest.java always fail

[Rajeev Chamyal]

Hello Alexandr,

 

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8159135/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 10 June 2016 19:52
To: Rajeev Chamyal; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8159135 [PIT] 
javax/swing/JMenuItem/8152981/MenuItemIconTest.java always fail

 


On 6/10/2016 12:36 PM, Rajeev Chamyal wrote:



Hello All,

 

Please review the following fix.

Bug : https://bugs.openjdk.java.net/browse/JDK-8159135 

Webrev : HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8159135/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8159135/webrev.00/

 

Issue : MenuItemIconTest fails on windows 10.

Cause : In windows 10  menu bar color is white and test checks for red color 
only.

Fix: checking for other color values(green and blue) also.

Is it possible to compare color using equals method like other objects:   
Color.RED.equals(c)?

   Thanks,
   Alexandr.



 

Regards,

Rajeev Chamyal

 

[9] Review request for JDK-8159135 [PIT] javax/swing/JMenuItem/8152981/MenuItemIconTest.java always fail

[Rajeev Chamyal]

Hello All,

 

Please review the following fix.

Bug : https://bugs.openjdk.java.net/browse/JDK-8159135 

Webrev : http://cr.openjdk.java.net/~rchamyal/8159135/webrev.00/

 

Issue : MenuItemIconTest fails on windows 10.

Cause : In windows 10  menu bar color is white and test checks for red color 
only.

Fix: checking for other color values(green and blue) also.

 

Regards,

Rajeev Chamyal

[9] Review request for JDK-8150176 [hidpi] wrong resolution variant of multi-res. image is used for TrayIcon

[Rajeev Chamyal]

Hello All,

 

Please review the following fix.

Bug: https://bugs.openjdk.java.net/browse/JDK-8150176 

Webrev: http://cr.openjdk.java.net/~rchamyal/8150176/webrev.00/ 

 

Issue: Wrong resolution variant image is used in Tray Icon.

Fix : Applying the device transform to graphics object to select the correct 
image.

 

Regards,

Rajeev Chamyal

 

Re: Review request for 8132771: [TEST_BUG][macosx] Test javax/swing/JTree/DnD/LastNodeLowerHalfDrop.java fails for MacOSX

[Rajeev Chamyal]

Looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 31 May 2016 13:28
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  Review request for 8132771: [TEST_BUG][macosx] Test 
javax/swing/JTree/DnD/LastNodeLowerHalfDrop.java fails for MacOSX

 

Hi All,

Please review the code changes with inputs provided.

 

http://cr.openjdk.java.net/~aniyogi/8132771/webrev.01/

 

With Regards,

Avik Niyogi

 

On 31-May-2016, at 1:24 pm, Rajeev Chamyal mailto:rajeev.cham...@oracle.com"rajeev.cham...@oracle.com> wrote:

 

Hello Avik,

 

The frame should be disposed in case of exception also.

 

Regards,

Rajeev Chamyal

 

 

From: Avik Niyogi 
Sent: 31 May 2016 11:14
To: Rajeev Chamyal; Alexander Scherbatiy; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject:  Review request for 8132771: [TEST_BUG][macosx] Test 
javax/swing/JTree/DnD/LastNodeLowerHalfDrop.java fails for MacOSX

 

Hi All,

 

Kindly review the fix for JDK9.

Bug: https://bugs.openjdk.java.net/browse/JDK-8132771

 

Webrev: http://cr.openjdk.java.net/~aniyogi/8132771/webrev.00/

 

Issue: LastNodeLowerHalfDrop Test case throws an exception when the behaviour 
is as expected.

 

Cause: The robot does not have enough delay to run appropriately.

 

Fix: The test case was fixed to have enough delay for the test case.

 

With Regards,

Avik Niyogi

 

Re: [9] Review request for JDK-8146319 JEditorPane function setPage leaves a file lock

[Rajeev Chamyal]

Hello Alexandr,

 

Please review the updated webrev.

 

http://cr.openjdk.java.net/~rchamyal/8146319/webrev.02/

Update: Updated code to use  try with resources.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 01 June 2016 19:30
To: Rajeev Chamyal; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8146319 JEditorPane 
function setPage leaves a file lock

 

On 6/1/2016 12:21 PM, Rajeev Chamyal wrote:



Hello All,

 

Please review the following webrev.

Bug : https://bugs.openjdk.java.net/browse/JDK-8146319 

Webrev : HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8146319/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8146319/webrev.00/
 

 

Issue: JEditorPane::read method is not closing the InputStreamReader.

Fix: closing the InputStreamReader in finally block.

  Is it possible to use try-with-resources statement for the reader?

   Thanks,
   Alexandr. 

 

Regards,

Rajeev Chamyal

 

 

Re: [9] Review request for JDK-[TEST_BUG] test/javax/swing/JPopupMenu/8147521/PopupMenuTest.java: compilation failed

[Rajeev Chamyal]

Hello Alexandr,

 

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8158358/webrev.01/

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 02 June 2016 01:20
To: Rajeev Chamyal; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-[TEST_BUG] 
test/javax/swing/JPopupMenu/8147521/PopupMenuTest.java: compilation failed

 

On 6/1/2016 10:35 PM, Rajeev Chamyal wrote:



Hello All,

 

Please review the following webrev

 

Webrev : HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8158358/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8158358/webrev.00/

Bug : https://bugs.openjdk.java.net/browse/JDK-8158358 

 

Fix : Fixed the compilation error.

   The initial summary of the test should be preserved.

  Thanks,
  Alexandr.



 

Regards,

Rajeev Chamyal

 

 

[9] Review request for JDK-[TEST_BUG] test/javax/swing/JPopupMenu/8147521/PopupMenuTest.java: compilation failed

[Rajeev Chamyal]

Hello All,

 

Please review the following webrev

 

Webrev : http://cr.openjdk.java.net/~rchamyal/8158358/webrev.00/

Bug : https://bugs.openjdk.java.net/browse/JDK-8158358 

 

Fix : Fixed the compilation error.

 

Regards,

Rajeev Chamyal

 

Re: [9] Review request for JDK-8146319 JEditorPane function setPage leaves a file lock

[Rajeev Chamyal]

Hello Semyon,

 

Thanks for the review. Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8146319/webrev.01/

 

Update: Updated test to use File.createTempFile() for creating temp file.

 

Regards,

Rajeev Chamyal

 

From: Semyon Sadetsky 
Sent: 01 June 2016 15:01
To: Rajeev Chamyal; Alexander Scherbatiy; Sergey Bylokhov; 
swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8146319 JEditorPane 
function setPage leaves a file lock

 

Hi,

The fix looks good.

In the test you could use File.createTempFile() or better 
Files.createTempFile() 

--Semyon

On 6/1/2016 12:21 PM, Rajeev Chamyal wrote:

Hello All,

 

Please review the following webrev.

Bug : https://bugs.openjdk.java.net/browse/JDK-8146319 

Webrev : HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8146319/webrev.00/"http://cr.openjdk.java.net/~rchamyal/8146319/webrev.00/
 

 

Issue: JEditorPane::read method is not closing the InputStreamReader.

Fix: closing the InputStreamReader in finally block.

 

Regards,

Rajeev Chamyal

 

 

[9] Review request for JDK-8146319 JEditorPane function setPage leaves a file lock

[Rajeev Chamyal]

Hello All,

 

Please review the following webrev.

Bug : https://bugs.openjdk.java.net/browse/JDK-8146319 

Webrev : http://cr.openjdk.java.net/~rchamyal/8146319/webrev.00/ 

 

Issue: JEditorPane::read method is not closing the InputStreamReader.

Fix: closing the InputStreamReader in finally block.

 

Regards,

Rajeev Chamyal

 

Re: Review request for 8132771: [TEST_BUG][macosx] Test javax/swing/JTree/DnD/LastNodeLowerHalfDrop.java fails for MacOSX

[Rajeev Chamyal]

Hello Avik,

 

The frame should be disposed in case of exception also.

 

Regards,

Rajeev Chamyal

 

 

From: Avik Niyogi 
Sent: 31 May 2016 11:14
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject:  Review request for 8132771: [TEST_BUG][macosx] Test 
javax/swing/JTree/DnD/LastNodeLowerHalfDrop.java fails for MacOSX

 

Hi All,

 

Kindly review the fix for JDK9.

Bug: https://bugs.openjdk.java.net/browse/JDK-8132771

 

Webrev: http://cr.openjdk.java.net/~aniyogi/8132771/webrev.00/

 

Issue: LastNodeLowerHalfDrop Test case throws an exception when the behaviour 
is as expected.

 

Cause: The robot does not have enough delay to run appropriately.

 

Fix: The test case was fixed to have enough delay for the test case.

 

With Regards,

Avik Niyogi

Re: Fix for JDK-6827800 : Default button is activated even when it is invisible

[Rajeev Chamyal]

Fix looks fine to me.

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 25 May 2016 22:12
To: Ajit Ghaisas; swing-dev@openjdk.java.net; Alexander Scherbatiy; Rajeev 
Chamyal
Subject: Re: Fix for JDK-6827800 : Default button is activated even when it is 
invisible

Looks fine to me.

On 25.05.16 14:45, Ajit Ghaisas wrote:
> This the fix for bug : 
> https://bugs.openjdk.java.net/browse/JDK-6827800
>
> Default button is activated even when it is invisible
>
>
>
> Root cause :
>
> Accepting Key press event even if the default button is invisible.
>
>
>
> Fix :
>
> In BasicRootPaneUI.java, accept key press only if the default 
> button is being shown.
>
>
>
> Please review the webrev :
>  http://cr.openjdk.java.net/~aghaisas/6827800/webrev.00/
>
>
>
> Regards,
>
> Ajit
>
>
>


--
Best regards, Sergey.

Re: Review request for 8144161: [TESTBUG] [macosx] Test javax/swing/plaf/basic/BasicComboPopup/7072653/bug7072653.java fails for mac

[Rajeev Chamyal]

Looks ok to me.

Can you please add specific class imports instead of *.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 25 May 2016 12:53
To: Alexander Scherbatiy
Cc: Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re:  Review request for 8144161: [TESTBUG] [macosx] Test 
javax/swing/plaf/basic/BasicComboPopup/7072653/bug7072653.java fails for mac

 

Hi All,

 

Please find updated changes as per inputs received:

 Bug: https://bugs.openjdk.java.net/browse/JDK-8144161

 

Webrev: http://cr.openjdk.java.net/~aniyogi/8144161/webrev.01/

 

With Regards,

Avik Niyogi

 

On 24-May-2016, at 5:58 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

On 24/05/16 15:22, Avik Niyogi wrote:

Hi All, 

 

Kindly review the fix for JDK9.

Bug: HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-7124218"https://bugs.openjdk.java.net/browse/JDK-8144161

 

Webrev:http://HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8144161/webrev.00/"cr.openjdk.java.net/~aniyogi/8144161/webrev.00/

 

Issue: Test case throws an exception when the behaviour is as expected.

 

Cause: The expected behaviour for comboBox was not accounted for Aqua look and 
feel in test case.

 

Fix: The test case was fixed to account for the Aqua LAF behaviour for comboBox 
dropdown menu which extends beyond the screen Insets (the dock height) for Mac.

 

  - the bug link points to the bug 7124218 instead of 8144161
  - The exception should be re-thrown on the line 63
  - Some code formatting change is not clear (see for example line 24 or 133)

  Thanks,
  Alexandr.



With Regards,

Avik Niyogi 

 

 

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello Phil,

Please review the updated webrev.
http://cr.openjdk.java.net/~rchamyal/8147521/webrev.07/
Changes: Updated the Javadoc.
Added the following note: 
* This method is intended to be used only by  PopupFactory sub-classes.

Regards,
Rajeev Chamyal

-Original Message-
From: Alexander Scherbatiy 
Sent: 12 May 2016 17:05
To: Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net; Alan Snyder
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup


The fix looks good to me.

Thanks,
Alexandr.

On 12/05/16 15:26, Sergey Bylokhov wrote:
> Looks fine to me.
>
> On 12.05.16 14:07, Rajeev Chamyal wrote:
>> Hello All,
>>
>> Please review the updated webrev.
>>
>> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.06/
>> Changes: Updated the documentation.
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -Original Message-
>> From: Rajeev Chamyal
>> Sent: 12 May 2016 13:32
>> To: Sergey Bylokhov; Alexander Scherbatiy; 
>> swing-dev@openjdk.java.net; Alan Snyder
>> Subject: RE:  [9] Review request for JDK-8147521 [macosx] 
>> Internal API Usage: setPopupType used to force creation of 
>> heavyweight popup
>>
>> Hello Sergey,
>>
>> Please review the updated webrev as per review comments.
>>
>> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.05/
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -Original Message-
>> From: Sergey Bylokhov
>> Sent: 11 May 2016 18:16
>> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net; 
>> Alan Snyder
>> Subject: Re:  [9] Review request for JDK-8147521 [macosx] 
>> Internal API Usage: setPopupType used to force creation of 
>> heavyweight popup
>>
>> Code change looks fine.
>> I think that the spec of isHeavyWeightPopup() should be updated. the 
>> "true" means that the HW popup will be forced, but the false does not 
>> mean that LW will be used(I guess in this case some default type 
>> will/can be selected).
>>
>> On 11.05.16 14:45, Rajeev Chamyal wrote:
>>> Hello Alexandr,
>>>
>>> Please review the updated webrev.
>>>
>>> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.04/
>>>
>>> Regards,
>>> Rajeev Chamyal
>>>
>>> -Original Message-
>>> From: Alexander Scherbatiy
>>> Sent: 11 May 2016 16:29
>>> To: Rajeev Chamyal
>>> Cc: swing-dev@openjdk.java.net; Sergey Bylokhov; Alan Snyder
>>> Subject: Re:  [9] Review request for JDK-8147521 [macosx] 
>>> Internal API Usage: setPopupType used to force creation of 
>>> heavyweight popup
>>>
>>> On 5/11/2016 1:17 PM, Rajeev Chamyal wrote:
>>>> Hello All,
>>>>
>>>> Please review the updated webrev.
>>>> Bug : https://bugs.openjdk.java.net/browse/JDK-8147521
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~rchamyal/8147521/webrev.03/
>>>>
>>>> Update: Added a new protected method getpop in PopupFactory.java 
>>>> with a Boolean parameter for specifying heavy weight popup.
>>>
>>> The provided method looks good to me.
>>>
>>> I would leave the javadoc review to the native speakers. What I 
>>> only see that there is a missed space in {@codey}, typo in 
>>> "paramaters" word and probably there should be a comma before 
>>> "otherwise false".
>>>
>>>Thanks,
>>>Alexandr.
>>>
>>>>
>>>> Regards,
>>>> Rajeev Chamyal
>>>>
>>>> -Original Message-
>>>> From: Alan Snyder [mailto:javali...@cbfiddle.com]
>>>> Sent: 11 May 2016 03:39
>>>> To: Alexandr Scherbatiy
>>>> Cc: swing-dev@openjdk.java.net
>>>> Subject: Re:  [9] Review request for JDK-8147521 
>>>> [macosx] Internal API Usage: setPopupType used to force creation of 
>>>> heavyweight popup
>>>>
>>>> I use only heavy weight.
>>>>
>>>>
>>>>> On May 10, 2016, at 12:46 PM, Alexandr Scherbatiy 
>>>>>  wrote:
>>>>>
>>>>>
>>>>> Do you need to use medium-weight popups in your application or 
>>>>> light/heavy-weight popups are enough?
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>> On 5/10/2016 10:23 PM, Alan Snyder wrote:
>>>>>>> On May 10, 2016, at 5:58 AM, Sergey Bylokhov 
>>>>>>>  wrote:
>>>>>>>
>>>>>>> Hi, Alan.
>>>>>>> Can you please take a look to the proposed solutions? Thanks.
>>>>>>>
>>>>>>>
>>>>>> Approach 2 matches what I currently do. The problem noted by 
>>>>>> Rajeev does not happen because my popup factory calls
>>>>>> setPopupType() on each call to the public getPopup() before 
>>>>>> invoking the superclass method.
>>>>>>
>>>>>> I think the original version of Approach 1 would work. My factory 
>>>>>> would override the public getPopup() method and pass true to the 
>>>>>> five parameter method.
>>>>>>
>>>>>> I think the revised version of Approach 1 does not work for me 
>>>>>> because the new flag is only tested if the first attempt to 
>>>>>> create a popup fails.
>>>>>>
>>>
>>
>>
>> --
>> Best regards, Sergey.
>>
>
>

Re: [9] Review request for JDK-7070795 High contrast colour scheme fails to be applied to JFormattedTextField

[Rajeev Chamyal]

Hello Sergey,

I have updated webrev as per review comments.

http://cr.openjdk.java.net/~rchamyal/7070795/webrev.01/

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 19 May 2016 20:30
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-7070795 High contrast 
colour scheme fails to be applied to JFormattedTextField

The fix looks fine. Note that in the test the count of characters for 
JFormattedTextFieldTest is set to 5, but 6 is used.

On 19.05.16 18:25, Rajeev Chamyal wrote:
> Hello All,
>
>
>
> Please review the following webrev.
>
> Bug : https://bugs.openjdk.java.net/browse/JDK-7070795
>
> Webrev : http://cr.openjdk.java.net/~rchamyal/7070795/webrev.00/
>
>
>
> Issue: On changing the Windows theme to high contrast the high 
> contrast color scheme is not getting applied to JFormattedTextFiled.
>
> Fix: Updated Formatted text field foreground and background properties.
>
>
>
> Regards,
>
> Rajeev Chamyal
>


--
Best regards, Sergey.

[9] Review request for JDK-7070795 High contrast colour scheme fails to be applied to JFormattedTextField

[Rajeev Chamyal]

Hello All,

 

Please review the following webrev.

Bug : https://bugs.openjdk.java.net/browse/JDK-7070795 

Webrev : http://cr.openjdk.java.net/~rchamyal/7070795/webrev.00/ 

 

Issue: On changing the Windows theme to high contrast the high contrast color 
scheme is not getting applied to JFormattedTextFiled.

Fix: Updated Formatted text field foreground and background properties.

 

Regards,

Rajeev Chamyal

Re: [9] Fix for JDK-7172750 : Nimbus ScrollBar:ScrollBarThumb[Pressed].backgroundPainter is never invoked

[Rajeev Chamyal]

Hello Ajit,

Fix looks fine to me. Few comments about test case.
1) @run main is missing in test.
2) JFrame dispose should be done in swing thread.

Regards,
Rajeev Chamyal


-Original Message-
From: Ajit Ghaisas 
Sent: 06 May 2016 12:52
To: Alexander Scherbatiy; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject:  [9] Fix for JDK-7172750 : Nimbus 
ScrollBar:ScrollBarThumb[Pressed].backgroundPainter is never invoked

Hi,

Bug : 
 https://bugs.openjdk.java.net/browse/JDK-7172750

Issue :
Nimbus ScrollBar:ScrollBarThumb[Pressed].backgroundPainter is never invoked

Root Cause :
There is no differentiation between 'MouseOver' and Mouse 'Pressed' in 
scroll thumb painting.

Fix :
 1. Used existing member boolean 'dragging' from BasicScrollBarUI class in 
SynthScrollBarUI class to differentiate between "mouse over" and "mouse 
pressed" state.
 2. Added a test case - it passes on Windows, Linux and Mac.

Please review the webrev : 
http://cr.openjdk.java.net/~aghaisas/7172750/webrev.00/

Regards,
Ajit

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello All,

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.06/
Changes: Updated the documentation.

Regards,
Rajeev Chamyal

-Original Message-
From: Rajeev Chamyal 
Sent: 12 May 2016 13:32
To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net; Alan 
Snyder
Subject: RE:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

Hello Sergey,

Please review the updated webrev as per review comments.

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.05/

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov
Sent: 11 May 2016 18:16
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net; Alan 
Snyder
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

Code change looks fine.
I think that the spec of isHeavyWeightPopup() should be updated. the "true" 
means that the HW popup will be forced, but the false does not mean that LW 
will be used(I guess in this case some default type will/can be selected).

On 11.05.16 14:45, Rajeev Chamyal wrote:
> Hello Alexandr,
>
> Please review the updated webrev.
>
> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.04/
>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Alexander Scherbatiy
> Sent: 11 May 2016 16:29
> To: Rajeev Chamyal
> Cc: swing-dev@openjdk.java.net; Sergey Bylokhov; Alan Snyder
> Subject: Re:  [9] Review request for JDK-8147521 [macosx] 
> Internal API Usage: setPopupType used to force creation of heavyweight 
> popup
>
> On 5/11/2016 1:17 PM, Rajeev Chamyal wrote:
>> Hello All,
>>
>> Please review the updated webrev.
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8147521
>>
>> Webrev: http://cr.openjdk.java.net/~rchamyal/8147521/webrev.03/
>>
>> Update: Added a new protected method getpop in PopupFactory.java with a 
>> Boolean parameter for specifying heavy weight popup.
>
> The provided method looks good to me.
>
> I would leave the javadoc review to the native speakers. What I only see 
> that there is a missed space in {@codey}, typo in "paramaters" word and 
> probably there should be a comma before "otherwise false".
>
>Thanks,
>Alexandr.
>
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -Original Message-
>> From: Alan Snyder [mailto:javali...@cbfiddle.com]
>> Sent: 11 May 2016 03:39
>> To: Alexandr Scherbatiy
>> Cc: swing-dev@openjdk.java.net
>> Subject: Re:  [9] Review request for JDK-8147521 [macosx] 
>> Internal API Usage: setPopupType used to force creation of 
>> heavyweight popup
>>
>> I use only heavy weight.
>>
>>
>>> On May 10, 2016, at 12:46 PM, Alexandr Scherbatiy 
>>>  wrote:
>>>
>>>
>>> Do you need to use medium-weight popups in your application or 
>>> light/heavy-weight popups are enough?
>>>
>>> Thanks,
>>> Alexandr.
>>>
>>> On 5/10/2016 10:23 PM, Alan Snyder wrote:
>>>>> On May 10, 2016, at 5:58 AM, Sergey Bylokhov  
>>>>> wrote:
>>>>>
>>>>> Hi, Alan.
>>>>> Can you please take a look to the proposed solutions? Thanks.
>>>>>
>>>>>
>>>> Approach 2 matches what I currently do. The problem noted by Rajeev does 
>>>> not happen because my popup factory calls setPopupType() on each call to 
>>>> the public getPopup() before invoking the superclass method.
>>>>
>>>> I think the original version of Approach 1 would work. My factory would 
>>>> override the public getPopup() method and pass true to the five parameter 
>>>> method.
>>>>
>>>> I think the revised version of Approach 1 does not work for me because the 
>>>> new flag is only tested if the first attempt to create a popup fails.
>>>>
>


--
Best regards, Sergey.

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello Sergey,

Please review the updated webrev as per review comments.

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.05/

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 11 May 2016 18:16
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net; Alan 
Snyder
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

Code change looks fine.
I think that the spec of isHeavyWeightPopup() should be updated. the "true" 
means that the HW popup will be forced, but the false does not mean that LW 
will be used(I guess in this case some default type will/can be selected).

On 11.05.16 14:45, Rajeev Chamyal wrote:
> Hello Alexandr,
>
> Please review the updated webrev.
>
> http://cr.openjdk.java.net/~rchamyal/8147521/webrev.04/
>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Alexander Scherbatiy
> Sent: 11 May 2016 16:29
> To: Rajeev Chamyal
> Cc: swing-dev@openjdk.java.net; Sergey Bylokhov; Alan Snyder
> Subject: Re:  [9] Review request for JDK-8147521 [macosx] 
> Internal API Usage: setPopupType used to force creation of heavyweight 
> popup
>
> On 5/11/2016 1:17 PM, Rajeev Chamyal wrote:
>> Hello All,
>>
>> Please review the updated webrev.
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8147521
>>
>> Webrev: http://cr.openjdk.java.net/~rchamyal/8147521/webrev.03/
>>
>> Update: Added a new protected method getpop in PopupFactory.java with a 
>> Boolean parameter for specifying heavy weight popup.
>
> The provided method looks good to me.
>
> I would leave the javadoc review to the native speakers. What I only see 
> that there is a missed space in {@codey}, typo in "paramaters" word and 
> probably there should be a comma before "otherwise false".
>
>Thanks,
>Alexandr.
>
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -Original Message-
>> From: Alan Snyder [mailto:javali...@cbfiddle.com]
>> Sent: 11 May 2016 03:39
>> To: Alexandr Scherbatiy
>> Cc: swing-dev@openjdk.java.net
>> Subject: Re:  [9] Review request for JDK-8147521 [macosx] 
>> Internal API Usage: setPopupType used to force creation of 
>> heavyweight popup
>>
>> I use only heavy weight.
>>
>>
>>> On May 10, 2016, at 12:46 PM, Alexandr Scherbatiy 
>>>  wrote:
>>>
>>>
>>> Do you need to use medium-weight popups in your application or 
>>> light/heavy-weight popups are enough?
>>>
>>> Thanks,
>>> Alexandr.
>>>
>>> On 5/10/2016 10:23 PM, Alan Snyder wrote:
>>>>> On May 10, 2016, at 5:58 AM, Sergey Bylokhov  
>>>>> wrote:
>>>>>
>>>>> Hi, Alan.
>>>>> Can you please take a look to the proposed solutions? Thanks.
>>>>>
>>>>>
>>>> Approach 2 matches what I currently do. The problem noted by Rajeev does 
>>>> not happen because my popup factory calls setPopupType() on each call to 
>>>> the public getPopup() before invoking the superclass method.
>>>>
>>>> I think the original version of Approach 1 would work. My factory would 
>>>> override the public getPopup() method and pass true to the five parameter 
>>>> method.
>>>>
>>>> I think the revised version of Approach 1 does not work for me because the 
>>>> new flag is only tested if the first attempt to create a popup fails.
>>>>
>


--
Best regards, Sergey.

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello Alexandr,

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.04/

Regards,
Rajeev Chamyal

-Original Message-
From: Alexander Scherbatiy 
Sent: 11 May 2016 16:29
To: Rajeev Chamyal
Cc: swing-dev@openjdk.java.net; Sergey Bylokhov; Alan Snyder
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

On 5/11/2016 1:17 PM, Rajeev Chamyal wrote:
> Hello All,
>
> Please review the updated webrev.
> Bug : https://bugs.openjdk.java.net/browse/JDK-8147521
>
> Webrev: http://cr.openjdk.java.net/~rchamyal/8147521/webrev.03/
>
> Update: Added a new protected method getpop in PopupFactory.java with a 
> Boolean parameter for specifying heavy weight popup.

The provided method looks good to me.

I would leave the javadoc review to the native speakers. What I only see 
that there is a missed space in {@codey}, typo in "paramaters" word and 
probably there should be a comma before "otherwise false".

   Thanks,
   Alexandr.

>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Alan Snyder [mailto:javali...@cbfiddle.com]
> Sent: 11 May 2016 03:39
> To: Alexandr Scherbatiy
> Cc: swing-dev@openjdk.java.net
> Subject: Re:  [9] Review request for JDK-8147521 [macosx] 
> Internal API Usage: setPopupType used to force creation of heavyweight 
> popup
>
> I use only heavy weight.
>
>
>> On May 10, 2016, at 12:46 PM, Alexandr Scherbatiy 
>>  wrote:
>>
>>
>> Do you need to use medium-weight popups in your application or 
>> light/heavy-weight popups are enough?
>>
>> Thanks,
>> Alexandr.
>>
>> On 5/10/2016 10:23 PM, Alan Snyder wrote:
>>>> On May 10, 2016, at 5:58 AM, Sergey Bylokhov  
>>>> wrote:
>>>>
>>>> Hi, Alan.
>>>> Can you please take a look to the proposed solutions? Thanks.
>>>>
>>>>
>>> Approach 2 matches what I currently do. The problem noted by Rajeev does 
>>> not happen because my popup factory calls setPopupType() on each call to 
>>> the public getPopup() before invoking the superclass method.
>>>
>>> I think the original version of Approach 1 would work. My factory would 
>>> override the public getPopup() method and pass true to the five parameter 
>>> method.
>>>
>>> I think the revised version of Approach 1 does not work for me because the 
>>> new flag is only tested if the first attempt to create a popup fails.
>>>

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello All,

Please review the updated webrev.
Bug : https://bugs.openjdk.java.net/browse/JDK-8147521 

Webrev: http://cr.openjdk.java.net/~rchamyal/8147521/webrev.03/ 

Update: Added a new protected method getpop in PopupFactory.java with a Boolean 
parameter for specifying heavy weight popup.

Regards,
Rajeev Chamyal

-Original Message-
From: Alan Snyder [mailto:javali...@cbfiddle.com] 
Sent: 11 May 2016 03:39
To: Alexandr Scherbatiy
Cc: swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

I use only heavy weight.


> On May 10, 2016, at 12:46 PM, Alexandr Scherbatiy 
>  wrote:
> 
> 
> Do you need to use medium-weight popups in your application or 
> light/heavy-weight popups are enough?
> 
> Thanks,
> Alexandr.
> 
> On 5/10/2016 10:23 PM, Alan Snyder wrote:
>>> On May 10, 2016, at 5:58 AM, Sergey Bylokhov  
>>> wrote:
>>> 
>>> Hi, Alan.
>>> Can you please take a look to the proposed solutions? Thanks.
>>> 
>>> 
>> Approach 2 matches what I currently do. The problem noted by Rajeev does not 
>> happen because my popup factory calls setPopupType() on each call to the 
>> public getPopup() before invoking the superclass method.
>> 
>> I think the original version of Approach 1 would work. My factory would 
>> override the public getPopup() method and pass true to the five parameter 
>> method.
>> 
>> I think the revised version of Approach 1 does not work for me because the 
>> new flag is only tested if the first attempt to create a popup fails.
>> 
> 

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

[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-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello All,

Please let me know your thoughts on the below webrev.
http://cr.openjdk.java.net/~rchamyal/8147521/webrev.02/

This approach is similar to Aqua as suggested by Sergey.

Regards,
Rajeev Chamyal

-Original Message-
From: Alexandr Scherbatiy 
Sent: 11 May 2016 01:16
To: Alan Snyder; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup


Do you need to use medium-weight popups in your application or 
light/heavy-weight popups are enough?

Thanks,
Alexandr.

On 5/10/2016 10:23 PM, Alan Snyder wrote:
>> On May 10, 2016, at 5:58 AM, Sergey Bylokhov  
>> wrote:
>>
>> Hi, Alan.
>> Can you please take a look to the proposed solutions? Thanks.
>>
>>
> Approach 2 matches what I currently do. The problem noted by Rajeev does not 
> happen because my popup factory calls setPopupType() on each call to the 
> public getPopup() before invoking the superclass method.
>
> I think the original version of Approach 1 would work. My factory would 
> override the public getPopup() method and pass true to the five parameter 
> method.
>
> I think the revised version of Approach 1 does not work for me because the 
> new flag is only tested if the first attempt to create a popup fails.
>

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

I noticed following problem with the current webrev.
http://cr.openjdk.java.net/~rchamyal/8147521/webrev.01/

If we don't override the BasicPopupMenuUI :: getPopup method and use the below 
code for displaying popup.

JPopupMenu jpopup = new JPopupMenu();
PopupFactory popupFactory = PopupFactory.getSharedInstance();

//sets popup type to be heavy weight
popupFactory.setHeavyWeightPopupEnabled(true);
jpopup.show(frame, e.getX(), e.getY());

The JPopupMenu ::showPopup() method resets the popup type to lightweight. 
Unless we set popupMenu.setLightWeightPopupEnabled(false);

Regards,
Rajeev Chamyal

-Original Message-
From: Alexandr Scherbatiy 
Sent: 10 May 2016 20:06
To: Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

On 5/10/2016 3:05 PM, Sergey Bylokhov wrote:

> Since both of these methods are public I am not sure how it will solve 
> the initial requested problem: enable HW popup w/o possibility of 
> change it by the user's code?

I believe that a user can do something like:
--
JPopupMenu popupMenu = new JPopupMenu();
popupMenu.setLightWeightPopupEnabled(true);
popupMenu.showPopup();
   -
  and it overrides the popup factory type.
  Probably the JPopupMenu.showPopup() should be updated to create a popup with 
necessary type without rewriting the type from popup factory.

   Thanks,
   Alexandr.

>
> On 10.05.16 14:49, Alexandr Scherbatiy wrote:
>> What should be results of the
>> PopupFactory.setHeavyWeightPopupEnabled(false) call in case if the 
>> current popup factory type is heavy-weight? Is it correct to leave it 
>> as heavy weight or it should be changed to another type?
>>
>>   It is better to use {@code } tag instead of  in a javadoc 
>> description.
>>   The test extends JPanel and is created on main thread. It is better 
>> to move its creation to EDT to avoid possible race conditions between 
>> using the JPanel on both main and EDT threads.
>>
>>   Thanks,
>>   Alexandr.
>>>
>>>
>>>
>>> Regards,
>>>
>>> Rajeev Chamyal
>>>
>>>
>>>
>>> *From:*Alexandr Scherbatiy
>>> *Sent:* 10 May 2016 16:07
>>> *To:* Rajeev Chamyal; Sergey Bylokhov; swing-dev@openjdk.java.net
>>> *Subject:* Re:  [9] Review request for JDK-8147521 
>>> [macosx] Internal API Usage: setPopupType used to force creation of 
>>> heavyweight popup
>>>
>>>
>>>
>>> On 5/10/2016 1:10 PM, Rajeev Chamyal wrote:
>>>
>>> Hello Alexandr,
>>>
>>>
>>>
>>> Thanks for the review.
>>>
>>> Please review the updated webrev.
>>>
>>> <http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.00/>http://cr
>>> .openjdk.java.net/~rchamyal/8147521/webrev.00/
>>>
>>>
>>>
>>> Updates: Added a boolean field heavyWeightPopupEnabled and
>>> corresponding property methods.
>>>
>>> Applications can set this property to true for forcing popup to be
>>> heavyweight.
>>>
>>>Is it possible to implement setHeavyWeightPopupEnabled() method 
>>> body as setPopupType(HEAVY_WEIGHT_POPUP) and the
>>> isHeavyWeightPopupEnabled() as checking that current popup factory 
>>> type is HEAVY_WEIGHT_POPUP?
>>>
>>>   Thanks,
>>>   Alexandr.
>>>
>>>
>>>
>>> I am not sure about applications using MEDIUM_WEIGHT_POPUP, but in
>>> JDK source ToolTipManager.java /showTipWindow/ method sets popup
>>> to be MEDIUM_WEIGHT_POPUP.
>>>
>>>
>>>
>>> Regards,
>>>
>>> Rajeev Chamyal
>>>
>>>
>>>
>>>
>>>
>>> *From:*Alexandr Scherbatiy
>>> *Sent:* 10 May 2016 12:02
>>> *To:* Rajeev Chamyal; Sergey Bylokhov; 
>>> <mailto:swing-dev@openjdk.java.net>swing-dev@openjdk.java.net
>>> *Subject:* Re:  [9] Review request for JDK-8147521
>>> [macosx] Internal API Usage: setPopupType used to force creation
>>> of heavyweight popup
>>>
>>>
>>>
>>>
>>> The first approach implies that a user should change all
>>> PopupFactory.getPopup(owner, contents, x, y) calls to
>>> OverridenPopupFactory.getPopup(owner, contents, x, y, true) to get
>>> a heavy-weight popup in his code.
>>>
>>> The second on

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello Alexandr,

 

Please review the updated webrev.

 

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.01/

 

Update : Implemented review comments.

 

Regards,

Rajeev Chamyal

 

From: Alexandr Scherbatiy 
Sent: 10 May 2016 16:07
To: Rajeev Chamyal; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

 

On 5/10/2016 1:10 PM, Rajeev Chamyal wrote:

Hello Alexandr,

 

Thanks for the review.

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.00/

 

Updates: Added a boolean field heavyWeightPopupEnabled and corresponding 
property methods.

Applications can set this property to true for forcing popup to be heavyweight.

   Is it possible to implement setHeavyWeightPopupEnabled() method body as 
setPopupType(HEAVY_WEIGHT_POPUP) and the isHeavyWeightPopupEnabled() as 
checking that current popup factory type is HEAVY_WEIGHT_POPUP?

  Thanks,
  Alexandr.



 

I am not sure about applications using MEDIUM_WEIGHT_POPUP, but in JDK source 
ToolTipManager.java showTipWindow method sets popup to be MEDIUM_WEIGHT_POPUP.

 

Regards,

Rajeev Chamyal

 

 

From: Alexandr Scherbatiy 
Sent: 10 May 2016 12:02
To: Rajeev Chamyal; Sergey Bylokhov; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

 


The first approach implies that a user should change all 
PopupFactory.getPopup(owner, contents, x, y) calls to  
OverridenPopupFactory.getPopup(owner, contents, x, y, true) to get a 
heavy-weight popup in his code.

The second one looks better to me. It may have sense to restrict it only to 2 
possibilities: heavy-weight and light-weight popup if the medium-weight popup 
is not really required to be used by users applications.
Something like: setHeavyWeightPopupEnabled(boolean)/isHeavyWeightPopupEnabled().

Thanks,
Alexandr.

On 5/6/2016 2:44 PM, Rajeev Chamyal wrote:

Hello All,

 

Please review the below 2 webrevs.

 

Webrev:

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.00/"http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.00/

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.01/"http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.01/

 

Bug : https://bugs.openjdk.java.net/browse/JDK-8147521

 

Approach 1:

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.00/"http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.00/

 

A new protected API is provided in PopupFactory.java.

protected Popup getPopup(Component owner, Component contents, int x, int y,

boolean isHeavyWeightPopup)

 

Applications can override the new protected method and pass true value to 
isHeavyWeightPopup for forcing popup to be heavy weight.

Passing false will result in default behaviour.

 

Approach 2:

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.01/"http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.01/

 

In this approach access level of existing methods setPopupType and getPopupType 
has been changed to protected.

Applications can override these methods to set or return different types of 
popups.

 

Following  values can be passed to setPopupType.

0 : LIGHT_WEIGHT_POPUP

1 : MEDIUM_WEIGHT_POPUP

2: HEAVY_WEIGHT_POPUP

 

 

Regards,

Rajeev Chamyal

 

 

 

 

 

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello Alexandr,

 

Thanks for the review.

Please review the updated webrev.

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.00/

 

Updates: Added a boolean field heavyWeightPopupEnabled and corresponding 
property methods.

Applications can set this property to true for forcing popup to be heavyweight.

 

I am not sure about applications using MEDIUM_WEIGHT_POPUP, but in JDK source 
ToolTipManager.java showTipWindow method sets popup to be MEDIUM_WEIGHT_POPUP.

 

Regards,

Rajeev Chamyal

 

 

From: Alexandr Scherbatiy 
Sent: 10 May 2016 12:02
To: Rajeev Chamyal; Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8147521 [macosx] Internal 
API Usage: setPopupType used to force creation of heavyweight popup

 


The first approach implies that a user should change all 
PopupFactory.getPopup(owner, contents, x, y) calls to  
OverridenPopupFactory.getPopup(owner, contents, x, y, true) to get a 
heavy-weight popup in his code.

The second one looks better to me. It may have sense to restrict it only to 2 
possibilities: heavy-weight and light-weight popup if the medium-weight popup 
is not really required to be used by users applications.
Something like: setHeavyWeightPopupEnabled(boolean)/isHeavyWeightPopupEnabled().

Thanks,
Alexandr.

On 5/6/2016 2:44 PM, Rajeev Chamyal wrote:

Hello All,

 

Please review the below 2 webrevs.

 

Webrev:

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.00/"http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.00/

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.01/"http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.01/

 

Bug : https://bugs.openjdk.java.net/browse/JDK-8147521

 

Approach 1:

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.00/"http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.00/

 

A new protected API is provided in PopupFactory.java.

protected Popup getPopup(Component owner, Component contents, int x, int y,

boolean isHeavyWeightPopup)

 

Applications can override the new protected method and pass true value to 
isHeavyWeightPopup for forcing popup to be heavy weight.

Passing false will result in default behaviour.

 

Approach 2:

HYPERLINK 
"http://cr.openjdk.java.net/%7Erchamyal/8147521/webrev.app.01/"http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.01/

 

In this approach access level of existing methods setPopupType and getPopupType 
has been changed to protected.

Applications can override these methods to set or return different types of 
popups.

 

Following  values can be passed to setPopupType.

0 : LIGHT_WEIGHT_POPUP

1 : MEDIUM_WEIGHT_POPUP

2: HEAVY_WEIGHT_POPUP

 

 

Regards,

Rajeev Chamyal

 

 

 

 

Re: [9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello All,

 

 

Please review the updated webrev.

 

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.00/

 

Update: Added test case.

 

 

Regards,

Rajeev Chamyal

 

From: Rajeev Chamyal 
Sent: 06 May 2016 17:14
To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject:  [9] Review request for JDK-8147521 [macosx] Internal API 
Usage: setPopupType used to force creation of heavyweight popup

 

Hello All,

 

Please review the below 2 webrevs.

 

Webrev:

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.00/

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.01/

 

Bug : https://bugs.openjdk.java.net/browse/JDK-8147521

 

Approach 1:

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.00/

 

A new protected API is provided in PopupFactory.java.

protected Popup getPopup(Component owner, Component contents, int x, int y,

boolean isHeavyWeightPopup)

 

Applications can override the new protected method and pass true value to 
isHeavyWeightPopup for forcing popup to be heavy weight.

Passing false will result in default behaviour.

 

Approach 2:

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.01/

 

In this approach access level of existing methods setPopupType and getPopupType 
has been changed to protected.

Applications can override these methods to set or return different types of 
popups.

 

Following  values can be passed to setPopupType.

0 : LIGHT_WEIGHT_POPUP

1 : MEDIUM_WEIGHT_POPUP

2: HEAVY_WEIGHT_POPUP

 

 

Regards,

Rajeev Chamyal

 

 

 

[9] Review request for JDK-8147521 [macosx] Internal API Usage: setPopupType used to force creation of heavyweight popup

[Rajeev Chamyal]

Hello All,

 

Please review the below 2 webrevs.

 

Webrev:

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.00/

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.01/

 

Bug : https://bugs.openjdk.java.net/browse/JDK-8147521

 

Approach 1:

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.00/

 

A new protected API is provided in PopupFactory.java.

protected Popup getPopup(Component owner, Component contents, int x, int y,

boolean isHeavyWeightPopup)

 

Applications can override the new protected method and pass true value to 
isHeavyWeightPopup for forcing popup to be heavy weight.

Passing false will result in default behaviour.

 

Approach 2:

http://cr.openjdk.java.net/~rchamyal/8147521/webrev.app.01/

 

In this approach access level of existing methods setPopupType and getPopupType 
has been changed to protected.

Applications can override these methods to set or return different types of 
popups.

 

Following  values can be passed to setPopupType.

0 : LIGHT_WEIGHT_POPUP

1 : MEDIUM_WEIGHT_POPUP

2: HEAVY_WEIGHT_POPUP

 

 

Regards,

Rajeev Chamyal

 

 

 

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

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

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

[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

Re: [9] Review request for JDK-8153282 [TEST_BUG] some new JInternalFrame tests fail

[Rajeev Chamyal]

Hello Sergey,

Checked the tests with 1.9.0-ea-b86 build.
All tests failed.

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 02 May 2016 19:05
To: Rajeev Chamyal; swing-dev@openjdk.java.net; Avik Niyogi
Subject: Re:  [9] Review request for JDK-8153282 [TEST_BUG] some new 
JInternalFrame tests fail

Looks fine, please confirm that the tests still fails before corresponding bugs 
were fixed.

On 02.05.16 13:19, Rajeev Chamyal wrote:
> Hello All,
>
>
>
> Please review the following fix.
>
>
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8153282
>
> Webrev: http://cr.openjdk.java.net/~rchamyal/8153282/webrev.00/
>
>
>
> Fix: Added delay for test TestJInternalFrameMaximize.java and 
> TestJInternalFrameMinimize.java
>
> Increased the time value for TestJInternalFrameDispose.java
>
>
>
> Test are running fine on Mac,Windows and linux after the fix.
>
>
>
> Regards,
>
> Rajeev Chamyal
>
>
>


--
Best regards, Sergey.

[9] Review request for JDK-8153282 [TEST_BUG] some new JInternalFrame tests fail

[Rajeev Chamyal]

Hello All,

 

Please review the following fix.

 

Bug: https://bugs.openjdk.java.net/browse/JDK-8153282

Webrev: http://cr.openjdk.java.net/~rchamyal/8153282/webrev.00/

 

Fix: Added delay for test TestJInternalFrameMaximize.java and 
TestJInternalFrameMinimize.java

Increased the time value for TestJInternalFrameDispose.java

 

Test are running fine on Mac,Windows and linux after the fix.

 

Regards,

Rajeev Chamyal

 

Re: Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs

[Rajeev Chamyal]

Looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 24 March 2016 12:54
To: Rajeev Chamyal; Alexander Scherbatiy; Sergey Bylokhov; 
swing-dev@openjdk.java.net
Subject: Re:  Review Request of 8137169 : [macosx] Incorrect minimal 
heigh of JTabbedPane with more tabs

 

Hi All,

Please review code changes as per inputs received. 

 

http://cr.openjdk.java.net/~aniyogi/8137169/webrev.03

 

As SCROLL_TAB_LAYOUT is the default layout for Aqua LAF, with implementation 
within the derived AquaTruncatedTabbedPaneLayout, extending of 
TabbedPaneScrollLayout is not needed and management of row and column height is 
done within it itself. Hence preferredTabAreaWidth and preferredTabAreaHeight 
need not manage the number of columns and rows respectively and will remain 1 
as tabs can be brought forth to the UI by the arrow buttons AQUA provides 
instead of placing them in another row or column. This is also the expected 
behaviour for AquaLAF as per javadoc.

 

With Regards,

Avik Niyogi

 

On 24-Mar-2016, at 12:34 pm, Rajeev Chamyal mailto:rajeev.cham...@oracle.com"rajeev.cham...@oracle.com> wrote:

 

Hello Avik,

 

x variable on line 2195 is not used anywhere. Do we need for loop also?

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 24 March 2016 12:19
To: Alexander Scherbatiy
Cc: Sergey Bylokhov; Rajeev Chamyal; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re:  Review Request of 8137169 : [macosx] Incorrect minimal 
heigh of JTabbedPane with more tabs

 

Hi All,

 

Please review my code changes below as per the inputs received.

 

http://cr.openjdk.java.net/~aniyogi/8137169/webrev.02/

 

As SCROLL_TAB_LAYOUT is the default layout for Aqua LAF, with implementation 
within the derived AquaTruncatedTabbedPaneLayout, extending of 
TabbedPaneScrollLayout is not needed and management of row and column height is 
done within it itself. Hence preferredTabAreaWidth and preferredTabAreaHeight 
need not manage the number of columns and rows respectively and will remain 1 
as tabs can be brought forth to the UI by the arrow buttons AQUA provides 
instead of placing them in another row or column. This is also the expected 
behaviour for AquaLAF as per javadoc.

 

With Regards,

Avik Niyogi

 

 

On 23-Mar-2016, at 4:14 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

On 23/03/16 14:07, Avik Niyogi wrote:




 

On 23-Mar-2016, at 3:31 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

On 21/03/16 09:19, Avik Niyogi wrote:




Hi Alexander, 

I agree with what you said regarding the look and feel looking different. But 
this bug arrises due to setting of TabbedPaneScrollLayout only. If Scroll 
Layout is not meant for Aqua look and feel should not the setting of this 
parameter instead throw a helpful error saying this parameter is not accepted

  According to the JTabbedPane.setTabLayoutPolicy(int tabLayoutPolicy) javadoc: 
 
  "Some look and feels might only support a subset of the possible layout 
policies, in which case the value of this property may be ignored."
  
  Aqua L&F ignores WRAP_TAB_LAYOUT for JTabbedPane tabs layouting and always 
use SCROLL_TAB_LAYOUT. No exception should be thrown in this case.

 

Actually, it is doing the other way around for Aqua L&F. It is defaulting 
WRAP_TAB_LAYOUT and setting SCROLL_TAB_LAYOUT is still setting a subclass of 
TabbedPaneLayout and not TabbedPaneScrollLayout.

   According to the JTabbedPane javadoc:
  SCROLL_TAB_LAYOUT: Tab layout policy for providing a subset of available tabs 
when all the tabs will not fit within a single run.
  WRAP_TAB_LAYOUT The tab layout policy for wrapping tabs in multiple runs when 
all tabs will not fit within a single run.
 
   The Aqua L&F uses only AquaTabbedPaneUI and AquaTabbedPaneContrastUI for 
tabbed pane UI and they both returns AquaTruncatingTabbedPaneLayout which has 
been designed for to place tabs according to the SCROLL_TAB_LAYOUT.

  Thanks,
  Alexandr.









  Thanks,
  Alexandr.





instead of absorbing this parameter and letting it render itself into a dummy 
node which does not proceed further with this parameter? Maybe we need to 
discuss what the expected behaviour may be. Also, thank you for the inputs 
regarding how to proceed with removing duplicate code.

 

With Regards,

Avik Niyogi

On 19-Mar-2016, at 1:52 am, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 


I would think about something like:
-
public class TabbedPaneLayout implements LayoutManager {

protected int basePreferredTabAreaWidth(final int tabPlacement, final 
int height) {
// TabbedPaneLayout preferredTabAreaWidth implementation
}

protected int truncatingPreferredTabAreaWidth(final int

Re: Review Request for 6439354 : Win L&F: TitledBorder colors are not from desktop

[Rajeev Chamyal]

Looks good to me.

Regards,
Rajeev Chamyal

-Original Message-
From: Semyon Sadetsky 
Sent: 24 March 2016 11:45
To: Prem Balakrishnan; Sergey Bylokhov; Rajeev Chamyal; Alexander Scherbatiy; 
swing-dev@openjdk.java.net
Subject: Re: Review Request for 6439354 : Win L&F: TitledBorder colors are not 
from desktop

Looks good.

--Semyon

On 3/22/2016 11:56 AM, Prem Balakrishnan wrote:
> Hi Sergey,
>
> Updated test as per the review comments.
>
> Webrev: http://cr.openjdk.java.net/~arapte/prem/6439354/webrev.01/
>
> Regards,
> Prem
>
>
> -Original Message-
> From: Sergey Bylokhov
> Sent: Monday, March 21, 2016 7:31 PM
> To: Prem Balakrishnan; Semyon Sadetsky; Rajeev Chamyal; Alexander 
> Scherbatiy; swing-dev@openjdk.java.net
> Subject: Re: Review Request for 6439354 : Win L&F: TitledBorder colors 
> are not from desktop
>
> Hi, Prem.
> In the test the Swing components accessed on non-EDT thread. Probably the 
> test can be simplified further?
>
> On 21.03.16 11:20, Prem Balakrishnan wrote:
>> Hi*,*
>>
>> Please review fix for JDK 9,
>>
>> *Bug: *https://bugs.openjdk.java.net/browse/JDK-6439354 **
>>
>> *Webrev: *http://cr.openjdk.java.net/~arapte/prem/6439354/webrev.00/
>>
>> *Issue:*
>>
>> Win L&F: TitledBorder colors are not from desktop
>>
>> *Cause:*
>>
>> "TitledBorder.border" is set to EtchedBorder with default 
>> highlight/shadow colors.
>>
>> *Fix:*
>>
>> "TitledBorder.border" is set to  EtchedBorder with Desktop
>>highlight/shadow colors.
>>
>> **
>>
>> *Test: *Manual Test (since windows theme to be set)**
>>
>> **
>>
>> Regards,
>> Prem
>>
>
> --
> Best regards, Sergey.

Re: Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs

[Rajeev Chamyal]

Hello Avik,

 

x variable on line 2195 is not used anywhere. Do we need for loop also?

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 24 March 2016 12:19
To: Alexander Scherbatiy
Cc: Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re:  Review Request of 8137169 : [macosx] Incorrect minimal 
heigh of JTabbedPane with more tabs

 

Hi All,

 

Please review my code changes below as per the inputs received.

 

http://cr.openjdk.java.net/~aniyogi/8137169/webrev.02/

 

As SCROLL_TAB_LAYOUT is the default layout for Aqua LAF, with implementation 
within the derived AquaTruncatedTabbedPaneLayout, extending of 
TabbedPaneScrollLayout is not needed and management of row and column height is 
done within it itself. Hence preferredTabAreaWidth and preferredTabAreaHeight 
need not manage the number of columns and rows respectively and will remain 1 
as tabs can be brought forth to the UI by the arrow buttons AQUA provides 
instead of placing them in another row or column. This is also the expected 
behaviour for AquaLAF as per javadoc.

 

With Regards,

Avik Niyogi

 

 

On 23-Mar-2016, at 4:14 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

On 23/03/16 14:07, Avik Niyogi wrote:



 

On 23-Mar-2016, at 3:31 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 

On 21/03/16 09:19, Avik Niyogi wrote:



Hi Alexander, 

I agree with what you said regarding the look and feel looking different. But 
this bug arrises due to setting of TabbedPaneScrollLayout only. If Scroll 
Layout is not meant for Aqua look and feel should not the setting of this 
parameter instead throw a helpful error saying this parameter is not accepted

  According to the JTabbedPane.setTabLayoutPolicy(int tabLayoutPolicy) javadoc: 
 
  "Some look and feels might only support a subset of the possible layout 
policies, in which case the value of this property may be ignored."
  
  Aqua L&F ignores WRAP_TAB_LAYOUT for JTabbedPane tabs layouting and always 
use SCROLL_TAB_LAYOUT. No exception should be thrown in this case.

 

Actually, it is doing the other way around for Aqua L&F. It is defaulting 
WRAP_TAB_LAYOUT and setting SCROLL_TAB_LAYOUT is still setting a subclass of 
TabbedPaneLayout and not TabbedPaneScrollLayout.

   According to the JTabbedPane javadoc:
  SCROLL_TAB_LAYOUT: Tab layout policy for providing a subset of available tabs 
when all the tabs will not fit within a single run.
  WRAP_TAB_LAYOUT The tab layout policy for wrapping tabs in multiple runs when 
all tabs will not fit within a single run.
 
   The Aqua L&F uses only AquaTabbedPaneUI and AquaTabbedPaneContrastUI for 
tabbed pane UI and they both returns AquaTruncatingTabbedPaneLayout which has 
been designed for to place tabs according to the SCROLL_TAB_LAYOUT.

  Thanks,
  Alexandr.







  Thanks,
  Alexandr.




instead of absorbing this parameter and letting it render itself into a dummy 
node which does not proceed further with this parameter? Maybe we need to 
discuss what the expected behaviour may be. Also, thank you for the inputs 
regarding how to proceed with removing duplicate code.

 

With Regards,

Avik Niyogi

On 19-Mar-2016, at 1:52 am, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 


I would think about something like:
-
public class TabbedPaneLayout implements LayoutManager {

protected int basePreferredTabAreaWidth(final int tabPlacement, final 
int height) {
// TabbedPaneLayout preferredTabAreaWidth implementation
}

protected int truncatingPreferredTabAreaWidth(final int tabPlacement, 
final int height) {
if (tabPlacement == SwingConstants.LEFT || tabPlacement == 
SwingConstants.RIGHT) {
return basePreferredTabAreaWidth(tabPlacement, height);
}

return basePreferredTabAreaWidth(tabPlacement, height);
}

protected int preferredTabAreaWidth(final int tabPlacement, final int 
height) {
return basePreferredTabAreaWidth(tabPlacement, height);
}

}

class TabbedPaneScrollLayout extends TabbedPaneLayout {
@Override
protected int basePreferredTabAreaWidth(int tabPlacement, int height) {
// TabbedPaneScrollLayout preferredTabAreaWidth implementation
}
}

protected class AquaTruncatingTabbedPaneLayout extends 
AquaTabbedPaneCopyFromBasicUI.TabbedPaneLayout {

@Override
protected int preferredTabAreaWidth(final int tabPlacement, final int 
height) {
return truncatingPreferredTabAreaWidth(tabPlacement, height);
}
}

protected class AquaTruncatingTabbedScrollPaneLayout extends 
AquaTabbedPaneCopyFromBasicUI.TabbedPaneScrollLayout {

@Override
protected int preferredTabAreaW

Re: [9] Review request for JDK-8150225 api/javax_swing/text/AbstractWriter/index_indent failed

[Rajeev Chamyal]

Hello Sergey,

I had found below link about pre tag which states A P tag is strictly not 
permitted inside PRE, but if a browser encounters one, it should treat it as 
two newlines.
http://www.htmlhelp.com/reference/wilbur/block/pre.html 

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 22 March 2016 21:58
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8150225 
api/javax_swing/text/AbstractWriter/index_indent failed

Looks fine to me. But I am not an expert here. And I wonder why the  tag is 
so specific, can we get the similar issue if some other tags will be used 
instead?

On 22.03.16 11:35, Rajeev Chamyal wrote:
> Hello All,
>
> Gentle reminder.
> Please review the fix.
>
> Bug : https://bugs.openjdk.java.net/browse/JDK-8150225
> Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/
>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Rajeev Chamyal
> Sent: 09 March 2016 15:58
> To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net
> Subject: Re:  [9] Review request for JDK-8150225 
> api/javax_swing/text/AbstractWriter/index_indent failed
>
> Hello Sergey,
>
> I have run JCK tests for HTMLWriter and AbstractWriter with this fix and all 
> passed.
>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Sergey Bylokhov
> Sent: 09 March 2016 15:54
> To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
> Subject: Re:  [9] Review request for JDK-8150225 
> api/javax_swing/text/AbstractWriter/index_indent failed
>
> Hi, Rajeev.
> Please confirm that there are no new issues in the jck after this fix.
>
> On 09.03.16 12:18, Rajeev Chamyal wrote:
>> Hello All,
>>
>> Please review the following fix for Jdk9:
>>
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8150225
>>
>> Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/
>> <http://cr.openjdk.java.net/~rchamyal/8146276/webrev.00/>
>>
>> Issue : JCK conformance test failed due to fix for bug JDK-7104635
>>
>> Fix: Reverted the fix for JDK-7104635 and added a new method in 
>> HTMLWriter.java to check if P tag is within Pre tag.
>>
>> Decrement indentation is skipped if P tag is with a Pre tag.
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>
>
> --
> Best regards, Sergey.
>


--
Best regards, Sergey.

Re: Review request for JDK-8075084 JOptionPane.showMessageDialog causes JScrollBar to move

[Rajeev Chamyal]

Hello All,

Please review the re-worked fix.
Bug: https://bugs.openjdk.java.net/browse/JDK-8075084
Webrev : http://cr.openjdk.java.net/~rchamyal/8075084/webrev.03/

In the updated fix a global awt event listener has been added to 
BasicScrollBarUI to take actions on mouse events.
The awt event listener determines the state of arrow buttons and source of 
mouse events and based on these it stops the timer.

Regards,
Rajeev Chamyal


 -Original Message-
From: Alexander Scherbatiy 
Sent: 13 January 2016 21:48
To: Rajeev Chamyal
Cc: Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: Re:  Review request for JDK-8075084 
JOptionPane.showMessageDialog causes JScrollBar to move

On 1/12/2016 5:28 PM, Rajeev Chamyal wrote:
> Hello All,
>
> Gentle reminder to review the fix.
> http://cr.openjdk.java.net/~rchamyal/8075084/webrev.02/

My impression was that the scroll timer is started before an adjustment 
listener is executed. In this case the timer can track the open modal dialog 
and be stopped.
It looks like the real situation is opposite and the scroll timer should 
track the closed modal dialog which is not reliable because it is possible to 
press and hold  a scroll thumb on another scrollbar and it will detect the 
closed modal dialog too.

If I am correct for the first version of the fix the mouse release and exit 
events can be still missed if a modal dialog is shown outside the scroll bar.

I do not have a good idea how the mouse exit event can be caught in this 
case when a modal dialog is shown.
May be it possible to add a counter of mouse release events to the toolkit 
(or read it by AWTEventListener).
If number of mouse release events are different before the scroll bar  
adjustment listener is executed and after that it means that mouse was released 
on a modal dialog or missed by some other reason.

   Thanks,
   Alexandr.

>
>
> Regards,
> Rajeev Chamyal
>
> -Original Message-
> From: Rajeev Chamyal
> Sent: 27 December 2015 20:32
> To: Sergey Bylokhov
> Cc: swing-dev@openjdk.java.net
> Subject: Re:  Review request for JDK-8075084 
> JOptionPane.showMessageDialog causes JScrollBar to move
>
> Hello Sergey,
>
> The first webrev version is implemented on similar lines as you suggested but 
> it had issues as pointed my Alexandr in his review below.
> If the mouse pointer after clicking on modal dialog close lies outside the 
> parent window then parent window is not getting any mouse events and 
> BasicScrollBarUI::scrollByUnit method is again getting called recursively, 
> Because of which the scroll bar pointer keeps on moving till the end of 
> scrollbar.
> With this new implementation these issue are not seen.
>
> Regards,
> Rajeev Chamyal
>
>
> -----Original Message-
> From: Sergey Bylokhov
> Sent: 25 December 2015 21:52
> To: Rajeev Chamyal; Alexander Scherbatiy
> Cc: Prasanta Sadhukhan; swing-dev@openjdk.java.net
> Subject: Re: Review request for JDK-8075084 
> JOptionPane.showMessageDialog causes JScrollBar to move
>
> Probably this bug can be fixed in a different way. Is it possible to check 
> the state of the scroll bar in the timer? And if in some iteration the button 
> became unpressed then stops itself(timer).
>
> On 23/12/15 12:29, Rajeev Chamyal wrote:
>> Hello Alexandr,
>>
>> The modal dialog can be application modal, document modal and toolkit modal.
>>1) Application-modal dialog box blocks all windows from the same 
>> application, except windows from its child hierarchy
>>2) Document-modal dialog box blocks all windows from the same 
>> document, except windows from its child hierarchy.
>>3) Toolkit-modal dialog box blocks all windows that run in the 
>> same toolkit, except windows from its child hierarchy
>>
>> The current issue is reproducible with all modal dialog types. I have 
>> updated the condition in code to check for modal dialogs.
>>
>> http://cr.openjdk.java.net/~rchamyal/8075084/webrev.02/
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -Original Message-
>> From: Alexander Scherbatiy
>> Sent: 22 December 2015 05:13
>> To: Rajeev Chamyal
>> Cc: Sergey Bylokhov; Prasanta Sadhukhan; swing-dev@openjdk.java.net
>> Subject: Re: Review request for JDK-8075084 
>> JOptionPane.showMessageDialog causes JScrollBar to move
>>
>> On 21/12/15 12:21, Rajeev Chamyal wrote:
>>> Hello Alexandr,
>>>
>>> I have updated the fix. Please review it.
>>> http://cr.openjdk.java.net/~rchamyal/8075084/webrev.01/
>>   When a modal dialog is shown does it block all windows or is it 
>> possible that a modal dialog blocks some windows and does not block others?
>>
>> 

Re: [9] Review request for JDK-8150225 api/javax_swing/text/AbstractWriter/index_indent failed

[Rajeev Chamyal]

Hello All,

Gentle reminder.
Please review the fix.

Bug : https://bugs.openjdk.java.net/browse/JDK-8150225 
Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/ 

Regards,
Rajeev Chamyal

-Original Message-
From: Rajeev Chamyal 
Sent: 09 March 2016 15:58
To: Sergey Bylokhov; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8150225 
api/javax_swing/text/AbstractWriter/index_indent failed

Hello Sergey,

I have run JCK tests for HTMLWriter and AbstractWriter with this fix and all 
passed.

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov
Sent: 09 March 2016 15:54
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8150225 
api/javax_swing/text/AbstractWriter/index_indent failed

Hi, Rajeev.
Please confirm that there are no new issues in the jck after this fix.

On 09.03.16 12:18, Rajeev Chamyal wrote:
> Hello All,
>
> Please review the following fix for Jdk9:
>
> Bug : https://bugs.openjdk.java.net/browse/JDK-8150225
>
> Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/
> <http://cr.openjdk.java.net/~rchamyal/8146276/webrev.00/>
>
> Issue : JCK conformance test failed due to fix for bug JDK-7104635
>
> Fix: Reverted the fix for JDK-7104635 and added a new method in 
> HTMLWriter.java to check if P tag is within Pre tag.
>
> Decrement indentation is skipped if P tag is with a Pre tag.
>
> Regards,
>
> Rajeev Chamyal
>


--
Best regards, Sergey.

Re: Review Request of 8148555: [macosx] An uncaught exception was raised entering Emoji into JTextArea

[Rajeev Chamyal]

Test code looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 21 March 2016 14:02
To: Manajit Halder; Alexander Scherbatiy
Cc: swing-dev@openjdk.java.net; Rajeev Chamyal
Subject: Re:  Review Request of 8148555: [macosx] An uncaught 
exception was raised entering Emoji into JTextArea

 

Hi All,

Please review the below code changes as per the inputs received.

http://cr.openjdk.java.net/~aniyogi/8148555/webrev.01/

 

 

With Regards,

Avik Niyogi

On 21-Mar-2016, at 12:45 pm, Rajeev Chamyal mailto:rajeev.cham...@oracle.com"rajeev.cham...@oracle.com> wrote:

 

Hello Avik,

 

I can’t comment on objective C code.

As far as test is concerned below are my comments.

 

1)  UI should be created in Swing thread.

2)  Switch case in  actionPerformed should be refactored.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 21 March 2016 12:20
To: Sergey Bylokhov
Cc: HYPERLINK "mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; 
Alexander Scherbatiy; Rajeev Chamyal
Subject: Re:  Review Request of 8148555: [macosx] An uncaught 
exception was raised entering Emoji into JTextArea

 

Hi Rajeev,

Please review the following code changes.

 

With Regards,

Avik Niyogi

On 21-Mar-2016, at 12:17 pm, Avik Niyogi mailto:avik.niy...@oracle.com"avik.niy...@oracle.com> wrote:

 

Hi Sergey,

 

Please review the following code changes.

With Regards,

Avik Niyogi

On 17-Mar-2016, at 7:03 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 


The fix looks good to me.

Just a small note: it is better to remove comment "527 //" since it 
does not have a description.

Thanks,
Alexandr.

On 17/03/16 17:21, Avik Niyogi wrote:

It can be made into a class method, but herein this case it is needed for that 
instance only and hence the need for instance method and referred with “self”. 

 

With Regards,

Avik Niyogi

On 16-Mar-2016, at 11:55 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 


Could the -(NSMutableString *) parseString: method be declared as class method 
instead of instance?

Thanks,
Alexandr.

On 14/03/16 17:18, Sergey Bylokhov wrote:

Hi, Avik. 
Can you please take a look to these two tests before fixing this bug: 

TEST: javax/swing/JMenuItem/8139169/ScreenMenuBarInputTwice.java 
-- 
TEST: 
javax/swing/JMenuItem/ActionListenerCalledTwice/ActionListenerCalledTwiceTest.java

I remember they passed on jdk8, but it seems we have a regression in jdk9 and 
both of them fail. 

On 14.03.16 8:05, Avik Niyogi wrote: 




Hi All, 
A gentle reminder, please review my code changes. 

With Regards, 
Avik Niyogi 




On 08-Mar-2016, at 9:39 pm, Avik Niyogi mailto:avik.niy...@oracle.com"avik.niy...@oracle.com 
HYPERLINK "mailto:avik.niy...@oracle.com";<mailto:avik.niy...@oracle.com>> 
wrote: 

Hi All, 

Kindly review the bug fix for JDK 9. 

*Bug:* 

_https://bugs.openjdk.java.net/browse/JDK-8148555_ 
_ 
_ 
*Webrev:* 

_HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8148555/webrev.00/_"http://cr.openjdk.java.net/~aniyogi/8148555/webrev.00/_
 

*Issue:* 
Emoji selection in Character Viewer was causing exception in JNI 

*Cause:* 
Emojis are considered to be of different class type (namely, 
NSConcreteMutableAttributedString) from NSString which other 
characters are because of a surrogate pair for them. 

*Fix:* 
Major changes done for condition of emojis in JNI. Albeit rendering is 
not yet supported, they will appear as blank “Missing font” notation. 
Also, added debug point in case of issue with glyph arrises. 

With Regards, 
Avik Niyogi 

 

Re: Review Request of 8148555: [macosx] An uncaught exception was raised entering Emoji into JTextArea

[Rajeev Chamyal]

Hello Avik,

 

I can’t comment on objective C code.

As far as test is concerned below are my comments.

 

1)  UI should be created in Swing thread.

2)  Switch case in  actionPerformed should be refactored.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 21 March 2016 12:20
To: Sergey Bylokhov
Cc: swing-dev@openjdk.java.net; Alexander Scherbatiy; Rajeev Chamyal
Subject: Re:  Review Request of 8148555: [macosx] An uncaught 
exception was raised entering Emoji into JTextArea

 

Hi Rajeev,

Please review the following code changes.

 

With Regards,

Avik Niyogi

On 21-Mar-2016, at 12:17 pm, Avik Niyogi mailto:avik.niy...@oracle.com"avik.niy...@oracle.com> wrote:

 

Hi Sergey,

 

Please review the following code changes.

With Regards,

Avik Niyogi

On 17-Mar-2016, at 7:03 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 


The fix looks good to me.

Just a small note: it is better to remove comment "527 //" since it 
does not have a description.

Thanks,
Alexandr.

On 17/03/16 17:21, Avik Niyogi wrote:

It can be made into a class method, but herein this case it is needed for that 
instance only and hence the need for instance method and referred with “self”. 

 

With Regards,

Avik Niyogi

On 16-Mar-2016, at 11:55 pm, Alexander Scherbatiy mailto:alexandr.scherba...@oracle.com"alexandr.scherba...@oracle.com> wrote:

 


Could the -(NSMutableString *) parseString: method be declared as class method 
instead of instance?

Thanks,
Alexandr.

On 14/03/16 17:18, Sergey Bylokhov wrote:

Hi, Avik. 
Can you please take a look to these two tests before fixing this bug: 

TEST: javax/swing/JMenuItem/8139169/ScreenMenuBarInputTwice.java 
-- 
TEST: 
javax/swing/JMenuItem/ActionListenerCalledTwice/ActionListenerCalledTwiceTest.java

I remember they passed on jdk8, but it seems we have a regression in jdk9 and 
both of them fail. 

On 14.03.16 8:05, Avik Niyogi wrote: 



Hi All, 
A gentle reminder, please review my code changes. 

With Regards, 
Avik Niyogi 



On 08-Mar-2016, at 9:39 pm, Avik Niyogi mailto:avik.niy...@oracle.com"avik.niy...@oracle.com 
HYPERLINK "mailto:avik.niy...@oracle.com";<mailto:avik.niy...@oracle.com>> 
wrote: 

Hi All, 

Kindly review the bug fix for JDK 9. 

*Bug:* 

_https://bugs.openjdk.java.net/browse/JDK-8148555_ 
_ 
_ 
*Webrev:* 

_HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8148555/webrev.00/_"http://cr.openjdk.java.net/~aniyogi/8148555/webrev.00/_
 

*Issue:* 
Emoji selection in Character Viewer was causing exception in JNI 

*Cause:* 
Emojis are considered to be of different class type (namely, 
NSConcreteMutableAttributedString) from NSString which other 
characters are because of a surrogate pair for them. 

*Fix:* 
Major changes done for condition of emojis in JNI. Albeit rendering is 
not yet supported, they will appear as blank “Missing font” notation. 
Also, added debug point in case of issue with glyph arrises. 

With Regards, 
Avik Niyogi 

 

 

 

 

 

 

 

Re: Review request for JDK-8145896 JInternalFrame setMaximum before adding to desktop throws null pointer exception

[Rajeev Chamyal]

Hello Sergey,

I have updated the test as per review comments.
http://cr.openjdk.java.net/~rchamyal/8145896/webrev.02/

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 09 March 2016 18:57
To: Rajeev Chamyal; swing-dev@openjdk.java.net
Subject: Re:  Review request for JDK-8145896 JInternalFrame 
setMaximum before adding to desktop throws null pointer exception

Hi, Rajeev.
The fix looks fine, but the test should be updated. JFrame is created on 
non-EDT thread. It is unclear what l&f we should test(some 
specific,all,default?).
This also can be simplified:
70 if (testFailed) {
   71 dispose();
   72 throw new RuntimeException("Test Failed");
   73 }
   74 dispose();

On 09.03.16 7:44, Rajeev Chamyal wrote:
> Hello Sergey,
>
> Could you please review the updated webrev.
> http://cr.openjdk.java.net/~rchamyal/8145896/webrev.01/
>
> Regards,
> Rajeev Chamyal
>
>
> On 11-01-2016 15:27, Rajeev Chamyal wrote:
>> Hello Sergey,
>>
>> Could you please review the updated webrev.
>> http://cr.openjdk.java.net/~rchamyal/8145896/webrev.01/
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -Original Message-
>> From: Rajeev Chamyal
>> Sent: 02 January 2016 11:46
>> To: Sergey Bylokhov; swing-dev@openjdk.java.net
>> Subject: RE: Review request for JDK-8145896 JInternalFrame setMaximum 
>> before adding to desktop throws null pointer exception
>>
>> Hello Sergey,
>>
>> Thanks for review I have updated webrev.
>> There was one more issue with fix to fix it , I have updated 
>> BasicInternalFrameUI.java and added it to webrev.
>> http://cr.openjdk.java.net/~rchamyal/8145896/webrev.01/
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -Original Message-
>> From: Sergey Bylokhov
>> Sent: 30 December 2015 20:34
>> To: Rajeev Chamyal; Prasanta Sadhukhan; swing-dev@openjdk.java.net
>> Subject: Re: Review request for JDK-8145896 JInternalFrame setMaximum 
>> before adding to desktop throws null pointer exception
>>
>> Hi, Rajeev.
>> A few notes:
>> - The "Rectangle desktopBounds = f.getParent().getBounds();" can 
>> reuse the new "c" variable.
>> - Is the "JDesktopPane d = f.getDesktopPane();" is necessary? It 
>> seems that it is not used after the null check.
>>
>> On 30/12/15 10:30, Rajeev Chamyal wrote:
>>> Hello All,
>>>
>>> I need one more review for this webrev.
>>>
>>> http://cr.openjdk.java.net/~rchamyal/8145896/webrev.00/
>>> <http://cr.openjdk.java.net/%7Erchamyal/8145896/webrev.00/>
>>>
>>> Regards,
>>>
>>> Rajeev Chamyal
>>>
>>> *From:*Alexander Scherbatiy
>>> *Sent:* 23 December 2015 19:44
>>> *To:* Rajeev Chamyal
>>> *Cc:* Sergey Bylokhov; Prasanta Sadhukhan; 
>>> swing-dev@openjdk.java.net
>>> *Subject:* Re: Review request for JDK-8145896 JInternalFrame 
>>> setMaximum before adding to desktop throws null pointer exception
>>>
>>>
>>> The fix looks good to me.
>>>
>>> Thanks,
>>> Alexandr.
>>>
>>> On 12/21/2015 3:09 PM, Rajeev Chamyal wrote:
>>>
>>>  Hello All,
>>>
>>>  Please review the following fix for Jdk9:
>>>
>>>  Bug: https://bugs.openjdk.java.net/browse/JDK-8145896
>>>
>>>  Webrev:http://cr.openjdk.java.net/~rchamyal/8145896/webrev.00/
>>>  <http://cr.openjdk.java.net/%7Erchamyal/8145896/webrev.00/>
>>>
>>>  Issue: JInternalFrame setMaximum before adding to desktop throws
>>>  null pointer exception
>>>
>>>  Cause: Null checks for parent and Desktop pane are missing
>>>
>>>  Fix: Added null checks for parent and desktop pane.
>>>
>>>  Verified the fix on windows,Ubuntu and Mac with all supported LAF.
>>>
>>>  Regards,
>>>
>>>  Rajeev Chamyal
>>>
>>
>> --
>> Best regards, Sergey.
>


--
Best regards, Sergey.

Re: [9] Review request for JDK-8150225 api/javax_swing/text/AbstractWriter/index_indent failed

[Rajeev Chamyal]

Hello Sergey,

I have run JCK tests for HTMLWriter and AbstractWriter with this fix and all 
passed.

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov 
Sent: 09 March 2016 15:54
To: Rajeev Chamyal; Alexander Scherbatiy; swing-dev@openjdk.java.net
Subject: Re:  [9] Review request for JDK-8150225 
api/javax_swing/text/AbstractWriter/index_indent failed

Hi, Rajeev.
Please confirm that there are no new issues in the jck after this fix.

On 09.03.16 12:18, Rajeev Chamyal wrote:
> Hello All,
>
> Please review the following fix for Jdk9:
>
> Bug : https://bugs.openjdk.java.net/browse/JDK-8150225
>
> Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/
> <http://cr.openjdk.java.net/~rchamyal/8146276/webrev.00/>
>
> Issue : JCK conformance test failed due to fix for bug JDK-7104635
>
> Fix: Reverted the fix for JDK-7104635 and added a new method in 
> HTMLWriter.java to check if P tag is within Pre tag.
>
> Decrement indentation is skipped if P tag is with a Pre tag.
>
> Regards,
>
> Rajeev Chamyal
>


--
Best regards, Sergey.

[9] Review request for JDK-8150225 api/javax_swing/text/AbstractWriter/index_indent failed

[Rajeev Chamyal]

Hello All,

 

Please review the following fix for Jdk9:

 

Bug : https://bugs.openjdk.java.net/browse/JDK-8150225 

Webrev: http://cr.openjdk.java.net/~rchamyal/8150225/webrev.00/ 
http://cr.openjdk.java.net/~rchamyal/8146276/webrev.00/

 

Issue : JCK conformance test failed due to fix for bug JDK-7104635

 

Fix: Reverted the fix for JDK-7104635 and added a new method in HTMLWriter.java 
to check if P tag is within Pre tag.

Decrement indentation is skipped if P tag is with a Pre tag.

 

Regards,

Rajeev Chamyal

 

Re: Review request for JDK-8145896 JInternalFrame setMaximum before adding to desktop throws null pointer exception

[Rajeev Chamyal]

Hello Sergey,

Could you please review the updated webrev.
http://cr.openjdk.java.net/~rchamyal/8145896/webrev.01/

Regards,
Rajeev Chamyal


On 11-01-2016 15:27, Rajeev Chamyal wrote:

Hello Sergey,

Could you please review the updated webrev.
http://cr.openjdk.java.net/~rchamyal/8145896/webrev.01/

Regards,
Rajeev Chamyal

-Original Message-
From: Rajeev Chamyal
Sent: 02 January 2016 11:46
To: Sergey Bylokhov; swing-dev@openjdk.java.net
Subject: RE: Review request for JDK-8145896 JInternalFrame setMaximum before 
adding to desktop throws null pointer exception

Hello Sergey,

Thanks for review I have updated webrev.
There was one more issue with fix to fix it , I have updated 
BasicInternalFrameUI.java and added it to webrev.
http://cr.openjdk.java.net/~rchamyal/8145896/webrev.01/

Regards,
Rajeev Chamyal

-Original Message-
From: Sergey Bylokhov
Sent: 30 December 2015 20:34
To: Rajeev Chamyal; Prasanta Sadhukhan; swing-dev@openjdk.java.net
Subject: Re: Review request for JDK-8145896 JInternalFrame setMaximum before 
adding to desktop throws null pointer exception

Hi, Rajeev.
A few notes:
- The "Rectangle desktopBounds = f.getParent().getBounds();" can reuse the new 
"c" variable.
- Is the "JDesktopPane d = f.getDesktopPane();" is necessary? It seems that it 
is not used after the null check.

On 30/12/15 10:30, Rajeev Chamyal wrote:

Hello All,

I need one more review for this webrev.

http://cr.openjdk.java.net/~rchamyal/8145896/webrev.00/
<http://cr.openjdk.java.net/%7Erchamyal/8145896/webrev.00/>

Regards,

Rajeev Chamyal

*From:*Alexander Scherbatiy
*Sent:* 23 December 2015 19:44
*To:* Rajeev Chamyal
*Cc:* Sergey Bylokhov; Prasanta Sadhukhan; swing-dev@openjdk.java.net
*Subject:* Re: Review request for JDK-8145896 JInternalFrame
setMaximum before adding to desktop throws null pointer exception


The fix looks good to me.

Thanks,
    Alexandr.

On 12/21/2015 3:09 PM, Rajeev Chamyal wrote:

 Hello All,

 Please review the following fix for Jdk9:

 Bug: https://bugs.openjdk.java.net/browse/JDK-8145896

 Webrev:http://cr.openjdk.java.net/~rchamyal/8145896/webrev.00/
 <http://cr.openjdk.java.net/%7Erchamyal/8145896/webrev.00/>

 Issue: JInternalFrame setMaximum before adding to desktop throws
 null pointer exception

 Cause: Null checks for parent and Desktop pane are missing

 Fix: Added null checks for parent and desktop pane.

 Verified the fix on windows,Ubuntu and Mac with all supported LAF.

 Regards,

 Rajeev Chamyal



--
Best regards, Sergey.

Re: Review Request of 8137169 : [macosx] Incorrect minimal heigh of JTabbedPane with more tabs

[Rajeev Chamyal]

Hello Avik,

 

Fix looks good to me.

Can you please check if test case works on windows and linux.

 

Regards,

Rajeev Chamyal

 

From: Avik Niyogi 
Sent: 29 February 2016 09:46
To: Sergey Bylokhov; Alexander Scherbatiy; Rajeev Chamyal; 
swing-dev@openjdk.java.net
Subject: Re:  Review Request of 8137169 : [macosx] Incorrect minimal 
heigh of JTabbedPane with more tabs

 

Gentle reminder. Please review this fix.

 

On 26-Feb-2016, at 10:39 am, Avik Niyogi mailto:avik.niy...@oracle.com"avik.niy...@oracle.com> wrote:

 

The issue is with setting of TabbedPaneScrollLayout() for the option 
JTabbedPane.SCROLL_TAB_LAYOUT as is enabled in the test code

 and not TabbedPaneLayout() as which is the default.

 

The minimum size fixes itself because the ScrollLayout check fails in 
setTabLayoutPolicy() for the pane. So the issue is with the call to set layout 
manager.

There are only two configurations that the JTabbedPane can exist in of which 
SCROLL_TAB_LAYOUT is one of them.

 

Fixing the minimum size in AquaTabbedPaneUI will fix it for TabbedPaneLayout() 
only which is the WRAP_TAB_LAYOUT.

 

Also, I have checked other implementations such as for Metal and Motif and they 
have similar code for doing this process.

Hence, with in-depth analysis, this fix has no other impact apart from this fix.

 

In case the impact caused by this change has caused some definitive 
regressions, please mention them so they can be addressed. Thank you.

 

With Regards,

Avik Niyogi

 

On 25-Feb-2016, at 6:45 pm, Alexander Potochkin mailto:alexander.potoch...@oracle.com"alexander.potoch...@oracle.com> wrote:

 

Hello Avik

AquaTruncatingTabbedPaneLayout has a lot of code which is specific for the 
AquaTabbedPaneUI.
I don't think setting the layout manager from the base class is the right 
solution here.

If there is a problem with minimum size it should be fixed inside the 
AquaTabbedPaneUI

Thanks
alexp

On 2/24/2016 12:07, Avik Niyogi wrote:

Hi All, 

 

Kindly review the bug fix for JDK 9.

 

Bug:

https://bugs.openjdk.java.net/browse/JDK-8137169

 

Webrev:

 

HYPERLINK 
"http://cr.openjdk.java.net/%7Eaniyogi/8137169/webrev.00/"http://cr.openjdk.java.net/~aniyogi/8137169/webrev.00/

 

Issue:

For Aqua Look&Feel, multiple calls to pane.getMinimumSize().height causes 
incremental return of values.

 

Cause:

The impact was caused by a major broken code within AquaTabbedPaneUI.java for 
createLayoutManager()

 

Fix:

Major linking calls to super class fix done within createLayoutManager().

 

With Regards,

Avik Niyogi

 

 

 

Re: [9] Review fix for JDK-8020039 : SynthTableHeaderUI refers to possibly null parameter in cell renderer

[Rajeev Chamyal]

Looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Ajit Ghaisas 
Sent: 17 February 2016 16:51
To: Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy; 
swing-dev@openjdk.java.net
Subject: RE:  [9] Review fix for JDK-8020039 : SynthTableHeaderUI 
refers to possibly null parameter in cell renderer

 

Hi,

 

I have corrected formatting of test code and removed the additional 
System.out.printlns.

Please review : http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.02/

 

Regards,

Ajit

 

From: Ajit Ghaisas 
Sent: Tuesday, February 16, 2016 3:12 PM
To: Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: Re:   [9] Review fix for JDK-8020039 : 
SynthTableHeaderUI refers to possibly null parameter in cell renderer

 

Hi,

 

Thanks Rajeev for review comments.

 I have checked - windows LAF WindowsTableHeaderUI handles it correctly.

 

 I have added null check for MacOs Aqua.

 Also, I have added a test checking for this null pointer access.



Please review --  
http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.01/ 

 

Regards,

Ajit

 

From: Rajeev Chamyal 
Sent: Monday, February 15, 2016 5:39 PM
To: Ajit Ghaisas; Sergey Bylokhov; Alexander Scherbatiy; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: RE:  [9] Review fix for JDK-8020039 : SynthTableHeaderUI 
refers to possibly null parameter in cell renderer

 

Hello Ajit,

 

Can you please if similar fix is required for other LAF windows ,Aqua etc.

Please add a regression test case also.

 

Regards,

Rajeev Chamyal

 

From: Ajit Ghaisas 
Sent: 15 February 2016 17:30
To: Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy; HYPERLINK 
"mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject:  [9] Review fix for JDK-8020039 : SynthTableHeaderUI refers 
to possibly null parameter in cell renderer

 

Hi,

 

Please review the fix for jdk9,

Bug: https://bugs.openjdk.java.net/browse/JDK-8020039 

Webrev: http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.00/

 

 

Issue :

 

If null is passed as 'table' parameter to  
SynthTableHeaderUI::getTableCellRendererComponent() method in  
src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTableHeaderUI.java, 

there is Null Pointer Exception.

   

 

Analysis :

 

   This method already has a null check for 'table' parameter for second access 
of this parameter in method.

   Whereas the first access of the 'table' parameter lacks this check.

 

 

Fix :

 

   Added null check for the first access of 'table' parameter in 
SynthTableHeaderUI::getTableCellRendererComponent().

   There is no else block added as the flow continues and passes table to base 
class method using super. getTableCellRendererComponent().

   The passed parameter is already checked in base class method correctly. 
Hence, no change is needed in base class.

 

 

Test :

 

   The fix is pretty straight forward. 

   Executed the code snippet given in the bug description. There is no NPE 
after the fix.

 

Regards,

Ajit

Re: [9] Review fix for JDK-8020039 : SynthTableHeaderUI refers to possibly null parameter in cell renderer

[Rajeev Chamyal]

Hello Ajit,

 

Can you please if similar fix is required for other LAF windows ,Aqua etc.

Please add a regression test case also.

 

Regards,

Rajeev Chamyal

 

From: Ajit Ghaisas 
Sent: 15 February 2016 17:30
To: Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy; 
swing-dev@openjdk.java.net
Subject:  [9] Review fix for JDK-8020039 : SynthTableHeaderUI refers 
to possibly null parameter in cell renderer

 

Hi,

 

Please review the fix for jdk9,

Bug: https://bugs.openjdk.java.net/browse/JDK-8020039 

Webrev: http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.00/

 

 

Issue :

 

If null is passed as 'table' parameter to  
SynthTableHeaderUI::getTableCellRendererComponent() method in  
src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTableHeaderUI.java, 

there is Null Pointer Exception.

   

 

Analysis :

 

   This method already has a null check for 'table' parameter for second access 
of this parameter in method.

   Whereas the first access of the 'table' parameter lacks this check.

 

 

Fix :

 

   Added null check for the first access of 'table' parameter in 
SynthTableHeaderUI::getTableCellRendererComponent().

   There is no else block added as the flow continues and passes table to base 
class method using super. getTableCellRendererComponent().

   The passed parameter is already checked in base class method correctly. 
Hence, no change is needed in base class.

 

 

Test :

 

   The fix is pretty straight forward. 

   Executed the code snippet given in the bug description. There is no NPE 
after the fix.

 

Regards,

Ajit

Re: Review Request for 7126823 : JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

[Rajeev Chamyal]

Looks good to me.

 

Regards,

Rajeev Chamyal

 

From: Prem Balakrishnan 
Sent: 15 February 2016 14:22
To: Rajeev Chamyal; Alexander Scherbatiy
Cc: Sergey Bylokhov; Semyon Sadetsky; Ambarish Rapte; swing-dev@openjdk.java.net
Subject: RE:  Review Request for 7126823 : 
JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Hi Rajeev,

Test updated as per your comments.

 

Webrev: http://cr.openjdk.java.net/~arapte/prem/7126823/webrev.02/

 

Regards,
Prem

 

 

From: Rajeev Chamyal 
Sent: Monday, February 15, 2016 12:00 PM
To: Prem Balakrishnan
Cc: Sergey Bylokhov; Semyon Sadetsky; Alexander Scherbatiy; Ambarish Rapte; 
HYPERLINK "mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: RE:  Review Request for 7126823 : 
JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Hello Prem,

 

1)  UI should be created in a swing thread so please update the createUI 
method to use a swing thread.

2)  Also please use SwingUtilities.invokeAndWait instead of 
SwingUtilities.invokeLater.

 

Regards,

Rajeev Chamyal

From: Prem Balakrishnan 
Sent: 11 February 2016 12:49
To: Rajeev Chamyal
Cc: Sergey Bylokhov; Semyon Sadetsky; Alexander Scherbatiy; Ambarish Rapte; 
HYPERLINK "mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: RE:  Review Request for 7126823 : 
JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Hi Rajeev,

 

I have tested for all supported LAF's across all platforms(Windows, Linux and 
Mac)

Test updated for the same.

 

Webrev: http://cr.openjdk.java.net/~arapte/prem/7126823/webrev.01/

 

Regards,

Prem

 

From: Rajeev Chamyal 
Sent: Wednesday, February 10, 2016 2:00 PM
To: Prem Balakrishnan
Cc: Sergey Bylokhov; Semyon Sadetsky; Alexander Scherbatiy; Ambarish Rapte; 
HYPERLINK "mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: RE:  Review Request for 7126823 : 
JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Hello Prem,

 

Did you test this fix for other LAF's as well.

 

Regards,

Rajeev Chamyal

 

From: Prem Balakrishnan 
Sent: 09 February 2016 14:40
To: Sergey Bylokhov; Semyon Sadetsky; Alexander Scherbatiy; Ambarish Rapte; 
HYPERLINK "mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; 
HYPERLINK "mailto:awt-...@openjdk.java.net"awt-...@openjdk.java.net
Subject:  Review Request for 7126823 : 
JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Hi,

Please review fix for JDK9,

Bug: https://bugs.openjdk.java.net/browse/JDK-7126823

Webrev: http://cr.openjdk.java.net/~arapte/prem/7126823/webrev.00/

 

 

Issue:

JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Cause:

Regression: due to 4424247: DefaultDesktopManager does not handle InternalFrame 
state changes as expected.

https://bugs.openjdk.java.net/browse/JDK-4424247 related to 
https://bugs.openjdk.java.net/browse/JDK-4900778 

 

Fix:

DefaultDesktopManager.java

setNormalBounds(getBounds()); call removed from iconifyFrame(JInternalFrame f) 
method.

 

Justification:

This fix is not breaking any of the previous fix i.e., related to 4424247 and 
4900778.
(all previous scenarios are tested and validated with the current fix)

 

In JInternalFrame.java

getNormalBounds() returns getBounds() when normalBounds is NULL, 

Hence no need to setNormaBounds externally.

 

public Rectangle getNormalBounds() {

 

  /* we used to test (!isMaximum) here, but since this

 method is used by the property listener for the

 IS_MAXIMUM_PROPERTY, it ended up getting the wrong

 answer... Since normalBounds get set to null when the

 frame is restored, this should work better */

 

  if (normalBounds != null) {

  return normalBounds;

  } else {

return getBounds();

  }

}

 

 

As per Javadoc: 
public Rectangle getNormalBounds() 
If the JInternalFrame is not in maximized state, returns getBounds(); 
otherwise, returns the bounds that the JInternalFrame would be restored to. 

In maximizeFrame() , normalBounds is set to a value which is not NULL, i.e., 
setNormalBounds(getBounds()); 
In minimizeFrame(), normalBounds is set to NULL, i.e., setNormalBounds(null); 
NormalBounds should NOT be set elsewhere.

 

Test:

Integrated test for current bug and regression.

 

Regards,
Prem

 

Re: Review Request for 7126823 : JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

[Rajeev Chamyal]

Hello Prem,

 

1)  UI should be created in a swing thread so please update the createUI 
method to use a swing thread.

2)  Also please use SwingUtilities.invokeAndWait instead of 
SwingUtilities.invokeLater.

 

Regards,

Rajeev Chamyal

From: Prem Balakrishnan 
Sent: 11 February 2016 12:49
To: Rajeev Chamyal
Cc: Sergey Bylokhov; Semyon Sadetsky; Alexander Scherbatiy; Ambarish Rapte; 
swing-dev@openjdk.java.net
Subject: RE:  Review Request for 7126823 : 
JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Hi Rajeev,

 

I have tested for all supported LAF's across all platforms(Windows, Linux and 
Mac)

Test updated for the same.

 

Webrev: http://cr.openjdk.java.net/~arapte/prem/7126823/webrev.01/

 

Regards,

Prem

 

From: Rajeev Chamyal 
Sent: Wednesday, February 10, 2016 2:00 PM
To: Prem Balakrishnan
Cc: Sergey Bylokhov; Semyon Sadetsky; Alexander Scherbatiy; Ambarish Rapte; 
HYPERLINK "mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net
Subject: RE:  Review Request for 7126823 : 
JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Hello Prem,

 

Did you test this fix for other LAF's as well.

 

Regards,

Rajeev Chamyal

 

From: Prem Balakrishnan 
Sent: 09 February 2016 14:40
To: Sergey Bylokhov; Semyon Sadetsky; Alexander Scherbatiy; Ambarish Rapte; 
HYPERLINK "mailto:swing-dev@openjdk.java.net"swing-dev@openjdk.java.net; 
HYPERLINK "mailto:awt-...@openjdk.java.net"awt-...@openjdk.java.net
Subject:  Review Request for 7126823 : 
JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Hi,

Please review fix for JDK9,

Bug: https://bugs.openjdk.java.net/browse/JDK-7126823

Webrev: http://cr.openjdk.java.net/~arapte/prem/7126823/webrev.00/

 

 

Issue:

JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Cause:

Regression: due to 4424247: DefaultDesktopManager does not handle InternalFrame 
state changes as expected.

https://bugs.openjdk.java.net/browse/JDK-4424247 related to 
https://bugs.openjdk.java.net/browse/JDK-4900778 

 

Fix:

DefaultDesktopManager.java

setNormalBounds(getBounds()); call removed from iconifyFrame(JInternalFrame f) 
method.

 

Justification:

This fix is not breaking any of the previous fix i.e., related to 4424247 and 
4900778.
(all previous scenarios are tested and validated with the current fix)

 

In JInternalFrame.java

getNormalBounds() returns getBounds() when normalBounds is NULL, 

Hence no need to setNormaBounds externally.

 

public Rectangle getNormalBounds() {

 

  /* we used to test (!isMaximum) here, but since this

 method is used by the property listener for the

 IS_MAXIMUM_PROPERTY, it ended up getting the wrong

 answer... Since normalBounds get set to null when the

 frame is restored, this should work better */

 

  if (normalBounds != null) {

  return normalBounds;

  } else {

return getBounds();

  }

}

 

 

As per Javadoc: 
public Rectangle getNormalBounds() 
If the JInternalFrame is not in maximized state, returns getBounds(); 
otherwise, returns the bounds that the JInternalFrame would be restored to. 

In maximizeFrame() , normalBounds is set to a value which is not NULL, i.e., 
setNormalBounds(getBounds()); 
In minimizeFrame(), normalBounds is set to NULL, i.e., setNormalBounds(null); 
NormalBounds should NOT be set elsewhere.

 

Test:

Integrated test for current bug and regression.

 

Regards,
Prem

 

Re: Review Request for 7126823 : JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

[Rajeev Chamyal]

Hello Prem,

 

Did you test this fix for other LAF's as well.

 

Regards,

Rajeev Chamyal

 

From: Prem Balakrishnan 
Sent: 09 February 2016 14:40
To: Sergey Bylokhov; Semyon Sadetsky; Alexander Scherbatiy; Ambarish Rapte; 
swing-dev@openjdk.java.net; awt-...@openjdk.java.net
Subject:  Review Request for 7126823 : 
JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Hi,

Please review fix for JDK9,

Bug: https://bugs.openjdk.java.net/browse/JDK-7126823

Webrev: http://cr.openjdk.java.net/~arapte/prem/7126823/webrev.00/

 

 

Issue:

JInternalFrame.getNormalBounds() returns bad value after iconify/deiconify

 

Cause:

Regression: due to 4424247: DefaultDesktopManager does not handle InternalFrame 
state changes as expected.

https://bugs.openjdk.java.net/browse/JDK-4424247 related to 
https://bugs.openjdk.java.net/browse/JDK-4900778 

 

Fix:

DefaultDesktopManager.java

setNormalBounds(getBounds()); call removed from iconifyFrame(JInternalFrame f) 
method.

 

Justification:

This fix is not breaking any of the previous fix i.e., related to 4424247 and 
4900778.
(all previous scenarios are tested and validated with the current fix)

 

In JInternalFrame.java

getNormalBounds() returns getBounds() when normalBounds is NULL, 

Hence no need to setNormaBounds externally.

 

public Rectangle getNormalBounds() {

 

  /* we used to test (!isMaximum) here, but since this

 method is used by the property listener for the

 IS_MAXIMUM_PROPERTY, it ended up getting the wrong

 answer... Since normalBounds get set to null when the

 frame is restored, this should work better */

 

  if (normalBounds != null) {

  return normalBounds;

  } else {

return getBounds();

  }

}

 

 

As per Javadoc: 
public Rectangle getNormalBounds() 
If the JInternalFrame is not in maximized state, returns getBounds(); 
otherwise, returns the bounds that the JInternalFrame would be restored to. 

In maximizeFrame() , normalBounds is set to a value which is not NULL, i.e., 
setNormalBounds(getBounds()); 
In minimizeFrame(), normalBounds is set to NULL, i.e., setNormalBounds(null); 
NormalBounds should NOT be set elsewhere.

 

Test:

Integrated test for current bug and regression.

 

Regards,
Prem