Review Request: JDK-7108290 Mouse pressed event missing when canceling a JPopupMenu

2016-05-25 Thread Prem Balakrishnan 
Hi,

 

Please review the fix for JDK 9

 

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

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

 

 

Cause: Mouse pressed event was consumed by JPopupMenu during cancelation of 
Popup.

Except for Metal and Aqua LAF(Mac), this issue is reproducible in all supported 
Look And Feels. 

 

Fix: Mouse pressed event consumption  is Disabled for JPopupMenu during 
cancelation of Popup.

 

 

Regards,
Prem

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

2016-05-25 Thread Phil Race 

Approved

-phil.

On 05/24/2016 05:27 AM, Rajeev Chamyal wrote:

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: Fix for JDK-6827800 : Default button is activated even when it is invisible

2016-05-25 Thread Sergey Bylokhov 

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.

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

2016-05-25 Thread Ajit Ghaisas 
Hi,

 

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

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

2016-05-25 Thread Avik Niyogi 
Changes updated to same webrev.01
> http://cr.openjdk.java.net/~aniyogi/8144161/webrev.01 
> 
With regards,
Avik Niyogi

> On 25-May-2016, at 2:21 pm, Rajeev Chamyal  wrote:
> 
> 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>> 
> wrote:
>  
> On 24/05/16 15:22, Avik Niyogi wrote:
> Hi All, 
>  
> Kindly review the fix for JDK9.
> Bug: https://bugs.openjdk.java.net/browse/JDK-8144161 
> 
>  
> Webrev:http:// 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: Review request for 8144161: [TESTBUG] [macosx] Test javax/swing/plaf/basic/BasicComboPopup/7072653/bug7072653.java fails for mac

2016-05-25 Thread 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: Review request for 8144161: [TESTBUG] [macosx] Test javax/swing/plaf/basic/BasicComboPopup/7072653/bug7072653.java fails for mac

2016-05-25 Thread Avik Niyogi 
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 
>  wrote:
> 
> On 24/05/16 15:22, Avik Niyogi wrote:
>> Hi All,
>> 
>> Kindly review the fix for JDK9.
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8144161 
>> 
>> 
>> Webrev:http:// 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
>