Don't center a window that has already been removed. (#11956)

* Don't center a window that has already been removed.

Fixes #11942

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 12ce828f4798417471c7a02218cbf9a301fd0878..9603310e22c31f1e8f8031559c2d8e61fc7aca38 100644 (file)
--- 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 (file)
index 0000000..5b600bc
--- /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 (file)
index 0000000..c37d444
--- /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<ButtonElement> buttons = $(ButtonElement.class).all();
+        int index = 1;
+        for (ButtonElement button : buttons) {
+            button.click();
+            List<NotificationElement> notifications = $(
+                    NotificationElement.class).all();
+            if (!notifications.isEmpty()) {
+                notifications.get(0).close();
+            }
+            assertEquals("Unexpected log contents,",
+                    index + ". closed " + index, getLogRow(0));
+            ++index;
+        }
+    }
+}