On Tue, 7 Apr 2020 07:36:00 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. Verified the fix: test is failing before and passing after. 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: - copyright year doesn't seem to be updated - 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 ;) modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/ContextMenuContent.java line 658: > 657: ContextMenuContent cmContent = > (ContextMenuContent)submenu.getSkin().getNode(); > 658: if (cmContent != null) { > 659: if (cmContent.itemsContainer.getChildren().size() > 0) { just a mini-note: personally, I prefer early-return on no-match (vs. wrapping nearly the whole method inside an if match if (submenu == null) return; ------------- PR: https://git.openjdk.java.net/jfx/pull/161
