Hi Alexey, Thank you for the review. Unfortunately it is impossible to create the automated standalone test for this issue.
Thanks, Dmitry > On 15 Aug 2018, at 19:46, Alexey Ivanov <[email protected]> wrote: > > 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 <[email protected] >>> <mailto:[email protected]>> 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 <[email protected] >>>>> <mailto:[email protected]> <mailto:[email protected] >>>>> <mailto:[email protected]>>> 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/ >>>>>> <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 <[email protected] >>>>>>> <mailto:[email protected]> <mailto:[email protected] >>>>>>> <mailto:[email protected]>><mailto:[email protected] >>>>>>> <mailto:[email protected]>>> 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 >>>>>>>> <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/ >>>>>>>> <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. >> >
