Skip to content

Commit a883c7c

Browse files
vaadin-botsissbruecker
vaadin-bot
and
sissbruecker
authored
fix: cancel connector init if target is detached in beforeClientResponse (#7289) (#7298)
* fix: cancel connector init if target is detached * remove obsolete test Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
1 parent 82a2cd1 commit a883c7c

3 files changed

Lines changed: 187 additions & 49 deletions

File tree

‎vaadin-context-menu-flow-parent/vaadin-context-menu-flow/src/main/java/com/vaadin/flow/component/contextmenu/ContextMenuBase.java‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ public abstract class ContextMenuBase<C extends ContextMenuBase<C, I, S>, I exte
7171
private String openOnEventName = "vaadin-contextmenu";
7272
private Registration targetBeforeOpenRegistration;
7373
private Registration targetAttachRegistration;
74+
private Registration targetDetachRegistration;
7475
private PendingJavaScriptResult targetJsRegistration;
7576

7677
private boolean autoAddedToTheUi;
@@ -120,12 +121,10 @@ public void setTarget(Component target) {
120121
if (getTarget() != null) {
121122
targetBeforeOpenRegistration.remove();
122123
targetAttachRegistration.remove();
124+
targetDetachRegistration.remove();
123125
getTarget().getElement().executeJs(
124126
"if (this.$contextMenuTargetConnector) { this.$contextMenuTargetConnector.removeConnector() }");
125-
if (isTargetJsPending()) {
126-
targetJsRegistration.cancelExecution();
127-
targetJsRegistration = null;
128-
}
127+
cancelTargetJavascriptExecution();
129128
}
130129

131130
this.target = target;
@@ -142,6 +141,8 @@ public void setTarget(Component target) {
142141
target.getUI().ifPresent(this::onTargetAttach);
143142
targetAttachRegistration = target
144143
.addAttachListener(e -> onTargetAttach(e.getUI()));
144+
targetDetachRegistration = target
145+
.addDetachListener(e -> cancelTargetJavascriptExecution());
145146

146147
// Server round-trip before opening the overlay
147148
targetBeforeOpenRegistration = target.getElement()
@@ -424,19 +425,20 @@ private void onTargetAttach(UI ui) {
424425
*/
425426
private void requestTargetJsExecutions() {
426427
if (target != null) {
427-
if (isTargetJsPending()) {
428-
targetJsRegistration.cancelExecution();
429-
}
428+
cancelTargetJavascriptExecution();
430429
targetJsRegistration = target.getElement().executeJs(
431430
"window.Vaadin.Flow.contextMenuTargetConnector.init(this);"
432431
+ "this.$contextMenuTargetConnector.updateOpenOn($0);",
433432
openOnEventName);
434433
}
435434
}
436435

437-
private boolean isTargetJsPending() {
438-
return targetJsRegistration != null
439-
&& !targetJsRegistration.isSentToBrowser();
436+
private void cancelTargetJavascriptExecution() {
437+
if (targetJsRegistration != null
438+
&& !targetJsRegistration.isSentToBrowser()) {
439+
targetJsRegistration.cancelExecution();
440+
}
441+
targetJsRegistration = null;
440442
}
441443

442444
private void beforeOpenHandler(DomEvent event) {

‎vaadin-context-menu-flow-parent/vaadin-context-menu-flow/src/test/java/com/vaadin/flow/component/contextmenu/ContextMenuTargetTest.java‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
/*
2+
* Copyright 2000-2025 Vaadin Ltd.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
5+
* use this file except in compliance with the License. You may obtain a copy of
6+
* the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations under
14+
* the License.
15+
*/
16+
package com.vaadin.flow.component.contextmenu;
17+
18+
import java.util.List;
19+
20+
import org.junit.Assert;
21+
import org.junit.Before;
22+
import org.junit.Test;
23+
import org.mockito.Mockito;
24+
25+
import com.vaadin.flow.component.Component;
26+
import com.vaadin.flow.component.UI;
27+
import com.vaadin.flow.component.html.Div;
28+
import com.vaadin.flow.component.internal.PendingJavaScriptInvocation;
29+
import com.vaadin.flow.function.DeploymentConfiguration;
30+
import com.vaadin.flow.server.VaadinContext;
31+
import com.vaadin.flow.server.VaadinService;
32+
import com.vaadin.flow.server.VaadinSession;
33+
34+
public class ContextMenuTargetTest {
35+
private final UI ui = new UI();
36+
private final ContextMenu menu = new ContextMenu();
37+
38+
@Before
39+
public void setup() {
40+
var mockSession = Mockito.mock(VaadinSession.class);
41+
var mockService = Mockito.mock(VaadinService.class);
42+
var mockContext = Mockito.mock(VaadinContext.class);
43+
var mockConfiguration = Mockito.mock(DeploymentConfiguration.class);
44+
45+
Mockito.when(mockSession.getService()).thenReturn(mockService);
46+
Mockito.when(mockSession.getConfiguration())
47+
.thenReturn(mockConfiguration);
48+
Mockito.when(mockService.getContext()).thenReturn(mockContext);
49+
50+
ui.getInternals().setSession(mockSession);
51+
}
52+
53+
@Test
54+
public void setTarget_attachTarget_initTargetConnector() {
55+
var target = new Div();
56+
menu.setTarget(target);
57+
58+
ui.add(target);
59+
60+
assertTargetConnectorInit(getPendingInvocations(), target);
61+
}
62+
63+
@Test
64+
public void attachTarget_setTarget_initTargetConnector() {
65+
var target = new Div();
66+
ui.add(target);
67+
68+
menu.setTarget(target);
69+
70+
assertTargetConnectorInit(getPendingInvocations(), target);
71+
}
72+
73+
@Test
74+
public void attachTarget_detachTargetInBeforeClientResponse_cancelTargetConnectorInit() {
75+
var target = new Div();
76+
menu.setTarget(target);
77+
78+
ui.add(target);
79+
ui.beforeClientResponse(target, context -> ui.remove(target));
80+
81+
assertNoTargetConnectorInit(getPendingInvocations());
82+
}
83+
84+
@Test
85+
public void clearTarget_removeTargetConnector() {
86+
var target = new Div();
87+
ui.add(target);
88+
89+
menu.setTarget(target);
90+
91+
assertTargetConnectorInit(getPendingInvocations(), target);
92+
93+
menu.setTarget(null);
94+
95+
assertTargetConnectorRemove(getPendingInvocations(), target);
96+
}
97+
98+
@Test
99+
public void replaceTarget_updateTargetConnector() {
100+
var target = new Div();
101+
ui.add(target);
102+
103+
menu.setTarget(target);
104+
105+
assertTargetConnectorInit(getPendingInvocations(), target);
106+
107+
var newTarget = new Div();
108+
ui.add(newTarget);
109+
110+
menu.setTarget(newTarget);
111+
112+
var invocations = getPendingInvocations();
113+
assertTargetConnectorInit(invocations, newTarget);
114+
assertTargetConnectorRemove(invocations, target);
115+
}
116+
117+
private void assertTargetConnectorInit(
118+
List<PendingJavaScriptInvocation> invocations, Component target) {
119+
var initInvocations = filterTargetConnectorInitInvocations(invocations);
120+
121+
Assert.assertEquals(1, initInvocations.size());
122+
123+
var invocation = initInvocations.get(0);
124+
Assert.assertEquals(target.getElement().getNode(),
125+
invocation.getOwner());
126+
Assert.assertEquals(
127+
"return (async function() { window.Vaadin.Flow.contextMenuTargetConnector.init(this);this.$contextMenuTargetConnector.updateOpenOn($0);}).apply($1)",
128+
invocation.getInvocation().getExpression());
129+
}
130+
131+
private void assertNoTargetConnectorInit(
132+
List<PendingJavaScriptInvocation> invocations) {
133+
var initInvocations = filterTargetConnectorInitInvocations(invocations);
134+
135+
Assert.assertEquals(0, initInvocations.size());
136+
}
137+
138+
private void assertTargetConnectorRemove(
139+
List<PendingJavaScriptInvocation> invocations, Component target) {
140+
var removeInvocations = filterTargetConnectorRemoveInvocations(
141+
invocations);
142+
143+
Assert.assertEquals(1, removeInvocations.size());
144+
145+
var invocation = removeInvocations.get(0);
146+
Assert.assertEquals(target.getElement().getNode(),
147+
invocation.getOwner());
148+
Assert.assertEquals(
149+
"return (async function() { if (this.$contextMenuTargetConnector) { this.$contextMenuTargetConnector.removeConnector() }}).apply($0)",
150+
invocation.getInvocation().getExpression());
151+
}
152+
153+
private List<PendingJavaScriptInvocation> filterTargetConnectorInitInvocations(
154+
List<PendingJavaScriptInvocation> invocations) {
155+
return invocations.stream()
156+
.filter(invocation -> invocation.getInvocation().getExpression()
157+
.contains("contextMenuTargetConnector.init"))
158+
.toList();
159+
}
160+
161+
private List<PendingJavaScriptInvocation> filterTargetConnectorRemoveInvocations(
162+
List<PendingJavaScriptInvocation> invocations) {
163+
return invocations.stream()
164+
.filter(invocation -> invocation.getInvocation().getExpression()
165+
.contains("contextMenuTargetConnector.removeConnector"))
166+
.toList();
167+
}
168+
169+
protected List<PendingJavaScriptInvocation> getPendingInvocations() {
170+
ui.getInternals().getStateTree().runExecutionsBeforeClientResponse();
171+
ui.getInternals().getStateTree().collectChanges(ignore -> {
172+
});
173+
return ui.getInternals().dumpPendingJavaScriptInvocations();
174+
}
175+
}

‎vaadin-grid-flow-parent/vaadin-grid-flow/src/test/java/com/vaadin/flow/component/grid/contextmenu/GridContextMenuTest.java‎

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
*/
1616
package com.vaadin.flow.component.grid.contextmenu;
1717

18-
import java.util.Optional;
19-
2018
import org.junit.Assert;
2119
import org.junit.Test;
2220
import org.mockito.Mockito;
@@ -28,11 +26,7 @@
2826
import com.vaadin.flow.component.contextmenu.SubMenu;
2927
import com.vaadin.flow.component.grid.Grid;
3028
import com.vaadin.flow.component.html.NativeButton;
31-
import com.vaadin.flow.dom.DomListenerRegistration;
32-
import com.vaadin.flow.dom.Element;
3329
import com.vaadin.flow.function.SerializableRunnable;
34-
import com.vaadin.flow.internal.StateNode;
35-
import com.vaadin.flow.shared.Registration;
3630

3731
public class GridContextMenuTest {
3832

@@ -89,37 +83,4 @@ public void setTarget_targetIsGrid_getterReturnsSetTarget() {
8983

9084
Assert.assertEquals(grid, gridContextMenu.getTarget());
9185
}
92-
93-
@Test
94-
public void setTarget_nullTarget_connectorIsRemovedFromPreviousTarget() {
95-
Grid grid = Mockito.mock(Grid.class);
96-
Element element = Mockito.mock(Element.class);
97-
StateNode node = Mockito.mock(StateNode.class);
98-
Mockito.when(grid.getElement()).thenReturn(element);
99-
Mockito.when(element.getNode()).thenReturn(node);
100-
101-
DomListenerRegistration registration = Mockito
102-
.mock(DomListenerRegistration.class);
103-
Mockito.when(
104-
element.addEventListener(Mockito.anyString(), Mockito.any()))
105-
.thenReturn(registration);
106-
107-
Mockito.when(registration.addEventData(Mockito.anyString()))
108-
.thenReturn(registration);
109-
110-
Mockito.when(grid.getUI()).thenReturn(Optional.empty());
111-
112-
Registration attachRegistration = Mockito.mock(Registration.class);
113-
Mockito.when(grid.addAttachListener(Mockito.any()))
114-
.thenReturn(attachRegistration);
115-
116-
GridContextMenu gridContextMenu = new GridContextMenu<>();
117-
gridContextMenu.setTarget(grid);
118-
119-
gridContextMenu.setTarget(null);
120-
121-
Mockito.verify(registration).remove();
122-
Mockito.verify(element).executeJs(
123-
"if (this.$contextMenuTargetConnector) { this.$contextMenuTargetConnector.removeConnector() }");
124-
}
12586
}

0 commit comments

Comments
 (0)