The fix looks good to me.
Thanks,
Alexandr.
On 8/31/2015 2:10 PM, Sergey Bylokhov wrote:
The fix looks fine. Thanks.
Please fix this typo in the test before the push:
25 * @test @bug 8025082
On 31.08.15 9:27, Rajeev Chamyal wrote:
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