diff --git a/CodenameOne/src/com/codename1/ui/SideMenuBar.java b/CodenameOne/src/com/codename1/ui/SideMenuBar.java index c3d5306893..2fa0e1c4b8 100644 --- a/CodenameOne/src/com/codename1/ui/SideMenuBar.java +++ b/CodenameOne/src/com/codename1/ui/SideMenuBar.java @@ -1805,8 +1805,21 @@ public int hashCode() { @Override public void actionPerformed(final ActionEvent evt) { if (Toolbar.isOnTopSideMenu() && (Toolbar.isGlobalToolbar() || Display.getInstance().getCommandBehavior() != Display.COMMAND_BEHAVIOR_SIDE_NAVIGATION)) { - Display.getInstance().getCurrent().getToolbar().closeSideMenu(); - cmd.actionPerformed(evt); + // Issue #4979: fire the command *after* the side-menu + // dispose animation has finished and the Toolbar's + // layered-pane dim backdrop has been detached. The + // previous code ran cmd.actionPerformed synchronously + // right after starting the async dispose, so if the + // command invoked a modal Dialog.show() the Dialog's + // event pump took over the EDT before + // detachToolbarLayeredPane could fire -- leaving the + // dim backdrop visible after the Dialog was dismissed. + Display.getInstance().getCurrent().getToolbar().closeSideMenu(new Runnable() { + @Override + public void run() { + cmd.actionPerformed(evt); + } + }); return; } if (transitionRunning) { diff --git a/CodenameOne/src/com/codename1/ui/Toolbar.java b/CodenameOne/src/com/codename1/ui/Toolbar.java index 47000ab7fe..8605720f11 100644 --- a/CodenameOne/src/com/codename1/ui/Toolbar.java +++ b/CodenameOne/src/com/codename1/ui/Toolbar.java @@ -439,12 +439,91 @@ public void openRightSideMenu() { /// Closes the current side menu public void closeSideMenu() { - closeLeftSideMenu(); - closeRightSideMenu(); + closeSideMenu(null); + } + + /// Closes the current side menu and invokes `onFinish` once the + /// side-menu dispose animation has completed and the Toolbar + /// layered-pane dim has been detached. When no side menu is + /// currently open `onFinish` runs synchronously. + /// + /// Tapping a command in the on-top side menu was previously + /// implemented as `closeSideMenu()` followed by a synchronous + /// `cmd.actionPerformed(evt)` on the same EDT call. If the command + /// then showed a modal Dialog, the Dialog's event pump took over + /// before the dispose animation had a chance to advance, the + /// `detachToolbarLayeredPane` callback never fired, and the dim + /// backdrop stayed visible after the Dialog was dismissed (issue + /// #4979). `SideMenuBar.actionPerformed` now routes the + /// command-fire through this `onFinish`, so the layered pane is + /// guaranteed to be detached before the command runs. + /// + /// #### Parameters + /// + /// - `onFinish`: callback to invoke once the side menu is closed, + /// or `null` to skip + public void closeSideMenu(final Runnable onFinish) { + boolean leftShowing = onTopSideMenu && sidemenuDialog != null && sidemenuDialog.isShowing(); + boolean rightShowing = onTopSideMenu && rightSidemenuDialog != null && rightSidemenuDialog.isShowing(); + if (!leftShowing && !rightShowing) { + closeLeftSideMenu(); + closeRightSideMenu(); + if (onFinish != null) { + onFinish.run(); + } + return; + } + int pending = (leftShowing ? 1 : 0) + (rightShowing ? 1 : 0); + Runnable countdown = onFinish == null ? null : new CloseSideMenuCountdown(pending, onFinish); + if (leftShowing) { + closeLeftSideMenu(countdown); + } + if (rightShowing) { + closeRightSideMenu(countdown); + } + } + + /// Runnable that decrements a counter and fires its delegate when + /// the counter reaches zero. Used by [#closeSideMenu(Runnable)][closeSideMenu] + /// to wait for both left and right dispose animations when both + /// side menus are open. Declared as a static nested class so the + /// Runnable does not retain an implicit reference to the + /// surrounding Toolbar instance. + private static final class CloseSideMenuCountdown implements Runnable { + private int remaining; + private final Runnable delegate; + + CloseSideMenuCountdown(int remaining, Runnable delegate) { + this.remaining = remaining; + this.delegate = delegate; + } + + @Override + public void run() { + remaining--; + if (remaining == 0) { + delegate.run(); + } + } } /// Closes the left side menu public void closeLeftSideMenu() { + closeLeftSideMenu(null); + } + + /// Closes the left side menu and invokes `onFinish` once the + /// dispose animation has completed and the Toolbar layered-pane + /// dim has been detached. When no left side menu is currently + /// open `onFinish` runs synchronously. See + /// [#closeSideMenu(Runnable)][closeSideMenu] for rationale (issue + /// #4979). + /// + /// #### Parameters + /// + /// - `onFinish`: callback to invoke once the menu is closed, or + /// `null` to skip + public void closeLeftSideMenu(final Runnable onFinish) { if (onTopSideMenu) { if (sidemenuDialog != null && sidemenuDialog.isShowing()) { final Container cnt = getComponentForm().getFormLayeredPane(Toolbar.class, false); @@ -452,6 +531,9 @@ public void closeLeftSideMenu() { @Override public void run() { detachToolbarLayeredPane(cnt); + if (onFinish != null) { + onFinish.run(); + } } }; if (!isRTL()) { @@ -459,14 +541,33 @@ public void run() { } else { sidemenuDialog.disposeToTheRight(onDisposed); } + return; } } else { SideMenuBar.closeCurrentMenu(); } + if (onFinish != null) { + onFinish.run(); + } } /// Closes the right side menu public void closeRightSideMenu() { + closeRightSideMenu(null); + } + + /// Closes the right side menu and invokes `onFinish` once the + /// dispose animation has completed and the Toolbar layered-pane + /// dim has been detached. When no right side menu is currently + /// open `onFinish` runs synchronously. See + /// [#closeSideMenu(Runnable)][closeSideMenu] for rationale (issue + /// #4979). + /// + /// #### Parameters + /// + /// - `onFinish`: callback to invoke once the menu is closed, or + /// `null` to skip + public void closeRightSideMenu(final Runnable onFinish) { if (onTopSideMenu) { if (rightSidemenuDialog != null && rightSidemenuDialog.isShowing()) { final Container cnt = getComponentForm().getFormLayeredPane(Toolbar.class, false); @@ -474,6 +575,9 @@ public void closeRightSideMenu() { @Override public void run() { detachToolbarLayeredPane(cnt); + if (onFinish != null) { + onFinish.run(); + } } }; if (!isRTL()) { @@ -481,8 +585,12 @@ public void run() { } else { rightSidemenuDialog.disposeToTheLeft(onDisposed); } + return; } } + if (onFinish != null) { + onFinish.run(); + } } /// Detaches the Toolbar's shared FormLayeredPane once the side-menu diff --git a/maven/core-unittests/src/test/java/com/codename1/ui/ToolbarTest.java b/maven/core-unittests/src/test/java/com/codename1/ui/ToolbarTest.java index 3dc9807016..e0f420b8bc 100644 --- a/maven/core-unittests/src/test/java/com/codename1/ui/ToolbarTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/ui/ToolbarTest.java @@ -136,7 +136,7 @@ void rightBarCommandsAndTitleComponentCustomization() { } @FormTest - void rightSideMenuCommandsDispatch() { + void rightSideMenuCommandsDispatch() throws Exception { implementation.setBuiltinSoundsEnabled(false); Toolbar.setOnTopSideMenu(true); @@ -158,6 +158,14 @@ void rightSideMenuCommandsDispatch() { Button rightButton = toolbar.findCommandComponent(rightSide); assertNotNull(rightButton, "Right side menu button should be created"); + // Issue #4979: command dispatch from the side menu is deferred + // to the dispose-animation onFinish so the dim layered pane is + // fully detached before the command runs. The test EDT cannot + // tick wall-clock animations while blocked on the test body, + // so disable the dispose animation to make the deferred fire + // synchronous. + disableSideMenuAnimation(toolbar, "rightSidemenuDialog"); + int px = rightButton.getAbsoluteX() + rightButton.getWidth() / 2; int py = rightButton.getAbsoluteY() + rightButton.getHeight() / 2; rightButton.pointerPressed(px, py); @@ -172,6 +180,17 @@ void rightSideMenuCommandsDispatch() { awaitAnimations(form); } + private static void disableSideMenuAnimation(Toolbar toolbar, String dialogFieldName) throws Exception { + java.lang.reflect.Field dialogField = Toolbar.class.getDeclaredField(dialogFieldName); + dialogField.setAccessible(true); + Object dialog = dialogField.get(toolbar); + if (dialog == null) { + return; + } + java.lang.reflect.Method setAnimateShow = dialog.getClass().getMethod("setAnimateShow", boolean.class); + setAnimateShow.invoke(dialog, false); + } + @FormTest void sideMenuAndOverflowCommands() { implementation.setBuiltinSoundsEnabled(false); @@ -289,6 +308,82 @@ private static boolean isFormInRevalidateQueue(Form form) throws Exception { return false; } + /// Regression test for issue #4979: tapping a command in the + /// on-top side menu used to run cmd.actionPerformed synchronously + /// right after kicking off the side menu's async dispose + /// animation. If the command then showed a modal Dialog (the + /// archetype's default `hello()` does exactly this) the Dialog's + /// event pump stole the EDT before the dispose animation could + /// advance, the detachToolbarLayeredPane onFinish never fired, + /// and the dim backdrop stayed visible after the Dialog was + /// dismissed. + /// + /// The fix routes the command-fire through the new + /// closeSideMenu(Runnable) onFinish, so the layered pane is + /// guaranteed to be detached *before* the command runs. The + /// assertion below checks that ordering directly: when the + /// command's listener fires, the Toolbar layered pane must + /// already be gone. + @FormTest + void sideMenuCommandFiresAfterLayeredPaneDetach() throws Exception { + implementation.setBuiltinSoundsEnabled(false); + Toolbar.setOnTopSideMenu(true); + + Form form = Display.getInstance().getCurrent(); + Toolbar toolbar = new Toolbar(); + form.setToolbar(toolbar); + form.show(); + form.getAnimationManager().flush(); + flushSerialCalls(); + + // Capture the *specific* dim layered-pane instance via this + // array so the listener can check that exact reference. We + // cannot just call form.getFormLayeredPane(Toolbar.class, + // false) inside the listener because that method is + // get-or-create — once the original pane is removed it would + // hand back a brand new attached Container. + final Container[] capturedPane = new Container[1]; + final boolean[] paneAttachedWhenCommandFired = {true}; + final int[] invocation = {0}; + Command hello = toolbar.addCommandToSideMenu("Hello", null, evt -> { + Container p = capturedPane[0]; + paneAttachedWhenCommandFired[0] = p != null && p.getParent() != null; + invocation[0]++; + }); + + toolbar.openSideMenu(); + form.getAnimationManager().flush(); + flushSerialCalls(); + awaitAnimations(form); + assertTrue(toolbar.isSideMenuShowing(), "Side menu should be showing after open"); + capturedPane[0] = form.getFormLayeredPane(Toolbar.class, false); + assertNotNull(capturedPane[0], "Toolbar layered pane must exist while menu is open"); + assertNotNull(capturedPane[0].getParent(), "Layered pane must be attached while menu is open"); + + // Make the dispose synchronous so we don't depend on the + // animation thread inside the test; the bug we are pinning + // down is about ordering between the dispose onFinish and the + // command-fire, not about the animation duration. + disableSideMenuAnimation(toolbar, "sidemenuDialog"); + + // Dispatch through the same path the side-menu button click + // would take — the CommandWrapper.actionPerformed branch that + // does the close-then-fire dance. + Button helloButton = toolbar.findCommandComponent(hello); + assertNotNull(helloButton, "Side menu command should have a button while menu is open"); + int px = helloButton.getAbsoluteX() + helloButton.getWidth() / 2; + int py = helloButton.getAbsoluteY() + helloButton.getHeight() / 2; + helloButton.pointerPressed(px, py); + helloButton.pointerReleased(px, py); + flushSerialCalls(); + + assertEquals(1, invocation[0], "Command listener should have fired exactly once"); + assertFalse(paneAttachedWhenCommandFired[0], + "Issue #4979: command listener must run *after* the Toolbar layered pane " + + "is detached so a modal Dialog opened by the command cannot leave " + + "the dim backdrop visible behind it"); + } + /// Regression test for the JavaScript port "ghost side menu + /// previous preview visible as background" bug. closeLeftSideMenu /// used to synchronously detach the Toolbar's FormLayeredPane