Re: RFR: JDK-8076106: [macosx] Drag image of TransferHandler does not honor MultiResolutionImage
Thanks, Sergey. Alexandr, are you going to submit this? Is this also going to be submitted to Java8 automatically? Cheers, -hendrik > On Apr 13, 2015, at 15:34, Sergey Bylokhov wrote: > > Hi, Hendrik. > The fix looks fine. > > On 11.04.15 13:04, Hendrik Schreiber wrote: >> Do we need a second person to endorse this, before it can be committed to >> the source code repository? >> If so, would someone please be so kind? >> >> Thank you. >> >> -hendrik >> >>> On Apr 2, 2015, at 12:37, Alexander Scherbatiy >>> wrote: >>> >>> >>> The fix looks good to me. >>> >>> Thanks, >>> Alexandr. >>> >>> On 4/2/2015 10:09 AM, Hendrik Schreiber wrote: > On Apr 1, 2015, at 12:47, Hendrik Schreiber wrote: > > >> On Mar 27, 2015, at 16:09, Alexander >> Scherbatiy wrote: >> >> On 3/26/2015 7:46 PM, Hendrik Schreiber wrote: On Mar 26, 2015, at 17:17, Alexander Scherbatiy wrote: Could you add the automated test to the fix? >>> If you have a suggestion on how to test this automatically, please let >>> me know. >>Use Robot in your sample to drag the text and create the screen >> capture. >>You can put the test to the jdk/test/java/awt/datatransfer folder. Thanks, Alexandr, for taking a look. Can someone please review this? Webrev:http://cr.openjdk.java.net/~alexsch/hendrik-schreiber/8076106/webrev.01/ Bug:https://bugs.openjdk.java.net/browse/JDK-8076106 > > > -- > Best regards, Sergey. >
Re: RFR: JDK-8076106: [macosx] Drag image of TransferHandler does not honor MultiResolutionImage
Hi, Hendrik. The fix looks fine. On 11.04.15 13:04, Hendrik Schreiber wrote: Do we need a second person to endorse this, before it can be committed to the source code repository? If so, would someone please be so kind? Thank you. -hendrik On Apr 2, 2015, at 12:37, Alexander Scherbatiy wrote: The fix looks good to me. Thanks, Alexandr. On 4/2/2015 10:09 AM, Hendrik Schreiber wrote: On Apr 1, 2015, at 12:47, Hendrik Schreiber wrote: On Mar 27, 2015, at 16:09, Alexander Scherbatiy wrote: On 3/26/2015 7:46 PM, Hendrik Schreiber wrote: On Mar 26, 2015, at 17:17, Alexander Scherbatiy wrote: Could you add the automated test to the fix? If you have a suggestion on how to test this automatically, please let me know. Use Robot in your sample to drag the text and create the screen capture. You can put the test to the jdk/test/java/awt/datatransfer folder. Thanks, Alexandr, for taking a look. Can someone please review this? Webrev:http://cr.openjdk.java.net/~alexsch/hendrik-schreiber/8076106/webrev.01/ Bug:https://bugs.openjdk.java.net/browse/JDK-8076106 -- Best regards, Sergey.
Re: [9] Review request for 8073453: Focus doesn't move when pressing Shift + Tab keys
The fix looks good to me. Thanks, Alexandr. On 4/10/2015 3:26 PM, Anton V. Tarasov wrote: Looks fine! Thanks, Anton. On 10.04.2015 10:22, dmitry markov wrote: Hi Anton, Thank you for review. Please find new version of the fix here: http://cr.openjdk.java.net/~dmarkov/8073453/jdk9/webrev.01/ Changes: - Modified SortingFocusTraversalPolicy.getLastComponent() - Added regression test for the swing case I ran focus related regression test and did not observe any new failures. Thanks, Dmitry On 09/04/2015 14:10, Anton V. Tarasov wrote: Hi Dmitry, Well, the fix seems correct to me. I tried to thought of any possible regressions but nothing came to my mind (let's suppose this was really a mistake in the code). However, wouldn't you like to do the same for swing's SortingFocusTraversalPolicy? And also, include it into the test scenario? (Hope you've run all the focus related regression tests). Thanks, Anton. On 06.04.2015 10:14, dmitry markov wrote: Hello, Could you review the fix for jdk9, please? bug: https://bugs.openjdk.java.net/browse/JDK-8073453 webrev: http://cr.openjdk.java.net/~dmarkov/8073453/jdk9/webrev.00/ Problem description: The method ContainerOrderFocusTraversalPolicy.getLastComponent() always returns null if the last component is a container with focus traversal policy and does not have any sub-components. In some cases such behaviour of getLastComponent() causes failure during reverse focus transition, (i.e. focus stays on the selected component when SHIFT+TAB is pressed). Fix: If the last component is a container with focus traversal policy and does not have any sub-components, the method getLastComponent() should return a previous component instead of null. Please note: the same approach is already implemented for ContainerOrderFocusTraversalPolicy.getFirstComponent(). Thanks, Dmitry
Re: [9] Review Request: 8074757 Remove java.awt.Toolkit methods which return peer types
Hi Sergey, I'm fine with it. Regards, Anton. On 10.04.2015 18:08, Sergey Bylokhov wrote: Hello, The new version of the fix: - @deprecated tag was removed - the message was changed to "UI components are unsupported by: " + toolkit http://cr.openjdk.java.net/~serb/8074757/webrev.05 On 10.04.15 11:52, Anton V. Tarasov wrote: On 07.04.2015 17:28, Sergey Bylokhov wrote: On 03.04.15 20:14, Phil Race wrote: It does not need to be deprecated. It can be 'undeprecated' It was deprecated only because it was the public Toolkit method that is now gone .. Ok, I'll update it. So perhaps there's just a small adjustment needed in the case of where we use createComponent() ?? It is used in 3 places: - Indirectly in Canvas and Panel where our headless toolkits creates NullComponentPeer instead of the native peer. So the question is this is implementation detail of our headless toolkit or all such toolkits should use the same things. - In Component class I can reuse NullComponentPeer, but it is unclear how we survive this later when external tollkit is in use. If nobody objects then I suggest for now to use this new error as an assertion to find possible usage of these methods, instead of silent usage of some empty stub, and fail sometime later with unclear reason. Hi Sergey, I don't object throwing AWTError. However, if we still claim to support external toolkits at least in headless env, then doesn't it make sense to clarify the error message? +throw new AWTError("Unsupported toolkit: " + toolkit); to something like "Unsupported headful toolkit" or "Only headless custom toolkits are supported"? Thanks, Anton. -phil. -phil. On 04/02/2015 08:15 AM, Sergey Bylokhov wrote: Hello. Please review the fix for jdk 9. There are a number of public methods in the java.awt.Toolkit class, which reference the unsupported java.awt.dnd.peer and java.awt.peer interfaces. There is a decision to remove these references as described: http://mail.openjdk.java.net/pipermail/awt-dev/2015-February/008924.html Changes description: - All such methods were moved from Toolkit.java to the ComponentFactory.java. Note that all our toolkits implement ComponentFactory interface. - HToolkit, HeadlessToolkit, SunToolkit were cleared because they have the same implementation of these methods as in ComponentFactory. - The questionable moment is that I throw an AWTError in a some places if a default toolkit not implements ComponentFactory interface. Bug: https://bugs.openjdk.java.net/browse/JDK-8074757 Webrev can be found at: http://cr.openjdk.java.net/~serb/8074757/webrev.04 -- Best regards, Sergey. -- Best regards, Sergey. -- Best regards, Sergey.