Re: RFR: JDK-8076106: [macosx] Drag image of TransferHandler does not honor MultiResolutionImage

2015-04-13 Thread Hendrik Schreiber
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

2015-04-13 Thread Sergey Bylokhov

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

2015-04-13 Thread Alexander Scherbatiy


 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

2015-04-13 Thread Anton V. Tarasov

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.