From ac14f117a9dd0013e72a0f46b594cbe312501321 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 19 May 2026 07:36:16 +0300 Subject: [PATCH 1/4] Fix #4979 dim backdrop lingers after Dialog opened from on-top side menu Tapping a command in the on-top side menu used to translate to a synchronous closeSideMenu() followed immediately by cmd.actionPerformed on the same EDT call (SideMenuBar.CommandWrapper.actionPerformed). When the command shows a modal Dialog -- which is exactly what the default initializr archetype's `hello()` does -- the Dialog's event pump took over the EDT before the side menu's async dispose animation had a chance to advance. The dispose onFinish runnable that calls detachToolbarLayeredPane never fired, so the Toolbar's dim layered- pane backdrop (bgColor=0, bgTransparency around 64) stayed attached to the form. After the user dismissed the Dialog the form was still shaded. This is a sibling of #4912 (fixed in d04297194). #4912 handled the "close the side menu by tapping outside" path; #4979 is the "tap a command" path where the close races with the command's Dialog. Fix lands in two pieces: * Toolbar gains public closeSideMenu(Runnable), closeLeftSideMenu( Runnable), and closeRightSideMenu(Runnable) overloads that invoke the supplied callback once the dispose animation has completed and the layered pane has been detached. When no menu is open the callback runs synchronously. closeSideMenu(Runnable) waits for both left and right to finish if both are showing. * SideMenuBar.CommandWrapper.actionPerformed routes the cmd.actionPerformed(evt) through this onFinish in the on-top branch. The non-onTop branch (lines 1812+) was already correct -- it defers via ShowWaiter. Regression test added: ToolbarTest.sideMenuCommandFiresAfterLayered- PaneDetach opens the side menu, captures the dim pane reference, fires the command button via pointer events, and asserts that when the command's listener runs the captured pane is already detached. The dispose animation is disabled via reflection so the test does not depend on wall-clock animation timing (which the test EDT can't tick while blocked on the test body). The existing right-side dispatch test needed the same animation-disable for the same reason -- pulled into a disableSideMenuAnimation helper. All 2501 tests in core-unittests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/com/codename1/ui/SideMenuBar.java | 17 +++- CodenameOne/src/com/codename1/ui/Toolbar.java | 97 ++++++++++++++++++- .../java/com/codename1/ui/ToolbarTest.java | 97 ++++++++++++++++++- 3 files changed, 206 insertions(+), 5 deletions(-) diff --git a/CodenameOne/src/com/codename1/ui/SideMenuBar.java b/CodenameOne/src/com/codename1/ui/SideMenuBar.java index c3d5306893..6270b8a0df 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..b1af64aa6e 100644 --- a/CodenameOne/src/com/codename1/ui/Toolbar.java +++ b/CodenameOne/src/com/codename1/ui/Toolbar.java @@ -439,12 +439,76 @@ 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; + } + final int pending = (leftShowing ? 1 : 0) + (rightShowing ? 1 : 0); + final int[] remaining = {pending}; + Runnable countdown = onFinish == null ? null : new Runnable() { + @Override + public void run() { + remaining[0]--; + if (remaining[0] == 0) { + onFinish.run(); + } + } + }; + if (leftShowing) { + closeLeftSideMenu(countdown); + } + if (rightShowing) { + closeRightSideMenu(countdown); + } } /// 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 +516,9 @@ public void closeLeftSideMenu() { @Override public void run() { detachToolbarLayeredPane(cnt); + if (onFinish != null) { + onFinish.run(); + } } }; if (!isRTL()) { @@ -459,14 +526,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 +560,9 @@ public void closeRightSideMenu() { @Override public void run() { detachToolbarLayeredPane(cnt); + if (onFinish != null) { + onFinish.run(); + } } }; if (!isRTL()) { @@ -481,8 +570,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 From eb191ceb6cd24ea0caf9251941d68bb6a41bec5b Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 19 May 2026 08:05:31 +0300 Subject: [PATCH 2/4] Toolbar: hoist countdown Runnable to a static nested class SpotBugs rule SIC_INNER_SHOULD_BE_STATIC_ANON on the prior commit's anonymous countdown Runnable inside closeSideMenu(Runnable) -- it captured only locals (the remaining[] array and the onFinish Runnable) and never used the enclosing Toolbar instance, so it was holding the implicit outer-this reference for nothing. Extracts the logic into a private static final CloseSideMenuCountdown class with a plain int field, which is both cleaner and SpotBugs-clean. No behavioural change. Co-Authored-By: Claude Opus 4.7 (1M context) --- CodenameOne/src/com/codename1/ui/Toolbar.java | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/CodenameOne/src/com/codename1/ui/Toolbar.java b/CodenameOne/src/com/codename1/ui/Toolbar.java index b1af64aa6e..caa6ee84fe 100644 --- a/CodenameOne/src/com/codename1/ui/Toolbar.java +++ b/CodenameOne/src/com/codename1/ui/Toolbar.java @@ -473,17 +473,8 @@ public void closeSideMenu(final Runnable onFinish) { } return; } - final int pending = (leftShowing ? 1 : 0) + (rightShowing ? 1 : 0); - final int[] remaining = {pending}; - Runnable countdown = onFinish == null ? null : new Runnable() { - @Override - public void run() { - remaining[0]--; - if (remaining[0] == 0) { - onFinish.run(); - } - } - }; + int pending = (leftShowing ? 1 : 0) + (rightShowing ? 1 : 0); + Runnable countdown = onFinish == null ? null : new CloseSideMenuCountdown(pending, onFinish); if (leftShowing) { closeLeftSideMenu(countdown); } @@ -492,6 +483,29 @@ public void run() { } } + /// 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() { + if (--remaining == 0) { + delegate.run(); + } + } + } + /// Closes the left side menu public void closeLeftSideMenu() { closeLeftSideMenu(null); From 7eb21c6cddf8eb9fbf527cf8a815f9302b076641 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 19 May 2026 10:23:49 +0300 Subject: [PATCH 3/4] Toolbar: split predecrement out of conditional in CloseSideMenuCountdown PMD rule AssignmentInOperand on the prior commit's `--remaining == 0` predecrement-then-compare in CloseSideMenuCountdown.run. Hoist the decrement to its own statement. No behavioural change. Co-Authored-By: Claude Opus 4.7 (1M context) --- CodenameOne/src/com/codename1/ui/Toolbar.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CodenameOne/src/com/codename1/ui/Toolbar.java b/CodenameOne/src/com/codename1/ui/Toolbar.java index caa6ee84fe..8605720f11 100644 --- a/CodenameOne/src/com/codename1/ui/Toolbar.java +++ b/CodenameOne/src/com/codename1/ui/Toolbar.java @@ -500,7 +500,8 @@ private static final class CloseSideMenuCountdown implements Runnable { @Override public void run() { - if (--remaining == 0) { + remaining--; + if (remaining == 0) { delegate.run(); } } From 39db7a6790f7f03961f95a2b4e825442ca8d7001 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Tue, 19 May 2026 10:36:08 +0300 Subject: [PATCH 4/4] SideMenuBar: drop em-dash from #4979 comment so Ports/Android Ant javac is happy The Ports/Android Ant build compiles CodenameOne/src under encoding=ASCII (build.xml line 111). My PR-#4983 comment included a literal U+2014 em-dash which the ASCII javac rejected with "unmappable character (0xE2/0x80/0x94)". Swap the em-dash for a double hyphen. Co-Authored-By: Claude Opus 4.7 (1M context) --- CodenameOne/src/com/codename1/ui/SideMenuBar.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CodenameOne/src/com/codename1/ui/SideMenuBar.java b/CodenameOne/src/com/codename1/ui/SideMenuBar.java index 6270b8a0df..2fa0e1c4b8 100644 --- a/CodenameOne/src/com/codename1/ui/SideMenuBar.java +++ b/CodenameOne/src/com/codename1/ui/SideMenuBar.java @@ -1812,7 +1812,7 @@ public void actionPerformed(final ActionEvent evt) { // 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 + // detachToolbarLayeredPane could fire -- leaving the // dim backdrop visible after the Dialog was dismissed. Display.getInstance().getCurrent().getToolbar().closeSideMenu(new Runnable() { @Override