<AWT Dev> Swing dev>[12] Review request for JDK-8189253: [macos] JPopupMenu is inadvertently shown when using setComponentPopupMenu

Wed Oct 31 16:41:23 UTC 2018

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/
>
> 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/
>
> 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.bylokhov at 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>  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.sadetsky at oracle.com  <mailto:semyon.sadetsky at 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>. 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.sadetsky at oracle.com=20
>>>>>>> <mailto:semyon.sadetsky at 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
>>>>>>>> Webrev:http://cr.openjdk.java.net/~mhalder/8189253/webrev.00/=20
>>>>>>>> <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.
>

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