Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8244234

MenuButton: NPE on removing from scene with open popup

    Details

      Description

      Bubbled up on SO https://stackoverflow.com/a/61539677/203657 - copied the example here for convenience.

      To reproduce, run open popup and click on menuItem
      - expected: new root showing without errors
      - actual: new root showing, NPE in MenuButtonSkin

      Exception in thread "JavaFX Application Thread" java.lang.NullPointerException
          at java.base/java.util.Objects.requireNonNull(Objects.java:222)
          at javafx.controls/javafx.scene.control.skin.MenuButtonSkinBase.lambda$new$7(MenuButtonSkinBase.java:198)
          at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:428)
          at java.base/java.security.AccessController.doPrivileged(AccessController.java:391)
          at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$11(PlatformImpl.java:427)
          at javafx.graphics/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:96)
          at javafx.graphics/com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
          at javafx.graphics/com.sun.glass.ui.win.WinApplication.lambda$runLoop$3(WinApplication.java:174)
          at java.base/java.lang.Thread.run(Thread.java:832)

      The example:

      import javafx.application.Application;
      import javafx.scene.Scene;
      import javafx.scene.control.Label;
      import javafx.scene.control.MenuButton;
      import javafx.scene.control.MenuItem;
      import javafx.scene.layout.StackPane;
      import javafx.stage.Stage;

      public class Main extends Application {

        @Override
        public void start(Stage primaryStage) {
          MenuButton btn = new MenuButton("Switch...");

          MenuItem item = new MenuItem("To 'Hello, World!'");
          item.setOnAction(
              e -> {
                StackPane root = new StackPane(new Label("Hello, World!"));
                primaryStage.getScene().setRoot(root);
              });
          btn.getItems().add(item);

          primaryStage.setScene(new Scene(new StackPane(btn), 500, 300));
          primaryStage.show();
        }
      }

      Converted into a unit test:

          @Test
          public void testMenuButtonSkinNPEOnReplaceSceneOpenPopup() {
              MenuButton menuButton = new MenuButton("switch");
              MenuItem menuItem = new MenuItem("Press '_a'");
              menuButton.getItems().add(menuItem);

              Scene scene = new Scene(menuButton);
              Stage stage = new Stage();
              menuItem.setOnAction(e -> {
                  StackPane root = new StackPane(new Label("Hello, World!"));
                  scene.setRoot(root);
              });
              stage.setScene(scene);
              stage.show();
              menuButton.requestFocus();
              menuButton.show();
              assertTrue("sanity:", menuButton.isShowing());
              assertSame(scene.getFocusOwner(), menuButton);
              
              // trying to see the NPE
              // working options (the exact sequence of events is slightly different)
              // keyboard on menuItem and press accelerator
              KeyEventFirer keyboard = new KeyEventFirer(menuItem, scene);
              keyboard.doKeyPress(KeyCode.A, KeyModifier.ALT);
          }
          
      The error happens in the showingListener installed in skin:

                  if (popup.isShowing()) {
                      boolean showMnemonics = NodeHelper.isShowMnemonics(getSkinnable());
                      Utils.addMnemonics(popup, getSkinnable().getScene(), showMnemonics, mnemonics);
                  } else {
                      // we wrap this in a runLater so that mnemonics are not removed
                      // before all key events are fired (because KEY_PRESSED might have
                      // been used to hide the menu, but KEY_TYPED and KEY_RELEASED
                      // events are still to be fired, and shouldn't miss out on going
                      // through the mnemonics code (especially in case they should be
                      // consumed to prevent them being used elsewhere).
                      // See JDK-8090026 for more detail.
      >>>> the scene here might be null
                      Scene scene = getSkinnable().getScene();
                      List<Mnemonic> mnemonicsToRemove = new ArrayList<>(mnemonics);
                      mnemonics.clear();
      >>>> that's why we get the npe here
                      Platform.runLater(() -> mnemonicsToRemove.forEach(scene::removeMnemonic));

      Note that this is a different root than the other NPEs (JDK-8244081, JDK-8244110) around MenuButtonSkin, it's uneffected by cleaning up in the sceneListener and/or in dispose. There seem to be several paths that remove the accelerators from the scene, probably need to find them all and guard against null (or funnel into a single cleanup, don't know if that's possible)

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                aghaisas Ajit Ghaisas
                Reporter:
                fastegal Jeanette Winzenburg
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated: