From 2fbba24bd611a7638b156fd5f4b39bd18124f918 Mon Sep 17 00:00:00 2001 From: "totally-not-ai[bot]" <290682512+totally-not-ai[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 12:11:59 +0000 Subject: [PATCH 1/6] fix: open and close ContextMenu through the client path ContextMenuTester.open() and close() wrote the menu's opened element property with a plain server-side setProperty, so the resulting OpenedChangeEvent reported isFromClient() as false. An application that branches on isFromClient() saw the tester's open as a programmatic one. The opened property is @Synchronize-d on ContextMenuBase#isOpened(), so the update now goes through setPropertyAsUser, the same helper Details and Accordion use for that property. The usability check in open() moves ahead of the property update: a client originated update is refused on a component that is not usable, so running the check first keeps the tester's own error message and leaves the menu closed when it throws. --- .../contextmenu/ContextMenuTesterTest.java | 14 ++++++++++++++ .../component/contextmenu/ContextMenuTester.java | 6 ++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java b/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java index cad6f1a7..5a6658cc 100644 --- a/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java +++ b/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java @@ -15,6 +15,7 @@ */ package com.vaadin.flow.component.contextmenu; +import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.Assertions; @@ -51,6 +52,19 @@ void openCloseMenu_menuIsAttachedAndDetached() { "context menu should be detached from the UI, but was not"); } + @Test + void openCloseMenu_openedChangeEventsComeFromClient() { + List fromClient = new ArrayList<>(); + view.menu.addOpenedChangeListener( + event -> fromClient.add(event.isFromClient())); + + test(view.menu).open(); + test(view.menu).close(); + + Assertions.assertEquals(List.of(true, true), fromClient, + "opening and closing the menu should be reported as user actions"); + } + @Test void programmaticallyClose_menuIsDetached() { test(view.menu).open(); diff --git a/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java b/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java index 074a8d1b..0b81fcec 100644 --- a/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java +++ b/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java @@ -80,8 +80,10 @@ public void open() { } attachMenuToUI(); roundTrip(); - getComponent().getElement().setProperty("opened", true); ensureComponentIsUsable(); + // Simulate the overlay reporting itself as opened so that the + // resulting OpenedChangeEvent is seen as a user action. + setPropertyAsUser("opened", true); } /** @@ -89,7 +91,7 @@ public void open() { */ public void close() { ensureComponentIsUsable(); - getComponent().getElement().setProperty("opened", false); + setPropertyAsUser("opened", false); } /** From f5dfe2c9bb26f132ed4fdc4d83cc0248509d65c4 Mon Sep 17 00:00:00 2001 From: "totally-not-ai[bot]" <290682512+totally-not-ai[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 14:49:38 +0000 Subject: [PATCH 2/6] test: cover that a refused ContextMenu open leaves the menu closed The usability check in ContextMenuTester.open() now runs before the property update, so a menu that is not usable stays closed when open() throws instead of being left half-opened. Co-Authored-By: Claude Opus 5 (1M context) --- .../component/contextmenu/ContextMenuTesterTest.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java b/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java index 5a6658cc..1a4a97db 100644 --- a/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java +++ b/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java @@ -84,6 +84,16 @@ void openMenu_alreadyOpen_throws() { Assertions.assertTrue(exception.getMessage().contains("already open")); } + @Test + void openMenu_notUsable_throws() { + view.menu.setVisible(false); + + Assertions.assertThrows(IllegalStateException.class, + test(view.menu)::open); + Assertions.assertFalse(view.menu.isOpened(), + "a refused open should leave the menu closed"); + } + @Test void closeMenu_menuNotOpened_throws() { Assertions.assertThrows(IllegalStateException.class, From 70f46d2445886cca716de246e90af0e584b06336 Mon Sep 17 00:00:00 2001 From: "totally-not-ai[bot]" <290682512+totally-not-ai[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 14:57:32 +0000 Subject: [PATCH 3/6] fix: undo the menu attach when ContextMenuTester.open() is refused open() fires the before-open DOM event, which attaches the menu content to the UI, before it can check usability: the menu is not attached until that event, so ensureComponentIsUsable() cannot run any earlier. A menu that turned out not to be usable was therefore left attached with opened=false, so a closed menu stayed reachable through a top level find(). The attach is now undone when the check refuses the open, making open() leave no trace when it throws. The test also pins the message of the refusal, so that moving the usability check back after the property update is caught: that would throw the generic "unable to simulate a client side update" instead. Co-Authored-By: Claude Opus 5 (1M context) --- .../component/contextmenu/ContextMenuTesterTest.java | 11 +++++++++-- .../flow/component/contextmenu/ContextMenuTester.java | 10 +++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java b/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java index 1a4a97db..03592bf0 100644 --- a/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java +++ b/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java @@ -88,10 +88,17 @@ void openMenu_alreadyOpen_throws() { void openMenu_notUsable_throws() { view.menu.setVisible(false); - Assertions.assertThrows(IllegalStateException.class, - test(view.menu)::open); + IllegalStateException exception = Assertions.assertThrows( + IllegalStateException.class, test(view.menu)::open); + Assertions.assertTrue(exception.getMessage().contains("is not usable")); + Assertions.assertFalse(view.menu.isOpened(), "a refused open should leave the menu closed"); + Assertions.assertFalse(view.menu.isAttached(), + "a refused open should leave the menu detached from the UI"); + Assertions.assertEquals(0, + find(Div.class).withText("Component Item").all().size(), + "a refused open should not leave the menu content reachable through a top level find()"); } @Test diff --git a/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java b/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java index 0b81fcec..212e86f5 100644 --- a/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java +++ b/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java @@ -80,7 +80,15 @@ public void open() { } attachMenuToUI(); roundTrip(); - ensureComponentIsUsable(); + try { + ensureComponentIsUsable(); + } catch (RuntimeException e) { + // The before-open event above attached the menu content to the + // UI. A refused open must not leave it behind, otherwise a closed + // menu stays reachable through a top level find(). + getComponent().getElement().removeFromParent(); + throw e; + } // Simulate the overlay reporting itself as opened so that the // resulting OpenedChangeEvent is seen as a user action. setPropertyAsUser("opened", true); From 8d5435d5c4e05992a0b8def0ae320f7b055071ab Mon Sep 17 00:00:00 2001 From: "totally-not-ai[bot]" <290682512+totally-not-ai[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:05:02 +0000 Subject: [PATCH 4/6] fix: check usability before opening a GridContextMenu GridContextMenuTester.open(int, String) pushed opened=true through the client path before checking that the menu is usable. A menu that is not usable was therefore left opened and attached, and a GridContextMenuOpenedEvent was fired for an open that then threw. The usability check now runs before the property update, and the attach done by the before-open event is undone when the check refuses the open, so a refused open leaves no trace. This mirrors ContextMenuTester.open(). Co-Authored-By: Claude Opus 5 (1M context) --- .../contextmenu/GridContextMenuTesterTest.java | 16 ++++++++++++++++ .../grid/contextmenu/GridContextMenuTester.java | 10 +++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/junit6/src/test/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTesterTest.java b/junit6/src/test/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTesterTest.java index 51b61733..70d1b745 100644 --- a/junit6/src/test/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTesterTest.java +++ b/junit6/src/test/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTesterTest.java @@ -190,6 +190,22 @@ void clickItem_disabledItem_throws() { Assertions.assertTrue(exception.getMessage().contains("not usable")); } + @Test + void openOnRow_menuNotUsable_throws() { + view.menu.setVisible(false); + + IllegalStateException exception = Assertions.assertThrows( + IllegalStateException.class, () -> test(view.menu).open(0)); + Assertions.assertTrue(exception.getMessage().contains("is not usable")); + + Assertions.assertFalse(view.menu.isOpened(), + "a refused open should leave the menu closed"); + Assertions.assertFalse(view.menu.isAttached(), + "a refused open should leave the menu detached from the UI"); + Assertions.assertEquals(0, find(Checkbox.class).all().size(), + "a refused open should not leave the menu content reachable through a top level find()"); + } + @Test void clickItem_menuNotOpened_throws() { Assertions.assertThrows(IllegalStateException.class, diff --git a/shared/src/main/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTester.java b/shared/src/main/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTester.java index a89af38e..65065c56 100644 --- a/shared/src/main/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTester.java +++ b/shared/src/main/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTester.java @@ -168,11 +168,19 @@ public void open(int row, String columnKey) { throw new IllegalStateException( "Context menu did not open. Its dynamic content handler returned false for the target row."); } + try { + ensureComponentIsUsable(); + } catch (RuntimeException e) { + // The before-open event above attached the menu content to the + // UI. A refused open must not leave it behind, otherwise a closed + // menu stays reachable through a top level find(). + getComponent().getElement().removeFromParent(); + throw e; + } // opened is a synchronized property, so pushing it through the // client path makes the GridContextMenuOpenedEvent report // isFromClient() as true, the way a real open does setPropertyAsUser("opened", true); - ensureComponentIsUsable(); } /** From 684dfe462e9d266c5c9f0a38bbce0b357efd6333 Mon Sep 17 00:00:00 2001 From: "totally-not-ai[bot]" <290682512+totally-not-ai[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:09:49 +0000 Subject: [PATCH 5/6] refactor: share the detach-on-refused-open check between the menu testers ContextMenuTester.open() and GridContextMenuTester.open(int, String) had the same try/catch undoing the attach done by the before-open event when the usability check refuses the open. The invariant now lives once, as ComponentTester#ensureComponentIsUsableOrDetach(), next to the check it wraps, so the two menu testers cannot drift apart. Co-Authored-By: Claude Opus 5 (1M context) --- .../vaadin/browserless/ComponentTester.java | 23 +++++++++++++++++++ .../contextmenu/ContextMenuTester.java | 10 +------- .../contextmenu/GridContextMenuTester.java | 10 +------- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/shared/src/main/java/com/vaadin/browserless/ComponentTester.java b/shared/src/main/java/com/vaadin/browserless/ComponentTester.java index fc0ae9e6..dc40ebc4 100644 --- a/shared/src/main/java/com/vaadin/browserless/ComponentTester.java +++ b/shared/src/main/java/com/vaadin/browserless/ComponentTester.java @@ -186,6 +186,29 @@ public final void ensureComponentIsUsable() { this::notUsableReasons); } + /** + * Checks that the wrapped component is usable and, if it is not, detaches + * it from the UI before rethrowing. + *

+ * For an overlay the tester has to attach to the UI before it can tell + * whether it is usable, such as a context menu opened by a before-open + * event. Detaching it again keeps a refused interaction from leaving a + * closed overlay behind in the UI tree, where a top level {@code find(...)} + * would still reach its content. + * + * @throws IllegalStateException + * if the component is not usable, with details on its current + * state. + */ + protected void ensureComponentIsUsableOrDetach() { + try { + ensureComponentIsUsable(); + } catch (RuntimeException e) { + getComponent().getElement().removeFromParent(); + throw e; + } + } + /** * Throws an {@link IllegalStateException} with details on the current state * of the component if it is not usable according to the provided test. diff --git a/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java b/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java index 212e86f5..a047c07e 100644 --- a/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java +++ b/shared/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuTester.java @@ -80,15 +80,7 @@ public void open() { } attachMenuToUI(); roundTrip(); - try { - ensureComponentIsUsable(); - } catch (RuntimeException e) { - // The before-open event above attached the menu content to the - // UI. A refused open must not leave it behind, otherwise a closed - // menu stays reachable through a top level find(). - getComponent().getElement().removeFromParent(); - throw e; - } + ensureComponentIsUsableOrDetach(); // Simulate the overlay reporting itself as opened so that the // resulting OpenedChangeEvent is seen as a user action. setPropertyAsUser("opened", true); diff --git a/shared/src/main/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTester.java b/shared/src/main/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTester.java index 65065c56..cf69c829 100644 --- a/shared/src/main/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTester.java +++ b/shared/src/main/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTester.java @@ -168,15 +168,7 @@ public void open(int row, String columnKey) { throw new IllegalStateException( "Context menu did not open. Its dynamic content handler returned false for the target row."); } - try { - ensureComponentIsUsable(); - } catch (RuntimeException e) { - // The before-open event above attached the menu content to the - // UI. A refused open must not leave it behind, otherwise a closed - // menu stays reachable through a top level find(). - getComponent().getElement().removeFromParent(); - throw e; - } + ensureComponentIsUsableOrDetach(); // opened is a synchronized property, so pushing it through the // client path makes the GridContextMenuOpenedEvent report // isFromClient() as true, the way a real open does From 42d6964b90a80026600fd78da8ecadaf060f4dcd Mon Sep 17 00:00:00 2001 From: "totally-not-ai[bot]" <290682512+totally-not-ai[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:12:38 +0000 Subject: [PATCH 6/6] test: expect the visible Duplicated Hidden item in getItemTexts The getItemTexts expectation was written against the item list as it was before a visible "Duplicated Hidden" item was appended to ContextMenuView, so it asserted 9 items where the menu now shows 10. Both changes pass on their own, which is why neither pull request caught it; together the expectation is stale. The tester is right: the appended item is visible, so it belongs in the texts a user sees. Co-Authored-By: Claude Opus 5 (1M context) --- .../flow/component/contextmenu/ContextMenuTesterTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java b/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java index 03592bf0..dc38b20f 100644 --- a/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java +++ b/junit6/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTesterTest.java @@ -512,7 +512,8 @@ void getItemTexts_hiddenItemIgnored_componentItemHasNoText() { Assertions.assertIterableEquals( List.of("Foo", "Bar", "Text", "Duplicated", "Duplicated", "", - "Checkable", "Disabled", "Hierarchical"), + "Checkable", "Disabled", "Hierarchical", + "Duplicated Hidden"), menu_.getItemTexts(), "texts should be the visible items, in the order the browser shows them"); }