On Tue, 7 Apr 2020 11:40:09 GMT, Ajit Ghaisas <[email protected]> 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
