Fix for issue #718; fixes problems that would occur when adding/inserting a child Widget into its own parent HorizontalPanel or VerticalPanel.
Found by: fredsa
Patch by: bobv
Review by: scottb, jgw
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@1008 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/client/ui/HorizontalPanel.java b/user/src/com/google/gwt/user/client/ui/HorizontalPanel.java
index 8498a0a..f7f99ba 100644
--- a/user/src/com/google/gwt/user/client/ui/HorizontalPanel.java
+++ b/user/src/com/google/gwt/user/client/ui/HorizontalPanel.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2006 Google Inc.
+ * Copyright 2007 Google Inc.
*
* 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
@@ -44,7 +44,8 @@
}
/**
- * Adds a child widget to the panel.
+ * Adds a child widget to the panel. If the Widget is already attached to the
+ * HorizontalPanel, it will be moved to the end of the panel.
*
* @param w the widget to be added
*/
@@ -73,7 +74,8 @@
}
/**
- * Inserts a widget before the specified index.
+ * Inserts a widget before the specified index. If the Widget is already
+ * attached to the HorizontalPanel, it will be moved to the specified index.
*
* @param w the widget to be inserted
* @param beforeIndex the index before which it will be inserted
@@ -83,7 +85,18 @@
public void insert(Widget w, int beforeIndex) {
// Call this early to ensure that the table doesn't end up partially
// constructed when an exception is thrown from adopt().
- w.removeFromParent();
+ int idx = getWidgetIndex(w);
+ if (idx == -1) {
+ w.removeFromParent();
+ } else {
+ remove(w);
+
+ // If the Widget's previous position was left of the desired new position
+ // shift the desired position left to reflect the removal
+ if (idx < beforeIndex) {
+ beforeIndex--;
+ }
+ }
Element td = DOM.createTD();
DOM.insertChild(tableRow, td, beforeIndex);
diff --git a/user/src/com/google/gwt/user/client/ui/VerticalPanel.java b/user/src/com/google/gwt/user/client/ui/VerticalPanel.java
index 107a5fa..947ccf2 100644
--- a/user/src/com/google/gwt/user/client/ui/VerticalPanel.java
+++ b/user/src/com/google/gwt/user/client/ui/VerticalPanel.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2006 Google Inc.
+ * Copyright 2007 Google Inc.
*
* 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
@@ -40,7 +40,8 @@
}
/**
- * Adds a child widget to the panel.
+ * Adds a child widget to the panel. If the Widget is already attached to the
+ * VerticalPanel, it will be moved to the end of the panel.
*
* @param w the widget to be added
*/
@@ -69,7 +70,8 @@
}
/**
- * Inserts a widget before the specified index.
+ * Inserts a widget before the specified index. If the Widget is already
+ * attached to the VerticalPanel, it will be moved to the specified index.
*
* @param w the widget to be inserted
* @param beforeIndex the index before which it will be inserted
@@ -79,7 +81,18 @@
public void insert(Widget w, int beforeIndex) {
// Call this early to ensure that the table doesn't end up partially
// constructed when an exception is thrown from adopt().
- w.removeFromParent();
+ int idx = getWidgetIndex(w);
+ if (idx == -1) {
+ w.removeFromParent();
+ } else {
+ remove(w);
+
+ // If the Widget's previous position was left of the desired new position
+ // shift the desired position left to reflect the removal
+ if (idx < beforeIndex) {
+ beforeIndex--;
+ }
+ }
Element tr = DOM.createTR();
Element td = DOM.createTD();
diff --git a/user/test/com/google/gwt/user/client/ui/HorizontalPanelTest.java b/user/test/com/google/gwt/user/client/ui/HorizontalPanelTest.java
new file mode 100644
index 0000000..154d4f0
--- /dev/null
+++ b/user/test/com/google/gwt/user/client/ui/HorizontalPanelTest.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2007 Google Inc.
+ *
+ * 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.google.gwt.user.client.ui;
+
+import com.google.gwt.junit.client.GWTTestCase;
+
+import java.util.Iterator;
+
+/**
+ * A test for {@link HorizontalPanel}.
+ */
+public class HorizontalPanelTest extends GWTTestCase {
+
+ public String getModuleName() {
+ return "com.google.gwt.user.User";
+ }
+
+ public void testInsertMultipleTimes() {
+ HorizontalPanel p = new HorizontalPanel();
+
+ TextBox tb = new TextBox();
+ p.add(tb);
+ p.add(tb);
+ p.add(tb);
+
+ assertEquals(1, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(tb));
+ Iterator i = p.iterator();
+ assertTrue(i.hasNext());
+ assertTrue(tb.equals(i.next()));
+ assertFalse(i.hasNext());
+
+ Label l = new Label();
+ p.add(l);
+ p.add(l);
+ p.add(l);
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(tb));
+ assertEquals(1, p.getWidgetIndex(l));
+
+ p.insert(l, 0);
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(l));
+ assertEquals(1, p.getWidgetIndex(tb));
+
+ p.insert(l, 1);
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(l));
+ assertEquals(1, p.getWidgetIndex(tb));
+
+ p.insert(l, 2);
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(tb));
+ assertEquals(1, p.getWidgetIndex(l));
+ }
+}
diff --git a/user/test/com/google/gwt/user/client/ui/VerticalPanelTest.java b/user/test/com/google/gwt/user/client/ui/VerticalPanelTest.java
new file mode 100644
index 0000000..7f424e9
--- /dev/null
+++ b/user/test/com/google/gwt/user/client/ui/VerticalPanelTest.java
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2007 Google Inc.
+ *
+ * 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.google.gwt.user.client.ui;
+
+import com.google.gwt.junit.client.GWTTestCase;
+
+import java.util.Iterator;
+
+/**
+ * A test for {@link VerticalPanel}.
+ */
+public class VerticalPanelTest extends GWTTestCase {
+
+ public String getModuleName() {
+ return "com.google.gwt.user.User";
+ }
+
+ public void testInsertMultipleTimes() {
+ VerticalPanel p = new VerticalPanel();
+
+ TextBox tb = new TextBox();
+ p.add(tb);
+ p.add(tb);
+ p.add(tb);
+
+ assertEquals(1, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(tb));
+ Iterator i = p.iterator();
+ assertTrue(i.hasNext());
+ assertTrue(tb.equals(i.next()));
+ assertFalse(i.hasNext());
+
+ Label l = new Label();
+ p.add(l);
+ p.add(l);
+ p.add(l);
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(tb));
+ assertEquals(1, p.getWidgetIndex(l));
+
+ p.insert(l, 0);
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(l));
+ assertEquals(1, p.getWidgetIndex(tb));
+
+ p.insert(l, 1);
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(l));
+ assertEquals(1, p.getWidgetIndex(tb));
+
+ p.insert(l, 2);
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(tb));
+ assertEquals(1, p.getWidgetIndex(l));
+ }
+}
