Prevent adding/setting parent component as child (#12720).

Change-Id: I9b7b43f176e88d9edca591001297b94475a31854

diff --git a/server/src/com/vaadin/ui/AbstractComponent.java b/server/src/com/vaadin/ui/AbstractComponent.java
index d980c2791676991c1e88a97c995d8b3003b74a2c..b6289e0b7d4c829560708e4a36b830dfb30a9d5b 100644 (file)
--- a/server/src/com/vaadin/ui/AbstractComponent.java
+++ b/server/src/com/vaadin/ui/AbstractComponent.java
@@ -991,4 +991,24 @@ public abstract class AbstractComponent extends AbstractClientConnector
             actionManager.removeAction(shortcut);
         }
     }
+
+    /**
+     * Determine whether a <code>content</code> component is equal to, or the
+     * ancestor of this component.
+     * 
+     * @param content
+     *            the potential ancestor element
+     * @return <code>true</code> if the relationship holds
+     */
+    protected boolean isOrHasAncestor(Component content) {
+        if (content instanceof HasComponents) {
+            for (Component parent = this; parent != null; parent = parent
+                    .getParent()) {
+                if (parent == content) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
 }

diff --git a/server/src/com/vaadin/ui/AbstractComponentContainer.java b/server/src/com/vaadin/ui/AbstractComponentContainer.java
index 0aa185d55739453c1cf285c531e9b00b7f9d7f1b..b1e69ba76bf91646a5b78125a83ab60da355446e 100644 (file)
--- a/server/src/com/vaadin/ui/AbstractComponentContainer.java
+++ b/server/src/com/vaadin/ui/AbstractComponentContainer.java
@@ -196,15 +196,10 @@ public abstract class AbstractComponentContainer extends AbstractComponent
      */
     @Override
     public void addComponent(Component c) {
-        if (c instanceof ComponentContainer) {
-            // Make sure we're not adding the component inside it's own content
-            for (Component parent = this; parent != null; parent = parent
-                    .getParent()) {
-                if (parent == c) {
-                    throw new IllegalArgumentException(
-                            "Component cannot be added inside it's own content");
-                }
-            }
+        // Make sure we're not adding the component inside it's own content
+        if (isOrHasAncestor(c)) {
+            throw new IllegalArgumentException(
+                    "Component cannot be added inside it's own content");
         }
 
         if (c.getParent() != null) {

diff --git a/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java b/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java
index de1bb298469865e687a57dcf7080cc5f7e47f7ee..ba108fc302fa812e92d8dfd62ddaf0892ec4af1f 100644 (file)
--- a/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java
+++ b/server/src/com/vaadin/ui/AbstractSingleComponentContainer.java
@@ -123,6 +123,12 @@ public abstract class AbstractSingleComponentContainer extends
      */
     @Override
     public void setContent(Component content) {
+        // Make sure we're not adding the component inside it's own content
+        if (isOrHasAncestor(content)) {
+            throw new IllegalArgumentException(
+                    "Component cannot be added inside it's own content");
+        }
+
         Component oldContent = getContent();
         if (oldContent == content) {
             // do not set the same content twice

diff --git a/server/tests/src/com/vaadin/tests/server/component/abstractcomponentcontainer/AddParentAsChild.java b/server/tests/src/com/vaadin/tests/server/component/abstractcomponentcontainer/AddParentAsChild.java
new file mode 100644 (file)
index 0000000..67e6d09
--- /dev/null
+++ b/server/tests/src/com/vaadin/tests/server/component/abstractcomponentcontainer/AddParentAsChild.java
@@ -0,0 +1,64 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.server.component.abstractcomponentcontainer;
+
+import java.util.Iterator;
+
+import org.easymock.EasyMock;
+import org.junit.Test;
+
+import com.vaadin.ui.AbstractComponentContainer;
+import com.vaadin.ui.Component;
+import com.vaadin.ui.HasComponents;
+
+/**
+ * Tests for avoiding add parent as child for
+ * {@link AbstractComponentContainer#addComponent(Component)}
+ * 
+ * @author Vaadin Ltd
+ */
+public class AddParentAsChild {
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testAddComponent() {
+        AbstractComponentContainer container = new ComponentContainer();
+        HasComponents hasComponentsMock = EasyMock
+                .createMock(HasComponents.class);
+        container.setParent(hasComponentsMock);
+
+        container.addComponent(hasComponentsMock);
+    }
+
+    class ComponentContainer extends AbstractComponentContainer {
+
+        @Override
+        public void replaceComponent(Component oldComponent,
+                Component newComponent) {
+        }
+
+        @Override
+        public int getComponentCount() {
+            return 0;
+        }
+
+        @Override
+        public Iterator<Component> iterator() {
+            return null;
+        }
+
+    }
+
+}

diff --git a/server/tests/src/com/vaadin/tests/server/component/abstractsinglecomponentcontainer/SetParentAsContent.java b/server/tests/src/com/vaadin/tests/server/component/abstractsinglecomponentcontainer/SetParentAsContent.java
new file mode 100644 (file)
index 0000000..dae0e57
--- /dev/null
+++ b/server/tests/src/com/vaadin/tests/server/component/abstractsinglecomponentcontainer/SetParentAsContent.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.server.component.abstractsinglecomponentcontainer;
+
+import org.easymock.EasyMock;
+import org.junit.Test;
+
+import com.vaadin.ui.AbstractSingleComponentContainer;
+import com.vaadin.ui.Component;
+import com.vaadin.ui.HasComponents;
+
+/**
+ * 
+ * Tests for avoiding set parent as child for
+ * {@link AbstractSingleComponentContainer#setContent(Component)}
+ * 
+ * @author Vaadin Ltd
+ */
+public class SetParentAsContent {
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testSetContent() {
+        AbstractSingleComponentContainer container = new AbstractSingleComponentContainer() {
+        };
+        HasComponents hasComponentsMock = EasyMock
+                .createMock(HasComponents.class);
+        container.setParent(hasComponentsMock);
+
+        container.setContent(hasComponentsMock);
+    }
+
+}