<AWT Dev> [11] RFR 8205479: OS X: requestFocus() does not work properly for embedded frame

[Alexey Ivanov]

Hi Dmitry,

The fix looks good.

Can an automated test be written for this issue?

Regards,
Alexey

On 06/08/2018 15:46, Dmitry Markov wrote:
> Thank you, Sergey!
>
> Looking for the second +1 from someone else.
>
> Thanks,
> Dmitry
>
>> On 5 Aug 2018, at 01:15, Sergey Bylokhov <Sergey.Bylokhov at oracle.com 
>> <mailto:Sergey.Bylokhov at oracle.com>> wrote:
>>
>> Looks fine.
>>
>> On 01/07/2018 02:26, Dmitry Markov wrote:
>>> Hi Sergey,
>>> The changes in CPlatformEmbeddedFrame are intended for handling the 
>>> case when the embedder window contains several embedded frames and 
>>> focus is transferred programmatically between the frames. In 
>>> particular if requestFocus() is invoked for some component located 
>>> at inactive frame it is necessary to activate the frame via 
>>> CPlatformEmbeddedFrame.requestWindowFocus() to make such focus 
>>> transition possible.
>>> I am sorry, I do not have information about whether the situation 
>>> described above is applicable for SWT or not. Anyway it is out of 
>>> scope for this review.
>>> Thanks,
>>> Dmitry
>>>> On 1 Jul 2018, at 00:21, Sergey Bylokhov 
>>>> <Sergey.Bylokhov at oracle.com 
>>>> <mailto:Sergey.Bylokhov at oracle.com><mailto:Sergey.Bylokhov at oracle.com>> 
>>>> wrote:
>>>>
>>>> Hi, Dmitry
>>>> Can you please clarify why the changes in CPlatformEmbeddedFrame 
>>>> are necessary? Why the same code does not exists in 
>>>> CViewPlatformEmbeddedFrame?
>>>>
>>>> On 27/06/2018 10:25, Dmitry Markov wrote:
>>>>> Hi Sergey,
>>>>> You are right, it is better to use synthesizeWindowActivation(). 
>>>>> Please find the updated webrew 
>>>>> here:http://cr.openjdk.java.net/~dmarkov/8205479/webrev.01/ 
>>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.01/><http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.01/>
>>>>> Changes:
>>>>>  - Overrode synthesizeWindowActivation() for CEmbeddedFrame. It 
>>>>> calls handleWindowFocusEvent() which actually activates or 
>>>>> deactivates embedded frame.
>>>>>  - Added updateGlobalFocusedWindow() to CEmbeddedFrame. This 
>>>>> method should be called when the focus is transferred to embedded 
>>>>> frame programmatically since handleWindowFocusEvent() skips 
>>>>> activation for non-focused embedded frames.
>>>>> Thanks,
>>>>> Dmitry
>>>>>> On 27 Jun 2018, at 01:20, Sergey Bylokhov 
>>>>>> <Sergey.Bylokhov at oracle.com 
>>>>>> <mailto:Sergey.Bylokhov at oracle.com><mailto:Sergey.Bylokhov at oracle.com><mailto:Sergey.Bylokhov at oracle.com>> 
>>>>>> wrote:
>>>>>>
>>>>>> Hi, Dmitry.
>>>>>> Can you please confirm that we should not implement 
>>>>>> synthesizeWindowActivation() to achieve this behavior?
>>>>>> I guess we should do the same as in CViewEmbeddedFrame which is 
>>>>>> used by SWT.
>>>>>>
>>>>>> On 25/06/2018 09:11, Dmitry Markov wrote:
>>>>>>> Hello,
>>>>>>> Could you review a fix for jdk11, please?
>>>>>>>  bug:https://bugs.openjdk.java.net/browse/JDK-8205479
>>>>>>>  webrev:http://cr.openjdk.java.net/~dmarkov/8205479/webrev.00/ 
>>>>>>> <http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.00/><http://cr.openjdk.java.net/%7Edmarkov/8205479/webrev.00/>
>>>>>>> Problem description:
>>>>>>> On Mac OSX when focus is transferred to some component located 
>>>>>>> at embedded frame, CPlatformEmbeddedFrame.requestWindowFocus() 
>>>>>>> is called to activate owning frame. However that method does 
>>>>>>> nothing, (i.e. no activation happens). As a result the focus 
>>>>>>> cannot be transferred to the component because its owner is not 
>>>>>>> active.
>>>>>>> Fix:
>>>>>>> CPlatformEmbeddedFrame.requestWindowFocus() should activate the 
>>>>>>> embedded frame, (i.e. invoke notifyActivation() for the 
>>>>>>> corresponding peer).
>>>>>>> Thanks,
>>>>>>> Dmitry
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards, Sergey.
>>>>
>>>>
>>>> --
>>>> Best regards, Sergey.
>>
>>
>> --
>> Best regards, Sergey.
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20180815/176e2585/attachment-0001.html>