[Rev 01] RFR: 8241710: NullPointerException while entering empty submenu with "arrow right"

[Ajit Ghaisas]

On Tue, 7 Apr 2020 11:40:09 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

>> Bug : https://bugs.openjdk.java.net/browse/JDK-8241710
>> 
>> Root Cause : A menu can have empty submenu. This was not checked while processing RIGHT arrow key.
>> 
>> Fix : Added the null check for submenu. Added a unit test case which fails without fix and passes with it.
>
> Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Minor review fixes

> Verified the fix: test is failing before and passing after.

Thanks for the quick test.

> See one inline comment (just noting my personal pref :).
> 
> And again me fighting the system (can't seem to review code parts that are not near a change, so doing here:

Well, you are part of the system :)

>     * copyright year doesn't seem to be updated

I have updated this now.
 
>     * there's another fishy looking code line in MenuItemContainer actionHandler:
>       ```
>       actionEventHandler = e -> {
>            if (item instanceof Menu) {
>                  final Menu menu = (Menu) item;
>                  if (openSubmenu == menu && submenu.isShowing()) return;
>       ```
> 
> 
> don't know when/if that's ever reached (could get there - an action handler on the region itself?), anyway, at other
> places with a similar pattern (f.i processRightKey) there's an explicit guard against a null submenu, don't know if the
> latter is over-caution - logic and code is rather .. well .. inter-twined ;)

Yes. This code does not seem to be ideal, but, it has evolved and a lot of fixes have gone in. So rewriting is ruled
out.

-------------

PR: https://git.openjdk.java.net/jfx/pull/161