Hi Sergey, Thank you for the review. Do I need another review? Additionally, I need a sponsor of the fix, since I'm not a committer.
Thanks, Toshio Nakamura Sergey Bylokhov <sergey.bylok...@oracle.com> wrote on 2019/12/03 07:21:33: > From: Sergey Bylokhov <sergey.bylok...@oracle.com> > To: Toshio 5 Nakamura <toshi...@jp.ibm.com> > Cc: swing-dev@openjdk.java.net > Date: 2019/12/03 07:21 > Subject: [EXTERNAL] Re: <Swing Dev> RFR: JDK-8234386: [macos] NPE > was thrown at expanding Choice from maximized frame > > Looks fine. > > On 11/25/19 2:54 am, Toshio 5 Nakamura wrote: > > Hi Sergey, > > > > > One small question about the test, is it necessary to check macOS only? > > > > Yes. Keyboard operations by robot are easy to reproduce the problem, but > > Keyboard don't work for Choice component in other platforms, such as > > Windows, Linux, Solaris, or AIX. So, I think mac only is appropriate. > > > > By the way, I found pair of keyPress and keyRelease events by robot > > are required in some cases. I'd like to update the testcase to webrev.03. > > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Etnakamura_8234386_webrev.03&d=DwID-g&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-qsMtyulHUruf8lAzMlVpVRqw&m=McyywV2aJI5ue0cpeIRNTOGnzH8ZAn5MdLgSuzUioN0&s=OKPdGI5zzS02xmh686qONOuuhCHaT0Kita4S0-z3L_0&e= > > > > Thanks, > > Toshio Nakamura > > > > Sergey Bylokhov <sergey.bylok...@oracle.com> wrote on 2019/11/24 05:19:36: > > > > > From: Sergey Bylokhov <sergey.bylok...@oracle.com> > > > To: Toshio 5 Nakamura <toshi...@jp.ibm.com> > > > Cc: swing-dev@openjdk.java.net > > > Date: 2019/11/24 05:19 > > > Subject: [EXTERNAL] Re: <Swing Dev> RFR: JDK-8234386: [macos] NPE > > > was thrown at expanding Choice from maximized frame > > > > > > One small question about the test, is it necessary to check macOS only? > > > I assume the code will work on any platform? > > > > > > On 11/21/19 5:06 pm, Toshio 5 Nakamura wrote: > > > > Hi Sergey, > > > > > > > > Thank you for the comment. > > > > Please find webrev.02. > > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Etnakamura_8234386_webrev.02&d=DwID-g&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-qsMtyulHUruf8lAzMlVpVRqw&m=McyywV2aJI5ue0cpeIRNTOGnzH8ZAn5MdLgSuzUioN0&s=YVZB0T6J_5bVjnuvzzETZ5YFc1ksYQHscTVpIID0rIQ&e= > > > > > > > > Thanks, > > > > Toshio Nakamura > > > > > > > > Sergey Bylokhov <sergey.bylok...@oracle.com> wrote on 2019/ > 11/22 07:07:03: > > > > > > > > > From: Sergey Bylokhov <sergey.bylok...@oracle.com> > > > > > To: Toshio 5 Nakamura <toshi...@jp.ibm.com> > > > > > Cc: swing-dev@openjdk.java.net > > > > > Date: 2019/11/22 07:07 > > > > > Subject: [EXTERNAL] Re: <Swing Dev> RFR: JDK-8234386: [macos] NPE > > > > > was thrown at expanding Choice from maximized frame > > > > > > > > > > It looks like after the .01 version you can inline the usage of > > > > > Toolkit.getDefaultToolkit(), it is only used inside > > > > > "if (!canPopupOverlapTaskBar()) {". > > > > > > > > > > On 11/21/19 5:10 am, Toshio 5 Nakamura wrote: > > > > > > Hi Sergey, > > > > > > > > > > > > Thank you for the review. > > > > > > How about the fix in webrev.01? > > > > > > > > > > > > To calculate Insets of default screen could be same as to set gc > > > > > of default screen device, I think. > > > > > > I found similar code in RepaintManager. > > > > > > > > > > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Etnakamura_8234386_webrev.01&d=DwID-g&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-qsMtyulHUruf8lAzMlVpVRqw&m=McyywV2aJI5ue0cpeIRNTOGnzH8ZAn5MdLgSuzUioN0&s=HAuHJRofOczuiwLLRx551ypYgwQAIzVnVaDKD0EgQho&e= > > > > > > > > > > > > Thanks, > > > > > > Toshio Nakamura > > > > > > > > > > > > Sergey Bylokhov <sergey.bylok...@oracle.com> wrote on 2019/ > > > 11/21 17:42:39: > > > > > > > > > > > > > From: Sergey Bylokhov <sergey.bylok...@oracle.com> > > > > > > > To: Toshio 5 Nakamura <toshi...@jp.ibm.com>, swing- > > > d...@openjdk.java.net > > > > > > > Date: 2019/11/21 17:42 > > > > > > > Subject: [EXTERNAL] Re: <Swing Dev> RFR: JDK-8234386:[macos] NPE > > > > > > > was thrown at expanding Choice from maximized frame > > > > > > > > > > > > > > Hi, Toshio. > > > > > > > > > > > > > > In a few lines above your fix, the code takes care of null GC and use the main > > > > > > > screen size as a screen bound, so I think your changeshould calculate > > > > > > > Insets for the main screen as well instead of using zeros. > > > > > > > > > > > > > > BTW probably GraphicsEnvironment.getMaximumWindowBounds ()couldbe used at > > > > > > > line 336 additionally to your fix. > > > > > > > > > > > > > > On 11/19/19 7:55 pm, Toshio 5 Nakamura wrote: > > > > > > > > Hi All, > > > > > > > > > > > > > > > > Could you review the following fix? Also, I'd like to ask a > > > > > > > sponsor of this fix, since I'm not a committer. > > > > > > > > > > > > > > > > Bug: https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8234386&d=DwID-g&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-qsMtyulHUruf8lAzMlVpVRqw&m=McyywV2aJI5ue0cpeIRNTOGnzH8ZAn5MdLgSuzUioN0&s=Lo5diBUxrh8BLuk-bHh8on-GOiMxX0AMY1bo_M0lsuM&e= > > > > > > > > Webrev: https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Etnakamura_8234386_webrev.00&d=DwID-g&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-qsMtyulHUruf8lAzMlVpVRqw&m=McyywV2aJI5ue0cpeIRNTOGnzH8ZAn5MdLgSuzUioN0&s=YHZWLfSQ-rkH3zLQyZTYaaPsyWxmQjInB_ikfumAHeI&e= > > > > > > > > > > > > > > > > Issue: > > > > > > > > NullPointerException was thrown when Choice was expanded from > > > > > > > maximized Frame. > > > > > > > > > > > > > > > > Fix: > > > > > > > > Simply adding a null check to JPopupMenu. When the frame was > > > > > > > maximized and expanding > > > > > > > > Choice component, GraphicsConfiguration parameter can be null. > > > > > > > Detailed description was in JBS. > > > > > > > > > > > > > > > > Thanks, > > > > > > > > Toshio Nakamura > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Best regards, Sergey. > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > Best regards, Sergey. > > > > > > > > > > > > > > > > > > -- > > > Best regards, Sergey. > > > > > > > > -- > Best regards, Sergey. >
