Correctly track open windows and check modality.

Now checks the topmost modal window instead of randomly checking
some of the windows for modality (PopupPanel consumes events causing
some preview handlers not to be called). Now also correctly tracks open
windows so that closed windows will not be included in the windowOrder
array

2 files changed, 46 insertions, 18 deletions

diff --git a/src/com/vaadin/terminal/gwt/client/ui/VWindow.java b/src/com/vaadin/terminal/gwt/client/ui/VWindow.java
index 06f928168e..d3529987fa 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/VWindow.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VWindow.java
@@ -29,6 +29,7 @@ import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.terminal.gwt.client.ApplicationConnection;
 import com.vaadin.terminal.gwt.client.BrowserInfo;
 import com.vaadin.terminal.gwt.client.ComponentConnector;
+import com.vaadin.terminal.gwt.client.ConnectorMap;
 import com.vaadin.terminal.gwt.client.Console;
 import com.vaadin.terminal.gwt.client.EventId;
 import com.vaadin.terminal.gwt.client.Focusable;
@@ -174,7 +175,11 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner,
      * @return
      */
     private boolean isActive() {
-        return windowOrder.get(windowOrder.size() - 1).equals(this);
+        return equals(getTopmostWindow());
+    }
+
+    private static VWindow getTopmostWindow() {
+        return windowOrder.get(windowOrder.size() - 1);
     }
 
     void setWindowOrderAndPosition() {
@@ -360,6 +365,10 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner,
 
     @Override
     public void show() {
+        if (!windowOrder.contains(this)) {
+            windowOrder.add(this);
+        }
+
         if (vaadinModality) {
             showModalityCurtain();
         }
@@ -372,6 +381,8 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner,
             hideModalityCurtain();
         }
         super.hide();
+
+        windowOrder.remove(this);
     }
 
     void setVaadinModality(boolean modality) {
@@ -784,22 +795,34 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner,
         } else if (resizing) {
             onResizeEvent(event);
             return false;
-        } else if (vaadinModality) {
-            // return false when modal and outside window
-            final Element target = event.getEventTarget().cast();
+        }
+
+        // TODO This is probably completely unnecessary as the modality curtain
+        // prevents events from reaching other windows and any security check
+        // must be done on the server side and not here.
+        // The code here is also run many times as each VWindow has an event
+        // preview but we cannot check only the current VWindow here (e.g.
+        // if(isTopMost) {...}) because PopupPanel will cause all events that
+        // are not cancelled here and target this window to be consume():d
+        // meaning the event won't be sent to the rest of the preview handlers.
+
+        if (getTopmostWindow().vaadinModality) {
+            // Topmost window is modal. Cancel the event if it targets something
+            // outside that window (except debug console...)
             if (DOM.getCaptureElement() != null) {
                 // Allow events when capture is set
                 return true;
             }
 
-            if (!DOM.isOrHasChild(getElement(), target)) {
+            final Element target = event.getEventTarget().cast();
+            if (!DOM.isOrHasChild(getTopmostWindow().getElement(), target)) {
                 // not within the modal window, but let's see if it's in the
                 // debug window
                 Widget w = Util.findWidget(target, null);
                 while (w != null) {
                     if (w instanceof Console) {
                         return true; // allow debug-window clicks
-                    } else if (w instanceof ComponentConnector) {
+                    } else if (ConnectorMap.get(client).isConnector(w)) {
                         return false;
                     }
                     w = w.getParent();
diff --git a/tests/testbench/com/vaadin/tests/components/window/SubWindowOrder.html b/tests/testbench/com/vaadin/tests/components/window/SubWindowOrder.html
index 32928cdcd2..0476de6c35 100644
--- a/tests/testbench/com/vaadin/tests/components/window/SubWindowOrder.html
+++ b/tests/testbench/com/vaadin/tests/components/window/SubWindowOrder.html
@@ -65,10 +65,16 @@
 	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[2]/FocusableScrollPanel[0]/VVerticalLayout[0]/ChildComponentContainer[1]/VCssLayout[0]/VCssLayout$FlowPane[0]/VButton[1]/domChild[0]/domChild[0]</td>
 	<td></td>
 </tr>
-<!--Make dialog 5 modal and centered-->
+<!--Bring dialog 4 to front-->
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[2]/FocusableScrollPanel[0]/VVerticalLayout[0]/VCssLayout[0]/VCssLayout$FlowPane[0]/VButton[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<!--Make dialog 5 modal and centered. Dialog 4 still stays on top-->
 <tr>
 	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[3]/FocusableScrollPanel[0]/VVerticalLayout[0]/ChildComponentContainer[1]/VCssLayout[0]/VCssLayout$FlowPane[0]/VFilterSelect[0]/domChild[1]</td>
+	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[3]/FocusableScrollPanel[0]/VVerticalLayout[0]/VCssLayout[0]/VCssLayout$FlowPane[0]/VFilterSelect[0]/domChild[1]</td>
 	<td>11,11</td>
 </tr>
 <tr>
@@ -78,28 +84,27 @@
 </tr>
 <tr>
 	<td>click</td>
-	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[3]/FocusableScrollPanel[0]/VVerticalLayout[0]/ChildComponentContainer[1]/VCssLayout[0]/VCssLayout$FlowPane[0]/VButton[1]/domChild[0]/domChild[0]</td>
+	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[3]/FocusableScrollPanel[0]/VVerticalLayout[0]/VCssLayout[0]/VCssLayout$FlowPane[0]/VButton[1]/domChild[0]/domChild[0]</td>
 	<td></td>
 </tr>
-<!--Close window 5, which is not the topmost window (???)-->
+<!--Close window 4, which is the topmost window-->
 <tr>
 	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[4]/domChild[0]/domChild[0]/domChild[1]</td>
+	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[3]/domChild[0]/domChild[0]/domChild[1]</td>
 	<td>11,15</td>
 </tr>
-<!--The screenshot should really be ...-window-4-modal-->
 <tr>
 	<td>screenCapture</td>
 	<td></td>
-	<td>window5closed-window-5-modal</td>
+	<td>window4-closed-window-3-modal</td>
 </tr>
-<!--Close Dialog 4 (topmost)-->
+<!--Close Dialog 3 (topmost)-->
 <tr>
 	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[3]/domChild[0]/domChild[0]/domChild[1]</td>
+	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[2]/domChild[0]/domChild[0]/domChild[1]</td>
 	<td>6,8</td>
 </tr>
-<!--Make Dialog 3 (topmost) non-modal-->
+<!--Make Dialog 5 (topmost) non-modal-->
 <tr>
 	<td>mouseClick</td>
 	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[2]/FocusableScrollPanel[0]/VVerticalLayout[0]/ChildComponentContainer[1]/VCssLayout[0]/VCssLayout$FlowPane[0]/VFilterSelect[0]/domChild[1]</td>
@@ -107,7 +112,7 @@
 </tr>
 <tr>
 	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::Root/VFilterSelect$SuggestionPopup[0]/VFilterSelect$SuggestionMenu[0]#item3</td>
+	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::Root/VFilterSelect$SuggestionPopup[0]/VFilterSelect$SuggestionMenu[0]#item5</td>
 	<td>97,6</td>
 </tr>
 <tr>
@@ -131,7 +136,7 @@
 	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[2]/FocusableScrollPanel[0]/VVerticalLayout[0]/ChildComponentContainer[1]/VCssLayout[0]/VCssLayout$FlowPane[0]/VButton[0]/domChild[0]/domChild[0]</td>
 	<td></td>
 </tr>
-<!--Close dialog 3-->
+<!--Close dialog 5-->
 <tr>
 	<td>mouseClick</td>
 	<td>vaadin=runcomvaadintestscomponentswindowSubWindowOrder::/VWindow[2]/domChild[0]/domChild[0]/domChild[1]</td>