Hello Sergey,

Please review the updated webrev.

http://cr.openjdk.java.net/~psadhukhan/rajeev/8025082/webrev.03/

Regards,
Rajeev Chamyal

-----Original Message-----
From: Sergey Bylokhov 
Sent: Thursday, August 27, 2015 6:18 PM
To: Rajeev Chamyal; Alexander Scherbatiy; Alexander Zvegintsev; 
swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> RFR: [JDK-8025082] The behaviour of the highlight will 
be lost after clicking the set button

Hi, Rajeev.
A few comments about a tests:
  - The (SwingUtilities.isEventDispatchThread()) inside a invokeAndWait is not 
necessary, it is not a problem, but this is an overhead to validate this in 
each test.
  - I suggest to move the window to the center of the
screen(frame.setLocationRelativeTo(null)) and clicks to the center of the 
button, otherwise the test can fail if the window will be opened under the 
dashboard(dock), which can be placed on the left side of the screen.
  - Please always close all resources, which were opened by the test. in your 
case this is a frame, which should be disposed at the end of the test.

Small note about a style, probably this style will be a little bit readable? 
But this is up to you.
     /**
      * Updates ownsSelection based on text selection in the caret.
      */
     private void updateOwnsSelection() {
         ownsSelection = selectionTag != null
                 && SwingUtilities2.canAccessSystemClipboard();
     }



On 27.08.15 13:37, Rajeev Chamyal wrote:
> Hello All,
>
> Please review the updated webrev.
>
> Webrev: http://cr.openjdk.java.net/~psadhukhan/rajeev/8025082/webrev.02/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8025082
>   
>
> Regards,
> Rajeev Chamyal
>
> -----Original Message-----
> From: Alexander Scherbatiy
> Sent: Wednesday, August 26, 2015 6:39 PM
> To: Rajeev Chamyal
> Cc: Alexander Zvegintsev; Sergey Bylokhov; swing-dev@openjdk.java.net
> Subject: Re: <Swing Dev> RFR: [JDK-8025082] The behaviour of the highlight 
> will be lost after clicking the set button
>
> On 8/26/2015 10:20 AM, Rajeev Chamyal wrote:
>> Hello All,
>>
>> Please review the updated fix for bug JDK-8025082.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8025082
>> Webrev : http://cr.openjdk.java.net/~psadhukhan/rajeev/8025082/webrev.01/
>       - the updateOwnsSelection method should be properly formatted
>        See the Java Code Conventions:
> http://www.oracle.com/technetwork/java/codeconventions-150003.pdf
>        and new discussed Java Style Guidelines:
> http://cr.openjdk.java.net/~alundblad/styleguide
>      -  lines which are longer than 80 characters should be split
>
>       Thanks,
>       Alexandr.
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -----Original Message-----
>> From: Alexander Scherbatiy
>> Sent: Tuesday, August 25, 2015 2:28 PM
>> To: Rajeev Chamyal
>> Cc: swing-dev@openjdk.java.net
>> Subject: Re: <Swing Dev> RFR: [JDK-8025082] The behaviour of the highlight 
>> will be lost after clicking the set button
>>
>> On 8/21/2015 3:10 PM, Rajeev Chamyal wrote:
>>> Hi,
>>>
>>> Please review the following fix for jdk9:
>>>     
>>> Bug:https://bugs.openjdk.java.net/browse/JDK-8025082
>>>
>>> webrev:
>>> http://cr.openjdk.java.net/~psadhukhan/rajeev/8025082/webrev.00/
>>> <http://cr.openjdk.java.net/%7Epsadhukhan/rajeev/8025082/webrev.00/>
>>>
>>>     
>>> The highlight of selected text in JTextPane/JTextArea is lost if some other 
>>> component (e.g. button clicked) gains focus.
>>> Added a method to update the selection ownership for caret when text is 
>>> selected/unselected in the JTextPane/JTextArea.
>>       - The updateOwnsSelection() method can be shorter if use the 
>> conditional operator '?'
>> https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.25
>>       - SwingUtilities.invokeAndWait() should be used instead of 
>> invokeLater. In other case the system can suspend the EDT thread and robot 
>> will start its actions before the createUI() execution.
>>       - button.getLocationOnScreen() should be called on EDT in the test
>>      - The actionPerformed() method is not properly formatted
>>      - What is the reason to use double colons in the error messages?
>>       - System.out.println calls are not really necessary for the automated 
>> test. It also pollute the logs for SQE team when all tests are running. The 
>> only exception throwing can be left in the
>> actionPerformed() method.
>>       - line 30  '* **/' - small formatting issue
>>
>>      Thanks,
>>      Alexandr.
>>
>>>     
>>> Regards,
>>> Rajeev Chamyal
>>>     
>>>


-- 
Best regards, Sergey.

Reply via email to