From fb5e0c2fe112908a1390696a2d415583c94f524b Mon Sep 17 00:00:00 2001 From: Anna Koskinen Date: Fri, 24 Apr 2020 11:26:04 +0300 Subject: [PATCH] Don't center a window that has already been removed. (#11956) * Don't center a window that has already been removed. Fixes #11942 --- .../client/ui/window/WindowConnector.java | 10 +- .../components/window/CloseWindowAsync.java | 122 ++++++++++++++++++ .../window/CloseWindowAsyncTest.java | 32 +++++ 3 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/components/window/CloseWindowAsync.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/window/CloseWindowAsyncTest.java diff --git a/client/src/main/java/com/vaadin/client/ui/window/WindowConnector.java b/client/src/main/java/com/vaadin/client/ui/window/WindowConnector.java index 12ce828f47..9603310e22 100644 --- a/client/src/main/java/com/vaadin/client/ui/window/WindowConnector.java +++ b/client/src/main/java/com/vaadin/client/ui/window/WindowConnector.java @@ -433,11 +433,17 @@ public class WindowConnector extends AbstractSingleComponentContainerConnector // This had to be here because we might not know the content size before // everything is painted into the window - // centered is this is unset on move/resize + // centered if this is unset on move/resize window.centered = state.centered; // Ensure centering before setting visible (#16486) if (window.centered && getState().windowMode != WindowMode.MAXIMIZED) { - Scheduler.get().scheduleFinally(() -> getWidget().center()); + Scheduler.get().scheduleFinally(() -> { + // the window may have got removed again before this is + // triggered, and centering would re-display it + if (getWidget().isShowing()) { + getWidget().center(); + } + }); } window.setVisible(true); } diff --git a/uitest/src/main/java/com/vaadin/tests/components/window/CloseWindowAsync.java b/uitest/src/main/java/com/vaadin/tests/components/window/CloseWindowAsync.java new file mode 100644 index 0000000000..5b600bc306 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/window/CloseWindowAsync.java @@ -0,0 +1,122 @@ +package com.vaadin.tests.components.window; + +import java.util.concurrent.CompletableFuture; + +import com.vaadin.annotations.PreserveOnRefresh; +import com.vaadin.annotations.Push; +import com.vaadin.annotations.Theme; +import com.vaadin.navigator.PushStateNavigation; +import com.vaadin.server.VaadinRequest; +import com.vaadin.shared.communication.PushMode; +import com.vaadin.shared.ui.ui.Transport; +import com.vaadin.tests.components.AbstractTestUIWithLog; +import com.vaadin.ui.Button; +import com.vaadin.ui.Label; +import com.vaadin.ui.Notification; +import com.vaadin.ui.UI; +import com.vaadin.ui.UIDetachedException; +import com.vaadin.ui.Window; + +@Theme("tests-valo-disabled-animations") +@PreserveOnRefresh +@Push(value = PushMode.MANUAL, transport = Transport.LONG_POLLING) +@PushStateNavigation +public class CloseWindowAsync extends AbstractTestUIWithLog { + private final boolean manualPush; + + public CloseWindowAsync() { + this(true); + } + + public CloseWindowAsync(boolean manualPush) { + this.manualPush = manualPush; + } + + @Override + protected void setup(VaadinRequest request) { + final Button button = new Button("Open and directly close busy window"); + button.addClickListener((Button.ClickListener) event -> { + final Window window = createWindow(1); + final UI ui = getUI(); + ui.addWindow(window); + + CompletableFuture.runAsync(() -> { + // task duration variable, could be a few ms or longer + ui.accessSynchronously(() -> { + window.close(); + if (manualPush) { + ui.push(); + } + }); + }); + }); + addComponents(button); + + final Button button2 = new Button( + "Open and directly close busy window with error notification"); + button2.addClickListener((Button.ClickListener) event -> { + final Window window = createWindow(2); + final UI ui = getUI(); + ui.addWindow(window); + + CompletableFuture.runAsync(() -> { + // task duration variable, could be a few ms or longer + ui.accessSynchronously(() -> { + window.close(); + Notification.show("error", Notification.Type.ERROR_MESSAGE); + if (manualPush) { + ui.push(); + } + }); + }); + }); + addComponents(button2); + + // Reconstructed the issue using the vaadin push training + // https://vaadin.com/learn/training/vaadin-push + + final Button button3 = new Button( + "Open and directly close busy window (vaadin push training)"); + button3.addClickListener((Button.ClickListener) event -> { + final Window window = createWindow(3); + final UI ui = getUI(); + ui.addWindow(window); + + CompletableFuture.runAsync(() -> { + // task duration variable, could be a few ms or longer + try { + ui.access(() -> { + ui.removeWindow(window); + if (manualPush) { + ui.push(); + } + }); + } catch (UIDetachedException e) { + // browser closed + } + }); + }); + addComponents(button3); + } + + private Window createWindow(int index) { + final Window window = new Window(); + window.setCaption("Window"); + window.setWidth(30, Unit.PERCENTAGE); + window.setHeight(30, Unit.PERCENTAGE); + window.setModal(true); + window.setContent(new Label("Window content")); + window.addCloseListener(e -> log("closed " + index)); + return window; + } + + @Override + protected Integer getTicketNumber() { + return 11942; + } + + @Override + protected String getTestDescription() { + return "All buttons should successfully open and close window on all browsers."; + } +} \ No newline at end of file diff --git a/uitest/src/test/java/com/vaadin/tests/components/window/CloseWindowAsyncTest.java b/uitest/src/test/java/com/vaadin/tests/components/window/CloseWindowAsyncTest.java new file mode 100644 index 0000000000..c37d44430d --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/window/CloseWindowAsyncTest.java @@ -0,0 +1,32 @@ +package com.vaadin.tests.components.window; + +import static org.junit.Assert.assertEquals; + +import java.util.List; + +import org.junit.Test; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.NotificationElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class CloseWindowAsyncTest extends MultiBrowserTest { + + @Test + public void testOpeningAndClosing() throws Exception { + openTestURL(); + List buttons = $(ButtonElement.class).all(); + int index = 1; + for (ButtonElement button : buttons) { + button.click(); + List notifications = $( + NotificationElement.class).all(); + if (!notifications.isEmpty()) { + notifications.get(0).close(); + } + assertEquals("Unexpected log contents,", + index + ". closed " + index, getLogRow(0)); + ++index; + } + } +} -- 2.39.5