From 146924c65421cab4779c353706d477b45f63a433 Mon Sep 17 00:00:00 2001
From: Henri Sara <henri.sara@itmill.com>
Date: Thu, 28 Oct 2010 08:58:56 +0000
Subject: [PATCH] #3865 closing sub-windows from code should send close event,
 test for window closing

svn changeset:15758/svn branch:6.5
---
 WebContent/release-notes.html                 |   3 +
 src/com/vaadin/ui/Window.java                 |  21 ++--
 .../components/window/CloseSubWindow.html     |  87 +++++++++++++++
 .../components/window/CloseSubWindow.java     | 103 ++++++++++++++++++
 4 files changed, 204 insertions(+), 10 deletions(-)
 create mode 100644 tests/src/com/vaadin/tests/components/window/CloseSubWindow.html
 create mode 100644 tests/src/com/vaadin/tests/components/window/CloseSubWindow.java

diff --git a/WebContent/release-notes.html b/WebContent/release-notes.html
index 308b1417d0..727eaac791 100644
--- a/WebContent/release-notes.html
+++ b/WebContent/release-notes.html
@@ -51,6 +51,9 @@ contains a number of significant enhancements.</p>
 
 <ul>
   <li>Table.setColumnCollapsed throws IllegalStateException instead of IllegalAccessException is collapsing is not allowed.</li>
+  <li>Window.removeWindow(Window) now triggers window close listeners also when called explicitly.
+      See <a href="http://dev.vaadin.com/ticket/3865">#3865</a> for more info.
+  </li>
 </ul>
 
 <ul>
