From: Johannes Dahlström Date: Wed, 30 Nov 2011 08:47:12 +0000 (+0000) Subject: #7668 fix and tests merged from 6.7 X-Git-Tag: 7.0.0.alpha1~225^2~4 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=83bde3adc3fb0992fc936aef51e4f8611c75a5ac;p=vaadin-framework.git #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 0b0fff2a34..68c5ace0fc 100644 --- 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 5789a65ed3..78474d33fe 100644 --- 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 index 0000000000..ba5ea62181 --- /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; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.fail; + +import java.util.Iterator; +import java.util.NoSuchElementException; + +import org.junit.Test; + +import com.vaadin.ui.AbstractOrderedLayout; +import com.vaadin.ui.Component; +import com.vaadin.ui.HorizontalLayout; +import com.vaadin.ui.Label; +import com.vaadin.ui.Layout; +import com.vaadin.ui.VerticalLayout; + +public class AddComponentsTest { + + Component[] children = new Component[] { new Label("A"), new Label("B"), + new Label("C"), new Label("D") }; + + @Test + public void moveComponentsBetweenLayouts() { + AbstractOrderedLayout layout1 = new HorizontalLayout(); + AbstractOrderedLayout layout2 = new VerticalLayout(); + + layout1.addComponent(children[0]); + layout1.addComponent(children[1]); + + layout2.addComponent(children[2]); + layout2.addComponent(children[3]); + + layout2.addComponent(children[1], 1); + assertOrder(layout1, new int[] { 0 }); + assertOrder(layout2, new int[] { 2, 1, 3 }); + + layout1.addComponent(children[3], 0); + assertOrder(layout1, new int[] { 3, 0 }); + assertOrder(layout2, new int[] { 2, 1 }); + + layout2.addComponent(children[0]); + assertOrder(layout1, new int[] { 3 }); + assertOrder(layout2, new int[] { 2, 1, 0 }); + + layout1.addComponentAsFirst(children[1]); + assertOrder(layout1, new int[] { 1, 3 }); + assertOrder(layout2, new int[] { 2, 0 }); + } + + @Test + public void shuffleChildComponents() { + shuffleChildComponents(new HorizontalLayout()); + shuffleChildComponents(new VerticalLayout()); + } + + private void shuffleChildComponents(AbstractOrderedLayout layout) { + + for (int i = 0; i < children.length; ++i) { + layout.addComponent(children[i], i); + } + + assertOrder(layout, new int[] { 0, 1, 2, 3 }); + + // Move C from #2 to #1 + // Exhibits defect #7668 + layout.addComponent(children[2], 1); + assertOrder(layout, new int[] { 0, 2, 1, 3 }); + + // Move C from #1 to #4 (which becomes #3 when #1 is erased) + layout.addComponent(children[2], 4); + assertOrder(layout, new int[] { 0, 1, 3, 2 }); + + // Keep everything in place + layout.addComponent(children[1], 1); + assertOrder(layout, new int[] { 0, 1, 3, 2 }); + + // Move D from #2 to #0 + layout.addComponent(children[3], 0); + assertOrder(layout, new int[] { 3, 0, 1, 2 }); + + // Move A from #1 to end (#4 which becomes #3) + layout.addComponent(children[0]); + assertOrder(layout, new int[] { 3, 1, 2, 0 }); + + // Keep everything in place + layout.addComponent(children[0]); + assertOrder(layout, new int[] { 3, 1, 2, 0 }); + + // Move C from #2 to #0 + layout.addComponentAsFirst(children[2]); + assertOrder(layout, new int[] { 2, 3, 1, 0 }); + + // Keep everything in place + layout.addComponentAsFirst(children[2]); + assertOrder(layout, new int[] { 2, 3, 1, 0 }); + } + + /** + * Asserts that layout has the components in children in the order specified + * by indices. + */ + private void assertOrder(Layout layout, int[] indices) { + Iterator i = layout.getComponentIterator(); + try { + for (int index : indices) { + assertSame(children[index], i.next()); + } + assertFalse("Too many components in layout", i.hasNext()); + } catch (NoSuchElementException e) { + fail("Too few components in layout"); + } + } +} 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 index 0000000000..29657830e8 --- /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; + +import java.util.Iterator; +import java.util.NoSuchElementException; + +import org.junit.Test; + +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.fail; + +import com.vaadin.ui.Component; +import com.vaadin.ui.CssLayout; +import com.vaadin.ui.Label; +import com.vaadin.ui.Layout; + +public class AddComponentsTest { + + private Component[] children = new Component[] { new Label("A"), + new Label("B"), new Label("C"), new Label("D") }; + + @Test + public void moveComponentsBetweenLayouts() { + CssLayout layout1 = new CssLayout(); + CssLayout layout2 = new CssLayout(); + + layout1.addComponent(children[0]); + layout1.addComponent(children[1]); + + layout2.addComponent(children[2]); + layout2.addComponent(children[3]); + + layout2.addComponent(children[1], 1); + assertOrder(layout1, new int[] { 0 }); + assertOrder(layout2, new int[] { 2, 1, 3 }); + + layout1.addComponent(children[3], 0); + assertOrder(layout1, new int[] { 3, 0 }); + assertOrder(layout2, new int[] { 2, 1 }); + + layout2.addComponent(children[0]); + assertOrder(layout1, new int[] { 3 }); + assertOrder(layout2, new int[] { 2, 1, 0 }); + + layout1.addComponentAsFirst(children[1]); + assertOrder(layout1, new int[] { 1, 3 }); + assertOrder(layout2, new int[] { 2, 0 }); + } + + @Test + public void shuffleChildComponents() { + CssLayout layout = new CssLayout(); + + for (int i = 0; i < children.length; ++i) { + layout.addComponent(children[i], i); + } + + assertOrder(layout, new int[] { 0, 1, 2, 3 }); + + // Move C from #2 to #1 + // Exhibits defect #7668 + layout.addComponent(children[2], 1); + assertOrder(layout, new int[] { 0, 2, 1, 3 }); + + // Move C from #1 to #4 (which becomes #3 when #1 is erased) + layout.addComponent(children[2], 4); + assertOrder(layout, new int[] { 0, 1, 3, 2 }); + + // Keep everything in place + layout.addComponent(children[1], 1); + assertOrder(layout, new int[] { 0, 1, 3, 2 }); + + // Move D from #2 to #0 + layout.addComponent(children[3], 0); + assertOrder(layout, new int[] { 3, 0, 1, 2 }); + + // Move A from #1 to end (#4 which becomes #3) + layout.addComponent(children[0]); + assertOrder(layout, new int[] { 3, 1, 2, 0 }); + + // Keep everything in place + layout.addComponent(children[0]); + assertOrder(layout, new int[] { 3, 1, 2, 0 }); + + // Move C from #2 to #0 + layout.addComponentAsFirst(children[2]); + assertOrder(layout, new int[] { 2, 3, 1, 0 }); + + // Keep everything in place + layout.addComponentAsFirst(children[2]); + assertOrder(layout, new int[] { 2, 3, 1, 0 }); + } + + /** + * Asserts that layout has the components in children in the order specified + * by indices. + */ + private void assertOrder(Layout layout, int[] indices) { + Iterator i = layout.getComponentIterator(); + try { + for (int index : indices) { + assertSame(children[index], i.next()); + } + assertFalse("Too many components in layout", i.hasNext()); + } catch (NoSuchElementException e) { + fail("Too few components in layout"); + } + } +}