On Fri, 10 Nov 2023 10:34:08 GMT, Johan Vos <j...@openjdk.org> wrote:
> A listener was added but never removed. > This patch removes the listener when the menu it links to is cleared. Fix for > https://bugs.openjdk.org/browse/JDK-8319779 Should there be an updated test for this? I see a lot of raw type use (`ListChangeListener`, `ObservableList`), any chance those can be avoided? modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java line 63: > 61: private MenuBar glassSystemMenuBar = null; > 62: private final Map <Menu, ListChangeListener> menuListeners = new > HashMap<>(); > 63: private final Map <ListChangeListener, ObservableList> listenerItems > = new HashMap<>(); minor: Suggestion: private final Map<Menu, ListChangeListener> menuListeners = new HashMap<>(); private final Map<ListChangeListener, ObservableList> listenerItems = new HashMap<>(); modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java line 171: > 169: if (item instanceof MenuBase) { > 170: // submenu > 171: addMenu(glassMenu, (MenuBase)item); Suggestion: if (item instanceof MenuBase baseItem) { // submenu addMenu(glassMenu, baseItem); modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java line 189: > 187: > 188: private ListChangeListener createListener(final Menu glassMenu) { > 189: ListChangeListener<MenuItemBase> answer = > ((ListChangeListener.Change<? extends MenuItemBase> change) -> { minor: I see extra parenthesis around the whole, and I think you can just `return` this immediately (no need for `answer` local) ------------- PR Review: https://git.openjdk.org/jfx/pull/1283#pullrequestreview-1724683737 PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1389343964 PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1389338458 PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1389340165