diff --git a/src/com/vaadin/ui/Window.java b/src/com/vaadin/ui/Window.java
index fde6058f93..fed7dabd36 100644
--- a/src/com/vaadin/ui/Window.java
+++ b/src/com/vaadin/ui/Window.java
@@ -1102,11 +1102,9 @@ public class Window extends Panel implements URIHandler, ParameterHandler {
      * </p>
      * 
      * <p>
-     * If one wants change the default behavior, register a window close
-     * listenter and do something else. For example, you could re-open the
-     * browser-level window with mainWindow.open(), re-add the removed
-     * sub-window back to its parent or remove browser-level window
-     * automatically from the application.
+     * To explicitly close a sub-window, use {@link #removeWindow(Window)}. To
+     * react to a window being closed (after it is closed), register a
+     * {@link CloseListener}.
      * </p>
      */
     protected void close() {
@@ -1116,7 +1114,6 @@ public class Window extends Panel implements URIHandler, ParameterHandler {
         } else {
             // subwindow is removed from parent
             parent.removeWindow(this);
-            fireClose();
         }
     }
 
@@ -1250,8 +1247,8 @@ public class Window extends Panel implements URIHandler, ParameterHandler {
      * user closes the window.
      * 
      * <p>
-     * Note that removing windows using {@link #removeWindow(Window)} will not
-     * fire the CloseListener.
+     * Since Vaadin 6.5, removing windows using {@link #removeWindow(Window)}
+     * does fire the CloseListener.
      * </p>
      */
     public interface CloseListener extends Serializable {
@@ -1277,8 +1274,8 @@ public class Window extends Panel implements URIHandler, ParameterHandler {
      * mean it will be destroyed.
      * 
      * <p>
-     * Note that removing windows using {@link #removeWindow(Window)} will not
-     * fire the CloseListener.
+     * Since Vaadin 6.5, removing windows using {@link #removeWindow(Window)}
+     * does fire the CloseListener.
      * </p>
      * 
      * @param listener
@@ -1431,12 +1428,16 @@ public class Window extends Panel implements URIHandler, ParameterHandler {
     /**
      * Remove the given subwindow from this window.
      * 
+     * Since Vaadin 6.5, {@link CloseListener}s are called also when explicitly
+     * removing a window by calling this method.
+     * 
      * @param window
      *            Window to be removed.
      */
     public void removeWindow(Window window) {
         subwindows.remove(window);
         window.setParent(null);
+        window.fireClose();
         requestRepaint();
 
     }
diff --git a/tests/src/com/vaadin/tests/components/window/CloseSubWindow.html b/tests/src/com/vaadin/tests/components/window/CloseSubWindow.html
new file mode 100644
index 0000000000..d4d404fbe6
--- /dev/null
+++ b/tests/src/com/vaadin/tests/components/window/CloseSubWindow.html
@@ -0,0 +1,87 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head profile="http://selenium-ide.openqa.org/profiles/test-case">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+<link rel="selenium.base" href="http://localhost:8888/" />
+<title>CloseSubWindow</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">CloseSubWindow</td></tr>
+</thead><tbody>
+<tr>
+	<td>open</td>
+	<td>/run/com.vaadin.tests.components.window.CloseSubWindow</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForVaadin</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentswindowCloseSubWindow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentswindowCloseSubWindow::/VWindow[0]/ScrollPanel[0]/VVerticalLayout[0]/ChildComponentContainer[0]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>closeNotification</td>
+	<td>//body/div[2]</td>
+	<td>0,0</td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentswindowCloseSubWindow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentswindowCloseSubWindow::/VWindow[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]</td>
+	<td>4,4</td>
+</tr>
+<tr>
+	<td>closeNotification</td>
+	<td>//body/div[2]</td>
+	<td>0,0</td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentswindowCloseSubWindow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>waitForPopUp</td>
+	<td>nativewindow</td>
+	<td>30000</td>
+</tr>
+<tr>
+	<td>selectWindow</td>
+	<td>name=nativewindow</td>
+	<td></td>
+</tr>
+<tr>
+	<td>close</td>
+	<td></td>
+	<td></td>
+</tr>
+<tr>
+	<td>selectWindow</td>
+	<td>title=com.vaadin.tests.components.window.CloseSubWindow</td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentswindowCloseSubWindow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[2]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+
+</tbody></table>
+</body>
+</html>
diff --git a/tests/src/com/vaadin/tests/components/window/CloseSubWindow.java b/tests/src/com/vaadin/tests/components/window/CloseSubWindow.java
new file mode 100644
index 0000000000..fc8f20f1b9
--- /dev/null
+++ b/tests/src/com/vaadin/tests/components/window/CloseSubWindow.java
@@ -0,0 +1,103 @@
+package com.vaadin.tests.components.window;
+
+import java.net.URL;
+
+import com.vaadin.terminal.ExternalResource;
+import com.vaadin.tests.components.TestBase;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.Window;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Button.ClickListener;
+import com.vaadin.ui.Window.CloseEvent;
+import com.vaadin.ui.Window.CloseListener;
+
+public class CloseSubWindow extends TestBase {
+
+    private Window browserWindow;
+
+    @Override
+    protected void setup() {
+        Button openWindowButton = new Button("Open sub-window");
+        openWindowButton.addListener(new ClickListener() {
+            public void buttonClick(ClickEvent event) {
+                Window sub = createClosableSubWindow("Sub-window");
+                getMainWindow().addWindow(sub);
+            }
+        });
+
+        addComponent(openWindowButton);
+
+        Button openBrowserWindowButton = new Button("Open browser window");
+        openBrowserWindowButton.addListener(new ClickListener() {
+            public void buttonClick(ClickEvent event) {
+                browserWindow = new Window("Window");
+                browserWindow.addComponent(new Label("Close this window"));
+
+                browserWindow.addListener(new CloseListener() {
+                    public void windowClose(CloseEvent e) {
+                        getMainWindow().showNotification(
+                                "Browser window closed");
+                        // there is no push, so the user needs to click a button
+                        // to see the notification
+
+                        // Opera does not send a notification about the window
+                        // having been closed
+                    }
+                });
+
+                addWindow(browserWindow);
+                URL windowUrl = browserWindow.getURL();
+                // named for easier access by test tools
+                getMainWindow().open(new ExternalResource(windowUrl),
+                        "nativewindow");
+            }
+        });
+
+        addComponent(openBrowserWindowButton);
+
+        addComponent(new Button("Poll server"));
+    }
+
+    private Window createClosableSubWindow(final String title) {
+        final Window window = new Window(title);
+        window.setSizeUndefined();
+        window.getContent().setSizeFull();
+        window.setClosable(true);
+
+        Button closeButton = new Button("Close");
+        closeButton.addListener(new ClickListener() {
+            public void buttonClick(ClickEvent event) {
+                window.getParent().removeWindow(window);
+            }
+        });
+        window.addComponent(closeButton);
+
+        Button removeButton = new Button("Remove from parent");
+        removeButton.addListener(new ClickListener() {
+            public void buttonClick(ClickEvent event) {
+                window.getParent().removeWindow(window);
+            }
+        });
+        window.addComponent(closeButton);
+
+        window.addListener(new CloseListener() {
+            public void windowClose(CloseEvent e) {
+                getMainWindow().showNotification(title + " closed");
+            }
+        });
+
+        return window;
+    }
+
+    @Override
+    protected String getDescription() {
+        return "Close sub-windows both from code and with the close button in the window title bar, and check for close events.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 3865;
+    }
+
+}
-- 
2.39.5