diff --git a/flow-server/src/main/java/com/vaadin/flow/component/ShortcutRegistration.java b/flow-server/src/main/java/com/vaadin/flow/component/ShortcutRegistration.java index bd74cd4d395..acbf83db0f8 100644 --- a/flow-server/src/main/java/com/vaadin/flow/component/ShortcutRegistration.java +++ b/flow-server/src/main/java/com/vaadin/flow/component/ShortcutRegistration.java @@ -92,6 +92,8 @@ public class ShortcutRegistration implements Registration, Serializable { if (listenOnComponents == null) { initListenOnComponent(); } + addListenOnDetachListeners(); + boolean reinit = false; /* * In PreserveOnRefersh case the UI instance is not detached immediately @@ -111,6 +113,7 @@ public class ShortcutRegistration implements Registration, Serializable { if (reinit) { removeAllListenerRegistrations(); initListenOnComponent(); + addListenOnDetachListeners(); } for (int i = 0; i < listenOnComponents.length; i++) { @@ -620,7 +623,7 @@ private void registerLifecycleOwner(Component owner) { // remove shortcut listener when detached Registration detachRegistration = owner - .addDetachListener(e -> removeListenerRegistration()); + .addDetachListener(e -> removeLifecycleOwnerRegistrations()); lifecycleRegistration = new CompoundRegistration(attachRegistration, detachRegistration); @@ -659,7 +662,7 @@ private Component[] registerOwnerListeners() { listenOnIndex))); listenOnAttachListenerRegistrations[i] .addRegistration(component.addDetachListener( - detachEvent -> removeListenerRegistration())); + detachEvent -> removeLifecycleOwnerRegistrations())); } // either the scope is an active UI, or the component is attached to @@ -679,13 +682,16 @@ private void removeAllListenerRegistrations() { } listenOnAttachListenerRegistrations = null; } + removeLifecycleOwnerRegistrations(); + listenOnComponents = null; + } + + private void removeListenOnDetachListeners() { registrations.forEach(Registration::remove); registrations.clear(); - removeListenerRegistration(); - listenOnComponents = null; } - private void removeListenerRegistration() { + private void removeLifecycleOwnerRegistrations() { if (shortcutListenerRegistrations != null) { for (CompoundRegistration shortcutListenerRegistration : shortcutListenerRegistrations) { if (shortcutListenerRegistration != null) @@ -694,6 +700,7 @@ private void removeListenerRegistration() { shortcutListenerRegistrations = null; } shortcutActive = false; + removeListenOnDetachListeners(); } private void queueBeforeExecutionCallback() { @@ -823,6 +830,12 @@ public String toString() { private void initListenOnComponent() { listenOnComponents = registerOwnerListeners(); + } + + private void addListenOnDetachListeners() { + if (!registrations.isEmpty()) { + return; + } for (Component component : listenOnComponents) { Registration registration = component.addDetachListener( event -> removeAllListenerRegistrations()); diff --git a/flow-server/src/test/java/com/vaadin/flow/component/ShortcutRegistrationTest.java b/flow-server/src/test/java/com/vaadin/flow/component/ShortcutRegistrationTest.java index 2377deb86d5..58059500910 100644 --- a/flow-server/src/test/java/com/vaadin/flow/component/ShortcutRegistrationTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/component/ShortcutRegistrationTest.java @@ -19,6 +19,7 @@ import java.util.Arrays; import java.util.List; import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import org.junit.Assert; @@ -514,6 +515,86 @@ public void constructedRegistration_lifecycleOnwerHasInvisibleParent_shorcutEven Assert.assertNull(event.get()); } + @Test + public void constructedRegistration_lifeCycleOwnerIsDetached_detachListenerIsDeregisteredFromListenOnComponents() { + AtomicReference detachListener = new AtomicReference<>(); + Mockito.doAnswer(invocaation -> { + detachListener.set( + invocaation.getArgument(0, ComponentEventListener.class)); + return mock(Registration.class); + }).when(lifecycleOwner).addDetachListener(any()); + + new ShortcutRegistration(lifecycleOwner, () -> listenOn, event -> { + }, Key.KEY_A); + + Registration registration = Mockito.mock(Registration.class); + for (Component component : listenOn) { + Mockito.when(component.addDetachListener(Mockito.any())) + .thenReturn(registration); + } + + clientResponse(); + + detachListener.get().onComponentEvent(new DetachEvent(lifecycleOwner)); + + Mockito.verify(registration, Mockito.times(3)).remove(); + } + + @Test + public void reattachComponent_detachListenerIsAddedOnEveryAttach_listenOnUIIsClosing_eventIsPopulatedForANewUI() { + UI ui = Mockito.spy(UI.class); + Component owner = new FakeComponent(); + + Registration registration = Mockito.mock(Registration.class); + AtomicInteger count = new AtomicInteger(); + Mockito.when(ui.addDetachListener(any())).thenAnswer(invocation -> { + count.incrementAndGet(); + return registration; + }); + + Component[] components = new Component[] { ui }; + + ui.add(owner); + new ShortcutRegistration(owner, () -> components, event -> { + }, Key.KEY_A); + + ArgumentCaptor captor = ArgumentCaptor + .forClass(SerializableConsumer.class); + verify(ui, atLeastOnce()).beforeClientResponse(eq(owner), + captor.capture()); + + SerializableConsumer consumer = captor.getValue(); + + // Fake beforeClientExecution call. + consumer.accept(mock(ExecutionContext.class)); + Assert.assertEquals(1, count.get()); + + ui.remove(owner); + + // reattach + ui.add(owner); + + // Fake beforeClientExecution call. + consumer.accept(mock(ExecutionContext.class)); + Assert.assertEquals(2, count.get()); + + UI newUI = Mockito.spy(UI.class); + // close the previous UI + ui.close(); + components[0] = newUI; + + owner.getElement().removeFromTree(); + newUI.add(owner); + + verify(newUI, atLeastOnce()).beforeClientResponse(eq(owner), + captor.capture()); + // Fake beforeClientExecution call. + captor.getValue().accept(mock(ExecutionContext.class)); + + // the new UI should now also have expression with KeyA + Assert.assertTrue(hasKeyAInKeyDownExpression(newUI)); + } + private Element mockLifecycle(boolean visible) { Mockito.when(lifecycleOwner.isVisible()).thenReturn(visible); Element element = ElementFactory.createAnchor();