RFR: 8330304: MenuBar: Invisible Menu works incorrectly with keyboard arrows

Michael Strauß mstrauss at openjdk.org
Fri May 31 12:38:07 UTC 2024


On Thu, 30 May 2024 15:36:37 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

> The root cause is that the skin used two fields to store one entity (`focusedMenu` and `focusedMenuIndex`), causing mismatch when invisible menu(s) are present.
> 
> The fix involves using a single index variable.
> 
> Also wanted to note that in theory, `openMenu` and `openMenuButton` can also be replaced with a single boolean, but I decided not to change these in order to keep the amount of diffs to a minimum.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java line 735:

> 733:     // package protected for testing purposes
> 734:     MenuBarButton menuBarButtonAt(int i) {
> 735:         if (i < container.getChildren().size()) {

This method throws `IndexOutOfBoundsException` if the index is negative, but returns `null` if the index is larger than the collection size. None of the callers account for this, so we'd just get `NullPointerException` at the call sites.

Since you touched this method, I suggest replacing the implementation with

MenuBarButton menuBarButtonAt(int i) {
    return (MenuBarButton)container.getChildren().get(i);
}

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1467#discussion_r1622359643


More information about the openjfx-dev mailing list