Hi Manajit, Please find my answer inline.
Thanks, Dmitry > On 5 Nov 2018, at 17:29, Manajit Halder <manajit.hal...@oracle.com> wrote: > > Hi Dmitry, > > Thanks for you reply. Please find my reply inline. > > Regards, > Manajit > > On 01/11/18 4:59 PM, Dmitry Markov wrote: >> Hi Manajit, >> >> The usage of addChildWindow will introduce some undesirable behaviour. For >> example, parent and child windows are created on the same screen; on the >> build with your fix if the parent window is moved the child will be moved >> too. > As per my understanding that is the expected behaviour. Moving the parent > window will move the child window as the child window is attached to the > parent window. But moving the child window won't move the parent window. That behaviour is specified for addChildWindow in Cocoa spec but it is incorrect for AWT/Swing. If the current fix is integrated we will get different behaviour on OS X and other platforms. I think we shouldn’t use addChildWindow due to high risk of introducing some unwanted behaviour like this. >> It is better to not use addChildWindow without a good reasons. >> > addChildWindow need to used in this case because if we use orderWindow an > extra event is generated and the reported issue is reproduced, i.e. the > JPopupMenu is shown on the modeless Dialog (explained below in my first > proposed fix). That’s right. The problem is caused by incorrect handling of mouse events for some cases. In my opinion it is necessary to revise event handling logic for OS X and sort out what goes wrong. > >> Will your fix work if the parent with popup menu and the child are created >> on different screens? > I have tried my fix with a Frame with PopupMenu and a Dialog on different > screens, and it works. > >> Thanks, >> Dmitry >> >>> On 31 Oct 2018, at 16:41, Manajit Halder <manajit.hal...@oracle.com >>> <mailto:manajit.hal...@oracle.com>> wrote: >>> >>> Changed the targeted review version to 12. >>> >>> Regards, >>> Manajit >>> On 31/10/18 10:09 PM, Manajit Halder wrote: >>>> Hi Dmitry/Sergey, >>>> >>>> I have a different fix for this issue in Mac OS specific code. In this >>>> proposed fix I am trying to fix the window addition/ordering with respect >>>> to the screen on which the window is created. As per my debugging this >>>> issue was introduced while fixing JDK-8080729. The problem was introduced >>>> due to the use of orderWindow by replacing it with addChildWindow. The >>>> difference between these two Mac OS methods: >>>> >>>> addChildWindow: After the childWin is added as a child of the window, it >>>> is maintained in relative position indicated by place for subsequent >>>> ordering operations involving either window. While this attachment is >>>> active, moving childWin will not cause the window to move (as in sliding a >>>> drawer in or out), but moving the window will cause childWin to move. >>>> https://developer.apple.com/documentation/appkit/nswindow/1419152-addchildwindow?language=objc >>>> >>>> <https://developer.apple.com/documentation/appkit/nswindow/1419152-addchildwindow?language=objc> >>>> orderWindow: Repositions the window’s window device in the window server’s >>>> screen list. >>>> https://developer.apple.com/documentation/appkit/nswindow/1419672-orderwindow?language=occ >>>> >>>> <https://developer.apple.com/documentation/appkit/nswindow/1419672-orderwindow?language=occ> >>>> The main difference which was causing this issue is: addChildWindow >>>> attaches the child window to its parent, but orderWindow re positions it. >>>> >>>> Proposed fix: >>>> In my proposed fix I am trying to add/order the window based on which >>>> screen it is created. If the windows (parent and child) are created on >>>> same screen then addChildWindow is used to order child window on top of >>>> parent window. And orderWindow is used for the other case when windows are >>>> created on different screens. >>>> >>>> Please review the fix: >>>> http://cr.openjdk.java.net/~mhalder/8189253/webrev.01/ >>>> <http://cr.openjdk.java.net/%7Emhalder/8189253/webrev.01/> >>>> Regards, >>>> Manajit >>>> On 03/05/18 12:06 AM, Dmitry Markov wrote: >>>>> Hi Manajit, >>>>> >>>>> If I got it right, the problem is OSX specific and you suggest fixing it >>>>> in common code. I am sorry but that does not look like as a good approach >>>>> unless it has reasonable explanation. >>>>> I recommend that you should move the fix to more suitable place, (i.e. >>>>> OSX specific code). For example, LWWindowPeer.notifyMouseEvent() where we >>>>> receive mouse events from the platform and generate ours mouse enter/exit >>>>> events if necessary. I guess it is possible to implement something >>>>> similar to your current fix in LWWindowPeer. >>>>> >>>>> Thanks, >>>>> Dmitry >>>>> >>>>>> On 28 Apr 2018, at 00:54, Sergey Bylokhov <sergey.bylok...@oracle.com> >>>>>> <mailto:sergey.bylok...@oracle.com> wrote: >>>>>> >>>>>> Hi, Manajit. >>>>>> Please clarify why this bug is occurred after JDK-8080729, how=20 >>>>>> orderWindow/addChildWindow are related to the mouse events which we >>>>>> get?=20 >>>>>> Did we start to get more event or less events or we get events in the=20 >>>>>> different order, what is the difference? >>>>>> >>>>>> On 04/01/2018 10:22, Manajit Halder wrote: >>>>>>> Hi Semyon, >>>>>>> =20 >>>>>>> Fix for issue JDK-8080729=20 >>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8080729> >>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8080729> has caused this=20 >>>>>>> regression due to changes in method setVisible(boolean visible) in file= >>>>>> =20 >>>>>>> CPlatformWindow.java >>>>>>> orderWindow is causing the issue here, if we revert to addChildWindow=20 >>>>>>> then the issue is not observed but then the fix for issue JDK-8080729 f= >>>>>> ails. >>>>>>> Before this change the child window used to be added on to the parent a= >>>>>> s=20 >>>>>>> shown below in the commented code. But after the change child window is= >>>>>> =20 >>>>>>> ordered above the parent. >>>>>>> =20 >>>>>>> Below code causes the regression: >>>>>>> =20 >>>>>>> *CWrapper.NSWindow.orderWindow(ptr, CWrapper.NSWindow.NSWindowAbove,=20 >>>>>>> ownerPtr);* >>>>>>> *//CWrapper.NSWindow.addChildWindow(ownerPtr, ptr,=20 >>>>>>> CWrapper.NSWindow.NSWindowAbove);* >>>>>>> =20 >>>>>>> Debugging further I found that if we use orderWindow then the new windo= >>>>>> w=20 >>>>>>> is considered as new graphics device in the method notifyReshape in=20 >>>>>>> LWWindowPeer.java (the method updateGraphicsDevice() returns true) >>>>>>> and=20 >>>>>>> is the difference between using orderWindow and addChildWindow. >>>>>>> =20 >>>>>>> Since the option to add child window is between choosing oderWindow and= >>>>>> =20 >>>>>>> addChildWindow we don=E2=80=99t have any option to do the fix in the Ma= >>>>>> c OS=20 >>>>>>> native code. >>>>>>> =20 >>>>>>> Regards, >>>>>>> Manajit >>>>>>> =20 >>>>>>> =20 >>>>>>>> On 02-Jan-2018, at 11:30 PM, Semyon Sadetsky=20 >>>>>>>> <semyon.sadet...@oracle.com <mailto:semyon.sadet...@oracle.com> >>>>>>>> <mailto:semyon.sadet...@oracle.com> >>>>>>>> <mailto:semyon.sadet...@oracle.com>> wrote= >>>>>> : >>>>>>>> Hi Manajit, >>>>>>>> >>>>>>>> JDK-8080729 bug was Mac OS specific issue and its fix changed the >>>>>>>> Mac=20 >>>>>>>> OS code only. Nevertheless you are suggesting to fix the regression in= >>>>>> =20 >>>>>>>> generic code. This need to be explained somehow. >>>>>>>> >>>>>>>> --Semyon >>>>>>>> >>>>>>>> On 12/25/2017 02:42 AM, Manajit Halder wrote: >>>>>>>>> Hi Semyon, >>>>>>>>> >>>>>>>>> Regression is cause by JDK-8080729=20 >>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8080729> >>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8080729>. The fix can=E2=80= >>>>>> =99t be=20 >>>>>>>>> reversed since it is the=C2=A0choice between addChildWindow or=20 >>>>>>>>> orderWindow. Went through code flow related to the issue but=C2=A0did= >>>>>> n=E2=80=99t=20 >>>>>>>>> find any other better place in code to handle this issue. The best=20 >>>>>>>>> way to fix the issue would be to avoid retargeting of events=20 >>>>>>>>> (MOUSE_ENTER and MOUSE_EXIT) between MOUSE_PRESS and MOUSE_RELEASE on= >>>>>> =20 >>>>>>>>> the parent window (when the mouse is actually on the child window).=20 >>>>>>>>> Therefore request you to review the webrev.00. >>>>>>>>> >>>>>>>>> Regards, >>>>>>>>> Manajit >>>>>>>>> >>>>>>>>>> On 08-Dec-2017, at 9:55 PM, semyon.sadet...@oracle.com=20 >>>>>>>>>> <mailto:semyon.sadet...@oracle.com=20> >>>>>>>>>> <mailto:semyon.sadet...@oracle.com> >>>>>>>>>> <mailto:semyon.sadet...@oracle.com> wrote: >>>>>>>>>> >>>>>>>>>> Hi Manajit, >>>>>>>>>> >>>>>>>>>> Can you provide information which fix caused the regression? >>>>>>>>>> >>>>>>>>>> --Semyon >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 12/8/17 5:53 AM, Manajit Halder wrote: >>>>>>>>>>> Hi All, >>>>>>>>>>> >>>>>>>>>>> Kindly review the following Swing fix. >>>>>>>>>>> >>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8189253 >>>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8189253> >>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~mhalder/8189253/webrev.00/=20 >>>>>>>>>>> <http://cr.openjdk.java.net/%7Emhalder/8189253/webrev.00/=20> >>>>>>>>>>> <http://cr.openjdk.java.net/%7Emhalder/8189253/webrev.00/> >>>>>>>>>>> <http://cr.openjdk.java.net/%7Emhalder/8189253/webrev.00/> >>>>>>>>>>> >>>>>>>>>>> Cause: >>>>>>>>>>> Issue was due to retargeting of mouse enter exit events. >>>>>>>>>>> MOUSE_ENTER and MOUSE_EXIT events were sent on the parent window=20 >>>>>>>>>>> (JFrame) in between MOUSE_PRESS and MOUSE_RELEASE events on the=20 >>>>>>>>>>> modeless JDialog. >>>>>>>>>>> >>>>>>>>>>> Fix: >>>>>>>>>>> Retargeting of events is not done in-between MOUSE_PRESS and=20 >>>>>>>>>>> MOUSE_RELEASE. >>>>>>>>>>> >>>>>>>>>>> Regards, >>>>>>>>>>> Manajit >>>>>>>>>>> >>>>>>> =20 >>>>>> --=20 >>>>>> Best regards, Sergey. >>>> >>> >> >
