Re: AWT Dev Code Review Request for CR 7146237 - closed/java/awt/Focus/SetFocusTraversalKeysTest/SetFocusTraversalTest.html failed since 1.8.0b19
Looks fine.
Thanks,
Artem
On 3/30/2012 8:16 PM, Oleg Pekhovskiy wrote:
Hi Anthony,
thank you for this clarification, I modified those places:
http://cr.openjdk.java.net/~bagiras/8/7146237.2
Thanks,
Oleg.
3/30/2012 2:06 PM, Anthony Petrov wrote:
Hi Oleg,
The modern tagging approach for the javadoc is to use {@code ...}
instead of code.../code.
Otherwise the changes look fine to me.
--
best regards,
Anthony
On 3/29/2012 7:09 PM, Oleg Pekhovskiy wrote:
Hi guys,
please review these java-doc changes for:
http://bugs.sun.com/view_bug.do?bug_id=7146237
webrev:
http://cr.openjdk.java.net/~bagiras/8/7146237.1
Thanks,
Oleg
Re: AWT Dev Swing Dev Request for review: 7155298 : Editable TextArea blocks GUI application from exit
Thank you! -- best regards, Anthony On 03/31/12 13:01, Charles Lee wrote: Hi Sean, The patch has been committed @ Changeset: 96340349e35b Author:zhouyx Date: 2012-03-31 16:55 +0800 URL:http://hg.openjdk.java.net/jdk8/awt/jdk/rev/96340349e35b 7155298: Editable TextArea/TextField are blocking GUI applications from exit Summary: Stop default caret's timer by setVisible(false) when dispose Reviewed-by: anthony, ant Please verify it. Thank you all for reviewing. On 03/27/2012 11:22 AM, Sean Chou wrote: Hi Anthony, I tried the scenario you suggested, but it doesn't work. And I found the jtreg spec says: ' A main action is considered to be finished when the main method returns; if a test involves multiple threads, some synchronization may be necessary to ensure that the other threads finish their work before the thread running the main method returns. ' Then I tried to join TimerQueue in main, but it always blocks. So I started a new process to wait instead. I tested and found the / separated path works on windows, it is not a problem :) On Mon, Mar 26, 2012 at 9:55 PM, Anthony Petrov anthony.pet...@oracle.com mailto:anthony.pet...@oracle.com wrote: Hi Sean, 92 worker = Runtime.getRuntime().exec(System.getProperty(java.home)+/bin/java TestDispose workprocess); This won't work on MS Windows because the path separator character is different there. Actually, I don't understand why you need this Runtime stuff in the first place. If test JVM doesn't terminate, the test will fail. So why not create a frame and a text field right in the main(), then call dispose() and return from main()? Since the timer thread will still be running, the test's JVM won't exit, and the test will fail by timeout eventually. Will this testing scenario work? -- best regards, Anthony On 03/23/12 10:49, Sean Chou wrote: I modified the testcase according to Anthony Petrov's suggestion(http://mail.openjdk.java.net/pipermail/awt-dev/2012-March/002389.html) . The new webrev: http://cr.openjdk.java.net/~zhouyx/7155298/webrev.02/ http://cr.openjdk.java.net/%7Ezhouyx/7155298/webrev.02/ However, the timeout action in jtreg only checks the main method, but the timeout is caused by timer thread . So, I started an other process to run the testcase and the main testcase waitFor that process to stop. In order to kill the process started by the testcase, I added a ShutdownHook to the runtime of main testcase. And added /othervm action to testcase . It seems the testcase is a little over complex, is there any other method to make the testcase simpler ? On Fri, Mar 23, 2012 at 2:04 AM, Oleg Sukhodolsky son@gmail.com mailto:son@gmail.com mailto:son@gmail.com mailto:son@gmail.com wrote: On Thu, Mar 22, 2012 at 10:50 PM, Anton V. Tarasov anton.tara...@oracle.com mailto:anton.tara...@oracle.com mailto:anton.tara...@oracle.com mailto:anton.tara...@oracle.com wrote: On 3/22/12 6:15 PM, Oleg Sukhodolsky wrote: On Thu, Mar 22, 2012 at 5:55 PM, Anton V. Tarasov anton.tara...@oracle.com mailto:anton.tara...@oracle.com mailto:anton.tara...@oracle.com mailto:anton.tara...@oracle.com wrote: On 22.03.2012 14:37, Oleg Sukhodolsky wrote: On Thu, Mar 22, 2012 at 2:19 PM, Anton V. Tarasov anton.tara...@oracle.com mailto:anton.tara...@oracle.com mailto:anton.tara...@oracle.com mailto:anton.tara...@oracle.com wrote: On 22.03.2012 12:47, Oleg Sukhodolsky wrote: On Thu, Mar 22, 2012 at 12:01 PM, Sean Chouzho...@linux.vnet.ibm.com mailto:zho...@linux.vnet.ibm.com mailto:zho...@linux.vnet.ibm.com mailto:zho...@linux.vnet.ibm.com wrote: Hi Oleg, Seem there are misunderstanding . DefaultCaret can receive FocusLostEvent when another control get focused. But it doesn't receive FocusLostEvent when disposing. The reason is XTextAreaPeer doesn't receive FocusLostEvent when disposing. But I don't know if it is a rule that a FocusLostEvent must be sent to the focused component when the top-level window is disposed ? Well, for regular AWT component it is expected. And I'd expect that this should also be true for peer. That's right, focus_lost should be dispatched to a disposed focus owner. So, now we need to figure out why the caret doesn't get the event. Oleg. I
Re: AWT Dev [8] Review request for 7123476: DesktopOpenTests:When enter the file path and click the open button, it crash
Hi Denis, The fix looks good to me. Thanks! -- best regards, Anthony On 04/02/12 16:35, Denis S. Fokin wrote: Hi Anthony, I took you suggestions into account. I have not Please take another look. http://cr.openjdk.java.net/~denis/7123476/webrev.04/ Thank you, Denis. On 3/29/2012 4:48 PM, Anthony Petrov wrote: Hi Denis, It's not that I'm insisting on anything. I'm just looking into the gnome_interface.c code and seeing that we both check the return value for NULL as well as check the dlerror() status. I would like to see some consistency between loading the GTK and Gnome libs in this regard. Also, please take a look at man dlsym, it says: dlsym() The function dlsym() takes a handle of a dynamic library returned by dlopen() and the null-terminated symbol name, returning the address where that symbol is loaded into memory. If the symbol is not found, in the specified library or any of the libraries that were automatically loaded by dlopen() when that library was loaded, dlsym() returns NULL. (The search performed by dlsym() is breadth first through the dependency tree of these libraries.) Since the value of the symbol could actually be NULL (so that a NULL return from dlsym() need not indicate an error), the correct way to test for an error is to call dlerror() to clear any old error conditions, then call dlsym(), and then call dlerror() again, saving its return value into a variable, and check whether this saved value is not NULL. I realize that while a NULL pointer may be a valid return value for dlsym(), it's useless for our purposes and is very unlikely to happen in this case anyway. However, if I read the specification of dlsym() correctly, since we're going to call a function referenced by the return value of dlsym(), we must check it for NULL as well as check the dlerror() status. If any of this indicates an error, we should assume that the init() function has failed, and hence should return FALSE. -- best regards, Anthony On 03/28/12 21:14, Denis S. Fokin wrote: Hi Anthony, thank you for the review notes. Actually, I expect that if fp_gtk_show_uri is null we have some kind of dlerror. Anyway I check fp_gtk_show_uri in Java_sun_awt_X11_XDesktopPeer_gnome_1url_1show. So I would not add additional check here. But if you insist I will add the NULL check. Thank you, Denis. On 3/28/2012 7:01 PM, Anthony Petrov wrote: Hi Denis, src/solaris/native/sun/xawt/gnome_interface.c 2 * Copyright (c) 2005, 2011, Oracle and/or its affiliates. All rights reserved. I think this file has just been created in 2012 :) 30 fprintf(stderr, gnome_load\n); Please remove all the debugging output, or put it under the #ifdef INTERNAL_BUILD. 70 fprintf(stderr, can not find symble gnome_url_show\n); s/symble/symbol/ src/solaris/native/sun/awt/gtk2_interface.c 447 return TRUE; I think it also makes sense to check fp_gtk_show_uri for NULL before returning TRUE here. The rest of the fix looks fine to me. Thank you! -- best regards, Anthony On 3/27/2012 5:03 PM, Denis S. Fokin wrote: Hi Anthony, here is a new version of the fix. http://cr.openjdk.java.net/~denis/7123476/webrev.03/ I took into account you suggestions. Now the implementation loads gtk API if it exists on the library path. If it does not exists we try to load gnome API. If it is not successful we do not support the functionality. I introduced a couple of files to keep gnome interface separately like we do with gtk. I expect that we remove them as soon as all our supported OS configurations will be have installed the proper GTK library by default. As for the synchronization section, I do not see how to use the fp_gdk_threads_* functions with gnome API so I put the critical under gtk-specific if-clause section. Thank you, Denis. On 2/1/2012 9:03 PM, Anthony Petrov wrote: Hi Denis, The gtk_show_uri() is available since GTK 2.14. Did you verify if all platforms supposed to be supported by JDK 8 have this version of GTK libraries installed by default? I'm mostly concerned about Solaris systems, as well as corporate Linux desktops. If this is not the case, then perhaps using this function should be conditional, and with the old GTK library we should fall back to using the old API. You may notice that, for example, for the file chooser we have an explicit check for GTK 2.8.0 and use the new gtk_file_chooser_set_do_overwrite_confirmation() API only when it's available. I like that we move all the GTK-related utility code to the gtk2_interface files. A few comments: 1. Please use the TRUE and FALSE constants instead of 1 and 0 as a return value for gtk2_show_uri_load(). 2. Should the fprintf() call be #ifdef'ed for INTERNAL_BUILD's only? -- best regards, Anthony On 2/1/2012 7:39 PM, Denis S. Fokin wrote: Hi AWT team, Please review a fix for the CR 7123476 at http://cr.openjdk.java.net/~denis/7123476/webrev.01 CR URL: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7123476 The Gnome API is deprecated so we need to
Re: AWT Dev [8] Review request for 7148275: [macosx] setIconImages() not working correctly (distorted icon when minimized)
Hi Mike, Have you had a chance to take a look at the issue? -- best regards, Anthony On 03/27/12 20:49, Anthony Petrov wrote: On 3/27/2012 8:44 PM, Anthony Petrov wrote: Artem: this is a good idea, thanks. Mike: I'm trying to do something like this: http://cr.openjdk.java.net/~anthony/8-15-lowResIcon-7148275.1/ i.e. I'm feeding an NSImage with images of all available sizes. A sample test provides a list of icons 16x16, 32x32, 48x48, and 64x64. I've verified that they all get added into the representations array. However, even if I resize the dock to be very very tiny, it still chooses the 64x64 icon to represent a minimized window. This is actually wrong since the 16x16 icon would look better in this case than a resized 64x64 icon. Any idea how this can be fixed? Another solution would be to take an approach similar to what we use on MS Windows: we query the system icon size, and then choose the best image (see SunToolkit.getScaledIconData()). But I can't find API that Rather WWindowPeer.updateIconImages() which calls the SunToolkit method. -- best regards, Anthony would allow me to determine the current dock icon size on OS X. Is there any? -- best regards, Anthony On 3/26/2012 5:19 PM, Artem Ananiev wrote: It was fine for 7uX, but can we do anything better for JDK8? Is the largest icon always the best? I remember on Windows we use another approach to find what exactly icon from the list to apply. Thanks, Artem On 3/23/2012 8:18 PM, Anthony Petrov wrote: Hello, Please review a fix for http://bugs.sun.com/view_bug.do?bug_id=7148275 at: http://cr.openjdk.java.net/~anthony/8-15-lowResIcon-7148275.0/ This is a direct forward-port of the same fix from 7u4. More details: http://mail.openjdk.java.net/pipermail/macosx-port-dev/2012-March/003531.html -- best regards, Anthony
AWT Dev [8] Request for review: 7154025 [macosx] XAWTDifference case load nothing about the components in standframe except gray.
Hi Everyone, See comments from the bug description: /1. The image in Standard frame is not painted, until the frame is [manually] resized./ Fixed: Observer was added to the drawImage(). / 2. Test instructions should be corrected to have an explicit statement about components layout. Obviously, the motif image doesn't correspond to the test, it's just a set of motif widgets laid out randomly in the window. /Fixed: Image was updated./ /Old image: http://cr.openjdk.java.net/~serb/7154025/old/MotifColors.jpg New image: http://cr.openjdk.java.net/~serb/7154025/new/MotifColors.jpg / 3. Editable/non-editable TextField colors should differ. I'm not sure about this, though, we should first check how native Cocoa widgets behave. /This is correct behavior. Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7154025 Webrev can be found at: http://cr.openjdk.java.net/~serb/7154025/webrev.00/ Webrev against closed part: http://cr.openjdk.java.net/~serb/7154025/closed/webrev/ -- Best regards, Sergey.
AWT Dev [8] Request for review: 7124534 [macosx] Submenu title overlaps with Submenu indicator in JPopupMenu
Hi Everyone, This testcase was targeted to the bug in metal and motif lf. Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7124534 Webrev can be found at: http://cr.openjdk.java.net/~serb/7124534/webrev.00/ Webrev against closed part: http://cr.openjdk.java.net/~serb/7124534/closed/webrev/ -- Best regards, Sergey.
Re: AWT Dev [8] Review request for 7148275: [macosx] setIconImages() not working correctly (distorted icon when minimized)
This seems like a reasonable approach, though setting window icons is generally discouraged on OS X. Regards, Mike Swingler Apple Inc. On Apr 2, 2012, at 7:16 AM, Anthony Petrov wrote: Hi Mike, Have you had a chance to take a look at the issue? -- best regards, Anthony On 03/27/12 20:49, Anthony Petrov wrote: On 3/27/2012 8:44 PM, Anthony Petrov wrote: Artem: this is a good idea, thanks. Mike: I'm trying to do something like this: http://cr.openjdk.java.net/~anthony/8-15-lowResIcon-7148275.1/ i.e. I'm feeding an NSImage with images of all available sizes. A sample test provides a list of icons 16x16, 32x32, 48x48, and 64x64. I've verified that they all get added into the representations array. However, even if I resize the dock to be very very tiny, it still chooses the 64x64 icon to represent a minimized window. This is actually wrong since the 16x16 icon would look better in this case than a resized 64x64 icon. Any idea how this can be fixed? Another solution would be to take an approach similar to what we use on MS Windows: we query the system icon size, and then choose the best image (see SunToolkit.getScaledIconData()). But I can't find API that Rather WWindowPeer.updateIconImages() which calls the SunToolkit method. -- best regards, Anthony would allow me to determine the current dock icon size on OS X. Is there any? -- best regards, Anthony On 3/26/2012 5:19 PM, Artem Ananiev wrote: It was fine for 7uX, but can we do anything better for JDK8? Is the largest icon always the best? I remember on Windows we use another approach to find what exactly icon from the list to apply. Thanks, Artem On 3/23/2012 8:18 PM, Anthony Petrov wrote: Hello, Please review a fix for http://bugs.sun.com/view_bug.do?bug_id=7148275 at: http://cr.openjdk.java.net/~anthony/8-15-lowResIcon-7148275.0/ This is a direct forward-port of the same fix from 7u4. More details: http://mail.openjdk.java.net/pipermail/macosx-port-dev/2012-March/003531.html -- best regards, Anthony
Re: AWT Dev [8] Review request for 7148275: [macosx] setIconImages() not working correctly (distorted icon when minimized)
But this approach doesn't work as one would expect. Please read my quote bellow. In a nutshell we've got 2 options: 1. Pass all images representations in one NSImage to setMiniwindowImage:. This doesn't work - if the dock is small, still the largest image is chosen only. Is there way to fix this? 2. There's no API to request the current dock size, so I can't apply the approach as we do on Windows. What should we do to make it work - either as #1 or #2? -- best regards, Anthony On 4/2/2012 11:19 PM, Mike Swingler wrote: This seems like a reasonable approach, though setting window icons is generally discouraged on OS X. Regards, Mike Swingler Apple Inc. On Apr 2, 2012, at 7:16 AM, Anthony Petrov wrote: Hi Mike, Have you had a chance to take a look at the issue? -- best regards, Anthony On 03/27/12 20:49, Anthony Petrov wrote: On 3/27/2012 8:44 PM, Anthony Petrov wrote: Artem: this is a good idea, thanks. Mike: I'm trying to do something like this: http://cr.openjdk.java.net/~anthony/8-15-lowResIcon-7148275.1/ i.e. I'm feeding an NSImage with images of all available sizes. A sample test provides a list of icons 16x16, 32x32, 48x48, and 64x64. I've verified that they all get added into the representations array. However, even if I resize the dock to be very very tiny, it still chooses the 64x64 icon to represent a minimized window. This is actually wrong since the 16x16 icon would look better in this case than a resized 64x64 icon. Any idea how this can be fixed? Another solution would be to take an approach similar to what we use on MS Windows: we query the system icon size, and then choose the best image (see SunToolkit.getScaledIconData()). But I can't find API that Rather WWindowPeer.updateIconImages() which calls the SunToolkit method. -- best regards, Anthony would allow me to determine the current dock icon size on OS X. Is there any? -- best regards, Anthony On 3/26/2012 5:19 PM, Artem Ananiev wrote: It was fine for 7uX, but can we do anything better for JDK8? Is the largest icon always the best? I remember on Windows we use another approach to find what exactly icon from the list to apply. Thanks, Artem On 3/23/2012 8:18 PM, Anthony Petrov wrote: Hello, Please review a fix for http://bugs.sun.com/view_bug.do?bug_id=7148275 at: http://cr.openjdk.java.net/~anthony/8-15-lowResIcon-7148275.0/ This is a direct forward-port of the same fix from 7u4. More details: http://mail.openjdk.java.net/pipermail/macosx-port-dev/2012-March/003531.html -- best regards, Anthony
