Re: Review Request for 8055197: TextField deletes multiline strings
Hi Alexander, In single line mode , there is no rich edit control style to handle EOL. Referenced documentation: https://msdn.microsoft.com/en-us/library/windows/desktop/bb787605(v=vs.85).aspx Many Thanks Ambarish -Original Message- From: Alexander Scherbatiy Sent: Friday, October 16, 2015 4:45 PM To: Ambarish Rapte Cc: awt-dev@openjdk.java.net; Sergey Bylokhov Subject: Re: Review Request for 8055197: TextField deletes multiline strings The TextField is based on Rich Edit control on Windows. Does the Rich Edit control have properties to properly handle multiline strings in single-line mode? Thanks, Alexandr. On 10/14/2015 7:44 PM, Ambarish Rapte wrote: > Dear All, > > Please review the patch for jdk9. > Bug: https://bugs.openjdk.java.net/browse/JDK-8055197 > Webrev: > http://cr.openjdk.java.net/~rchamyal/ambarish/8055197/webrev.00/ > > > Issue: > - If text containing new line character is set to TextField, Text after > new line character was terminated. > - Issue occurs only on windows. > > Cause: > - For windows new line character is ā\r\nā. > - For windows code this new line character was not replaced by space > character. > - Other platforms replace the EOL character by space character. > > Fix: > - Added code to TextComponent.java to remove EOL on java side before > passing to peer. >=> Added a private method replaceEOL() , which replaces EOL by space. >=> removeEOL will be called by newly added TextComponent > construcotr and setText() > > - The text variable on in TextComponent.java & on string displayed on > native side will be same. > > > > Many Thanks, > Ambarish
Re: [9] Review Request for 8080395: consider making sun.awt.CausedFocusEvent functionality public
On 15.10.15 9:57, Semyon Sadetsky wrote: - Why did you add a check to the new constructor of FocusEvent? This check should be done already in the EventObject, and executes before your new check? Is it a typo and it should be cause? When why do not throw an NPE? The test should verify this also. Please review the modified fix http://cr.openjdk.java.net/~ssadetsky/8080395/webrev.01/ The new version has the same issue as in previous, removed code had a typo: 252 if (source == null) 253 throw new IllegalArgumentException("null cause"); I assume it should be reverted and updated to: 252 if (cause == null) 253 throw new IllegalArgumentException("null cause"); Since you update the serialVersionUID then the comment is not valid anymore: "JDK 1.1 serialVersionUID". Also I have requested an additional clarification from the core libs team to confirm that we have an ability to change serialVersionUID in the major release. On 11.09.15 0:15, Semyon Sadetsky wrote: Hello, Please review fix for JDK9: bug: https://bugs.openjdk.java.net/browse/JDK-8080395 webrev: http://cr.openjdk.java.net/~ssadetsky/8080395/webrev.00/ This fix moves the caused property to java.awt.event.FocusEvent to make it public. The sun.awt.CausedFocusEvent class is removed as unnecessary. The API change was approved http://ccc.us.oracle.com/8080395. --Semyon -- Best regards, Sergey.
Re: [9] Review Request for 8130895: Test javax/swing/system/6799345/TestShutdown.java fails on Solaris11 Sparcv9
On 18.09.15 18:50, Semyon Sadetsky wrote: Why do you think so? Each app context KFM has own keystrokes if it is initialized from the thread that belongs to its ThreadGroup. Sorry. I see what you mean. That is corrected as well: http://cr.openjdk.java.net/~ssadetsky/8130895/webrev.02/ After the changes in KeyboardFocusManager.java it seems it is unnecessary to add the KfmAccessor holder in KeyboardFocusManagerPeerImpl? --Semyon On 7/23/2015 7:38 AM, Semyon Sadetsky wrote: Hi Sergey, As you said there are 2 issues here. The first is about KeyboardFocusManager initialization in the main event loop. And the second about initialization of the KeyboardManagerManager itself where keystrokes are shared between contexts. Or do you mean if I remove the keystrokes sharing then the KeyboardFocusManager can be initialized in the toolkit thread? Is that what you mean? --Semyon On 7/22/2015 8:20 PM, Sergey Bylokhov wrote: It is unclear why it is unrelated, the stack trace from the bug: java.lang.ExceptionInInitializerError at sun.misc.Unsafe.ensureClassInitialized(Native Method) at sun.awt.AWTAccessor.getKeyboardFocusManagerAccessor(AWTAccessor.java:966) at sun.awt.KeyboardFocusManagerPeerImpl.(KeyboardFocusManagerPeerImpl.java:46) at sun.awt.X11.XToolkit.run(XToolkit.java:611) at sun.awt.X11.XToolkit.run(XToolkit.java:550) at java.lang.Thread.run(Thread.java:745) Caused by: java.lang.NullPointerException at java.awt.AWTKeyStroke.getCachedStroke(AWTKeyStroke.java:255) at java.awt.AWTKeyStroke.getAWTKeyStroke(AWTKeyStroke.java:394) at java.awt.KeyboardFocusManager.(KeyboardFocusManager.java:332) ... 6 more On 22.07.15 17:09, Semyon Sadetsky wrote: Hi Sergey, From the process point of view it's better to fix the issue you've found in another ticket. The failed test is not related to the keystrokes caching. So I suggest to push this fix as it is and file another JIRA for the keystrokes. --Semyon On 7/22/2015 3:58 PM, Sergey Bylokhov wrote: Hi, Semyon. NPE occurs when we initialize KFM on the Toolkit thread, but this is only a part of the bug, another issue is that we will use cached keystrokes on the toolkit thread. But this keystrokes is bound to the appcontext so we should not use objects which connect to the application on the toolkit thread. This code should be carefully checked to remove appcontext related stuff from the toolkit thread. On 21.07.15 12:40, Semyon Sadetsky wrote: Hello, Please review fix for JDK9: bug: https://bugs.openjdk.java.net/browse/JDK-8130895 webrev: http://cr.openjdk.java.net/~ssadetsky/8130895/webrev.00/ realSync() used in the test's TestRunnable class causes events come to the XAWT event loop but there are no any windows created at the moment and the system application context is not initialized. This results in attempt to create the KeyboardFocusManager instance on the XAWT's thread group during the XEvent dispatching. That in its turn causes NPE. The solution: since KeyboardFocusManager should never be instantiated in the toolkit event loop, the corresponding check was added. --Semyon -- Best regards, Sergey. -- Best regards, Sergey. -- Best regards, Sergey. -- Best regards, Sergey.
[9] Review Request for 8139227: Text fields in JPopupMenu structure do not receive focus in hosted Applets
Hello, Please review fix for JDK9: bug: https://bugs.openjdk.java.net/browse/JDK-8139227 webrev: http://cr.openjdk.java.net/~ssadetsky/8139227/webrev.00/ Win32 window owner query returns the browser frame for applet's child window because there are no any other overlapped or popup windows on the path. Since the browser frame is not an AWT window the focus proxy is lost and synthetic focus doesn't work. As a solution the AWT owner window is requested from the AWT parent in case if AWT owner cannot be found using Windows owner. --Semyon
Re: [9] Review request for: 8136763 [macosx] java always returns only one value for "text/uri-list" dataflavor even if several files were copied
Sergey, thank you. Alexander , could you please review the fix too? Thanks, Mikhail. On 10/19/2015 13:51, Sergey Bylokhov wrote: The fix looks fine. please fix the typo "returens" in CDataTransferer before the push. On 19.10.15 10:39, mikhail cherkasov wrote: Hi there, http://cr.openjdk.java.net/~mcherkas/8136763/webrev.02/ During adding the test, I've mistakenly returned flavormap version with removed "text/uri-list". Now "text/uri-list" is in place again. Thanks, Mikhail. On 10/15/2015 11:42, mikhail cherkasov wrote: Hi Sergey, Alexander, Please review the new version of web rev with updated test. Thanks, Mikhail. On 10/7/2015 15:40, mikhail cherkasov wrote: Hi Sergey, Sorry, I've added wrong test, there's a correct version: http://cr.openjdk.java.net/~mcherkas/8136763/webrev.01/ -Mikhail. On 10/6/2015 18:29, Sergey Bylokhov wrote: Looks like the test is not in the final state. On 06.10.15 11:01, mikhail cherkasov wrote: Hi there, Please review a fix: bug: https://bugs.openjdk.java.net/browse/JDK-8136763 webrev: http://cr.openjdk.java.net/~mcherkas/8136763/webrev.00/ The problem is similar to https://bugs.openjdk.java.net/browse/JDK-8081787 When user requests data for uri-list flavor, java returns xml without any interpretation. It is fixed by processing xml by dragQueryFile() method. The other problem: if several files has been copied, there're available two types in pasteboard for reading: NSURLPboardType NSFilenamesPboardType but text/uri-list is associated only with NSURLPboardType - which returns only first value from list of files that was copied. To resolve this I added: "FILE_NAME=text/uri-list;eoln="\r\n";terminators=1" line above "URL=text/uri-list;eoln="\r\n";terminators=1". So now, java will select FILE_NAME and return a full list of copied files. Thanks, Mikhail.
Re: [9] Review request for: 8136763 [macosx] java always returns only one value for "text/uri-list" dataflavor even if several files were copied
The fix looks fine. please fix the typo "returens" in CDataTransferer before the push. On 19.10.15 10:39, mikhail cherkasov wrote: Hi there, http://cr.openjdk.java.net/~mcherkas/8136763/webrev.02/ During adding the test, I've mistakenly returned flavormap version with removed "text/uri-list". Now "text/uri-list" is in place again. Thanks, Mikhail. On 10/15/2015 11:42, mikhail cherkasov wrote: Hi Sergey, Alexander, Please review the new version of web rev with updated test. Thanks, Mikhail. On 10/7/2015 15:40, mikhail cherkasov wrote: Hi Sergey, Sorry, I've added wrong test, there's a correct version: http://cr.openjdk.java.net/~mcherkas/8136763/webrev.01/ -Mikhail. On 10/6/2015 18:29, Sergey Bylokhov wrote: Looks like the test is not in the final state. On 06.10.15 11:01, mikhail cherkasov wrote: Hi there, Please review a fix: bug: https://bugs.openjdk.java.net/browse/JDK-8136763 webrev: http://cr.openjdk.java.net/~mcherkas/8136763/webrev.00/ The problem is similar to https://bugs.openjdk.java.net/browse/JDK-8081787 When user requests data for uri-list flavor, java returns xml without any interpretation. It is fixed by processing xml by dragQueryFile() method. The other problem: if several files has been copied, there're available two types in pasteboard for reading: NSURLPboardType NSFilenamesPboardType but text/uri-list is associated only with NSURLPboardType - which returns only first value from list of files that was copied. To resolve this I added: "FILE_NAME=text/uri-list;eoln="\r\n";terminators=1" line above "URL=text/uri-list;eoln="\r\n";terminators=1". So now, java will select FILE_NAME and return a full list of copied files. Thanks, Mikhail. -- Best regards, Sergey.
Re: [9] Review request for: 8136763 [macosx] java always returns only one value for "text/uri-list" dataflavor even if several files were copied
Hi there, http://cr.openjdk.java.net/~mcherkas/8136763/webrev.02/ During adding the test, I've mistakenly returned flavormap version with removed "text/uri-list". Now "text/uri-list" is in place again. Thanks, Mikhail. On 10/15/2015 11:42, mikhail cherkasov wrote: Hi Sergey, Alexander, Please review the new version of web rev with updated test. Thanks, Mikhail. On 10/7/2015 15:40, mikhail cherkasov wrote: Hi Sergey, Sorry, I've added wrong test, there's a correct version: http://cr.openjdk.java.net/~mcherkas/8136763/webrev.01/ -Mikhail. On 10/6/2015 18:29, Sergey Bylokhov wrote: Looks like the test is not in the final state. On 06.10.15 11:01, mikhail cherkasov wrote: Hi there, Please review a fix: bug: https://bugs.openjdk.java.net/browse/JDK-8136763 webrev: http://cr.openjdk.java.net/~mcherkas/8136763/webrev.00/ The problem is similar to https://bugs.openjdk.java.net/browse/JDK-8081787 When user requests data for uri-list flavor, java returns xml without any interpretation. It is fixed by processing xml by dragQueryFile() method. The other problem: if several files has been copied, there're available two types in pasteboard for reading: NSURLPboardType NSFilenamesPboardType but text/uri-list is associated only with NSURLPboardType - which returns only first value from list of files that was copied. To resolve this I added: "FILE_NAME=text/uri-list;eoln="\r\n";terminators=1" line above "URL=text/uri-list;eoln="\r\n";terminators=1". So now, java will select FILE_NAME and return a full list of copied files. Thanks, Mikhail.