#7668 fix and tests merged from 6.7

svn changeset:22188/svn branch:6.8

diff --git a/src/com/vaadin/ui/AbstractOrderedLayout.java b/src/com/vaadin/ui/AbstractOrderedLayout.java
index 0b0fff2a3463649081b95f73d16ff2fd013cee80..68c5ace0fc024b4630497ef2b915e40f39198dcb 100644 (file)
--- a/src/com/vaadin/ui/AbstractOrderedLayout.java
+++ b/src/com/vaadin/ui/AbstractOrderedLayout.java
@@ -53,6 +53,8 @@ public abstract class AbstractOrderedLayout extends AbstractLayout implements
      */
     @Override
     public void addComponent(Component c) {
+        // Add to components before calling super.addComponent
+        // so that it is available to AttachListeners
         components.add(c);
         try {
             super.addComponent(c);
@@ -71,6 +73,11 @@ public abstract class AbstractOrderedLayout extends AbstractLayout implements
      *            the component to be added.
      */
     public void addComponentAsFirst(Component c) {
+        // If c is already in this, we must remove it before proceeding
+        // see ticket #7668
+        if (c.getParent() == this) {
+            removeComponent(c);
+        }
         components.addFirst(c);
         try {
             super.addComponent(c);
@@ -87,10 +94,19 @@ public abstract class AbstractOrderedLayout extends AbstractLayout implements
      * @param c
      *            the component to be added.
      * @param index
-     *            the Index of the component position. The components currently
+     *            the index of the component position. The components currently
      *            in and after the position are shifted forwards.
      */
     public void addComponent(Component c, int index) {
+        // If c is already in this, we must remove it before proceeding
+        // see ticket #7668
+        if (c.getParent() == this) {
+            // When c is removed, all components after it are shifted down
+            if (index > getComponentIndex(c)) {
+                index--;
+            }
+            removeComponent(c);
+        }
         components.add(index, c);
         try {
             super.addComponent(c);

diff --git a/src/com/vaadin/ui/CssLayout.java b/src/com/vaadin/ui/CssLayout.java
index 5789a65ed3886a2fe044df114c9af505d70103e0..78474d33fe51ae90a841f6e03f4490377e73a98e 100644 (file)
--- a/src/com/vaadin/ui/CssLayout.java
+++ b/src/com/vaadin/ui/CssLayout.java
@@ -76,6 +76,8 @@ public class CssLayout extends AbstractLayout implements LayoutClickNotifier {
      */
     @Override
     public void addComponent(Component c) {
+        // Add to components before calling super.addComponent
+        // so that it is available to AttachListeners
         components.add(c);
         try {
             super.addComponent(c);
@@ -94,6 +96,11 @@ public class CssLayout extends AbstractLayout implements LayoutClickNotifier {
      *            the component to be added.
      */
     public void addComponentAsFirst(Component c) {
+        // If c is already in this, we must remove it before proceeding
+        // see ticket #7668
+        if (c.getParent() == this) {
+            removeComponent(c);
+        }
         components.addFirst(c);
         try {
             super.addComponent(c);
@@ -110,10 +117,19 @@ public class CssLayout extends AbstractLayout implements LayoutClickNotifier {
      * @param c
      *            the component to be added.
      * @param index
-     *            the Index of the component position. The components currently
+     *            the index of the component position. The components currently
      *            in and after the position are shifted forwards.
      */
     public void addComponent(Component c, int index) {
+        // If c is already in this, we must remove it before proceeding
+        // see ticket #7668
+        if (c.getParent() == this) {
+            // When c is removed, all components after it are shifted down
+            if (index > components.indexOf(c)) {
+                index--;
+            }
+            removeComponent(c);
+        }
         components.add(index, c);
         try {
             super.addComponent(c);

diff --git a/tests/server-side/com/vaadin/tests/server/component/abstractorderedlayout/AddComponentsTest.java b/tests/server-side/com/vaadin/tests/server/component/abstractorderedlayout/AddComponentsTest.java
new file mode 100644 (file)
index 0000000..ba5ea62
--- /dev/null
+++ b/tests/server-side/com/vaadin/tests/server/component/abstractorderedlayout/AddComponentsTest.java
@@ -0,0 +1,115 @@
+package com.vaadin.tests.server.component.abstractorderedlayout;\r
+\r
+import static org.junit.Assert.assertFalse;\r
+import static org.junit.Assert.assertSame;\r
+import static org.junit.Assert.fail;\r
+\r
+import java.util.Iterator;\r
+import java.util.NoSuchElementException;\r
+\r
+import org.junit.Test;\r
+\r
+import com.vaadin.ui.AbstractOrderedLayout;\r
+import com.vaadin.ui.Component;\r
+import com.vaadin.ui.HorizontalLayout;\r
+import com.vaadin.ui.Label;\r
+import com.vaadin.ui.Layout;\r
+import com.vaadin.ui.VerticalLayout;\r
+\r
+public class AddComponentsTest {\r
+\r
+    Component[] children = new Component[] { new Label("A"), new Label("B"),\r
+            new Label("C"), new Label("D") };\r
+\r
+    @Test\r
+    public void moveComponentsBetweenLayouts() {\r
+        AbstractOrderedLayout layout1 = new HorizontalLayout();\r
+        AbstractOrderedLayout layout2 = new VerticalLayout();\r
+\r
+        layout1.addComponent(children[0]);\r
+        layout1.addComponent(children[1]);\r
+\r
+        layout2.addComponent(children[2]);\r
+        layout2.addComponent(children[3]);\r
+\r
+        layout2.addComponent(children[1], 1);\r
+        assertOrder(layout1, new int[] { 0 });\r
+        assertOrder(layout2, new int[] { 2, 1, 3 });\r
+\r
+        layout1.addComponent(children[3], 0);\r
+        assertOrder(layout1, new int[] { 3, 0 });\r
+        assertOrder(layout2, new int[] { 2, 1 });\r
+\r
+        layout2.addComponent(children[0]);\r
+        assertOrder(layout1, new int[] { 3 });\r
+        assertOrder(layout2, new int[] { 2, 1, 0 });\r
+\r
+        layout1.addComponentAsFirst(children[1]);\r
+        assertOrder(layout1, new int[] { 1, 3 });\r
+        assertOrder(layout2, new int[] { 2, 0 });\r
+    }\r
+\r
+    @Test\r
+    public void shuffleChildComponents() {\r
+        shuffleChildComponents(new HorizontalLayout());\r
+        shuffleChildComponents(new VerticalLayout());\r
+    }\r
+\r
+    private void shuffleChildComponents(AbstractOrderedLayout layout) {\r
+\r
+        for (int i = 0; i < children.length; ++i) {\r
+            layout.addComponent(children[i], i);\r
+        }\r
+\r
+        assertOrder(layout, new int[] { 0, 1, 2, 3 });\r
+\r
+        // Move C from #2 to #1\r
+        // Exhibits defect #7668\r
+        layout.addComponent(children[2], 1);\r
+        assertOrder(layout, new int[] { 0, 2, 1, 3 });\r
+\r
+        // Move C from #1 to #4 (which becomes #3 when #1 is erased)\r
+        layout.addComponent(children[2], 4);\r
+        assertOrder(layout, new int[] { 0, 1, 3, 2 });\r
+\r
+        // Keep everything in place\r
+        layout.addComponent(children[1], 1);\r
+        assertOrder(layout, new int[] { 0, 1, 3, 2 });\r
+\r
+        // Move D from #2 to #0\r
+        layout.addComponent(children[3], 0);\r
+        assertOrder(layout, new int[] { 3, 0, 1, 2 });\r
+\r
+        // Move A from #1 to end (#4 which becomes #3)\r
+        layout.addComponent(children[0]);\r
+        assertOrder(layout, new int[] { 3, 1, 2, 0 });\r
+\r
+        // Keep everything in place\r
+        layout.addComponent(children[0]);\r
+        assertOrder(layout, new int[] { 3, 1, 2, 0 });\r
+\r
+        // Move C from #2 to #0\r
+        layout.addComponentAsFirst(children[2]);\r
+        assertOrder(layout, new int[] { 2, 3, 1, 0 });\r
+\r
+        // Keep everything in place\r
+        layout.addComponentAsFirst(children[2]);\r
+        assertOrder(layout, new int[] { 2, 3, 1, 0 });\r
+    }\r
+\r
+    /**\r
+     * Asserts that layout has the components in children in the order specified\r
+     * by indices.\r
+     */\r
+    private void assertOrder(Layout layout, int[] indices) {\r
+        Iterator<?> i = layout.getComponentIterator();\r
+        try {\r
+            for (int index : indices) {\r
+                assertSame(children[index], i.next());\r
+            }\r
+            assertFalse("Too many components in layout", i.hasNext());\r
+        } catch (NoSuchElementException e) {\r
+            fail("Too few components in layout");\r
+        }\r
+    }\r
+}\r

diff --git a/tests/server-side/com/vaadin/tests/server/component/csslayout/AddComponentsTest.java b/tests/server-side/com/vaadin/tests/server/component/csslayout/AddComponentsTest.java
new file mode 100644 (file)
index 0000000..2965783
--- /dev/null
+++ b/tests/server-side/com/vaadin/tests/server/component/csslayout/AddComponentsTest.java
@@ -0,0 +1,109 @@
+package com.vaadin.tests.server.component.csslayout;\r
+\r
+import java.util.Iterator;\r
+import java.util.NoSuchElementException;\r
+\r
+import org.junit.Test;\r
+\r
+import static org.junit.Assert.assertSame;\r
+import static org.junit.Assert.assertFalse;\r
+import static org.junit.Assert.fail;\r
+\r
+import com.vaadin.ui.Component;\r
+import com.vaadin.ui.CssLayout;\r
+import com.vaadin.ui.Label;\r
+import com.vaadin.ui.Layout;\r
+\r
+public class AddComponentsTest {\r
+\r
+    private Component[] children = new Component[] { new Label("A"),\r
+            new Label("B"), new Label("C"), new Label("D") };\r
+\r
+    @Test\r
+    public void moveComponentsBetweenLayouts() {\r
+        CssLayout layout1 = new CssLayout();\r
+        CssLayout layout2 = new CssLayout();\r
+\r
+        layout1.addComponent(children[0]);\r
+        layout1.addComponent(children[1]);\r
+\r
+        layout2.addComponent(children[2]);\r
+        layout2.addComponent(children[3]);\r
+\r
+        layout2.addComponent(children[1], 1);\r
+        assertOrder(layout1, new int[] { 0 });\r
+        assertOrder(layout2, new int[] { 2, 1, 3 });\r
+\r
+        layout1.addComponent(children[3], 0);\r
+        assertOrder(layout1, new int[] { 3, 0 });\r
+        assertOrder(layout2, new int[] { 2, 1 });\r
+\r
+        layout2.addComponent(children[0]);\r
+        assertOrder(layout1, new int[] { 3 });\r
+        assertOrder(layout2, new int[] { 2, 1, 0 });\r
+\r
+        layout1.addComponentAsFirst(children[1]);\r
+        assertOrder(layout1, new int[] { 1, 3 });\r
+        assertOrder(layout2, new int[] { 2, 0 });\r
+    }\r
+\r
+    @Test\r
+    public void shuffleChildComponents() {\r
+        CssLayout layout = new CssLayout();\r
+\r
+        for (int i = 0; i < children.length; ++i) {\r
+            layout.addComponent(children[i], i);\r
+        }\r
+\r
+        assertOrder(layout, new int[] { 0, 1, 2, 3 });\r
+\r
+        // Move C from #2 to #1\r
+        // Exhibits defect #7668\r
+        layout.addComponent(children[2], 1);\r
+        assertOrder(layout, new int[] { 0, 2, 1, 3 });\r
+\r
+        // Move C from #1 to #4 (which becomes #3 when #1 is erased)\r
+        layout.addComponent(children[2], 4);\r
+        assertOrder(layout, new int[] { 0, 1, 3, 2 });\r
+\r
+        // Keep everything in place\r
+        layout.addComponent(children[1], 1);\r
+        assertOrder(layout, new int[] { 0, 1, 3, 2 });\r
+\r
+        // Move D from #2 to #0\r
+        layout.addComponent(children[3], 0);\r
+        assertOrder(layout, new int[] { 3, 0, 1, 2 });\r
+\r
+        // Move A from #1 to end (#4 which becomes #3)\r
+        layout.addComponent(children[0]);\r
+        assertOrder(layout, new int[] { 3, 1, 2, 0 });\r
+\r
+        // Keep everything in place\r
+        layout.addComponent(children[0]);\r
+        assertOrder(layout, new int[] { 3, 1, 2, 0 });\r
+\r
+        // Move C from #2 to #0\r
+        layout.addComponentAsFirst(children[2]);\r
+        assertOrder(layout, new int[] { 2, 3, 1, 0 });\r
+\r
+        // Keep everything in place\r
+        layout.addComponentAsFirst(children[2]);\r
+        assertOrder(layout, new int[] { 2, 3, 1, 0 });\r
+    }\r
+\r
+    /**\r
+     * Asserts that layout has the components in children in the order specified\r
+     * by indices.\r
+     */\r
+    private void assertOrder(Layout layout, int[] indices) {\r
+        Iterator<?> i = layout.getComponentIterator();\r
+        try {\r
+            for (int index : indices) {\r
+                assertSame(children[index], i.next());\r
+            }\r
+            assertFalse("Too many components in layout", i.hasNext());\r
+        } catch (NoSuchElementException e) {\r
+            fail("Too few components in layout");\r
+        }\r
+    }\r
+}\r