Fixes a bug in SplitLayoutPanel where calling setWidgetMinSize() on the center widget, the last widget, or a widget that is not a child results in an NPE or IndexOutOfBoundsException. The occured because getAssocatedSplitter() had an off by one bug and didn't check for an idx of -1. This patch asserts that the widget is a child, which is typical of other methods in DockLayoutPanel.
In addition, this patch fixes a bug where remove(myWidget) would always remove widget 0 instead of the splitter associated with myWidget because we were getting the idx of myWidget after it was removed, at which point it was -1. We now get the idx before removing the widget.
Review at http://gwt-code-reviews.appspot.com/378802
Review by: jgw@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@7965 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/client/ui/DockLayoutPanel.java b/user/src/com/google/gwt/user/client/ui/DockLayoutPanel.java
index c026d10..dfac34a 100644
--- a/user/src/com/google/gwt/user/client/ui/DockLayoutPanel.java
+++ b/user/src/com/google/gwt/user/client/ui/DockLayoutPanel.java
@@ -373,7 +373,7 @@
layout.onDetach();
}
- private void assertIsChild(Widget widget) {
+ void assertIsChild(Widget widget) {
assert (widget == null) || (widget.getParent() == this) : "The specified widget is not a child of this panel";
}
diff --git a/user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java b/user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java
index f1e6880..b7277d7 100644
--- a/user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java
+++ b/user/src/com/google/gwt/user/client/ui/SplitLayoutPanel.java
@@ -222,12 +222,13 @@
public boolean remove(Widget child) {
assert !(child instanceof Splitter) : "Splitters may not be directly removed";
+ int idx = getWidgetIndex(child);
if (super.remove(child)) {
// Remove the associated splitter, if any.
- int idx = getWidgetIndex(child);
- if (idx < getWidgetCount() - 1) {
+ // Now that the widget is removed, idx is the index of the splitter.
+ if (idx < getWidgetCount()) {
// Call super.remove(), or we'll end up recursing.
- super.remove(getWidget(idx + 1));
+ super.remove(getWidget(idx));
}
return true;
}
@@ -238,7 +239,7 @@
* Sets the minimum allowable size for the given widget.
*
* <p>
- * Its assocated splitter cannot be dragged to a position that would make it
+ * Its associated splitter cannot be dragged to a position that would make it
* smaller than this size. This method has no effect for the
* {@link DockLayoutPanel.Direction#CENTER} widget.
* </p>
@@ -247,8 +248,12 @@
* @param minSize the minimum size for this widget
*/
public void setWidgetMinSize(Widget child, int minSize) {
+ assertIsChild(child);
Splitter splitter = getAssociatedSplitter(child);
- splitter.setMinSize(minSize);
+ // The splitter is null for the center element.
+ if (splitter != null) {
+ splitter.setMinSize(minSize);
+ }
}
private Splitter getAssociatedSplitter(Widget child) {
@@ -256,7 +261,7 @@
// widget that *isn't* followed by a splitter must be the CENTER, which has
// no associated splitter.
int idx = getWidgetIndex(child);
- if (idx < getWidgetCount() - 2) {
+ if (idx > -1 && idx < getWidgetCount() - 1) {
Widget splitter = getWidget(idx + 1);
assert splitter instanceof Splitter : "Expected child widget to be splitter";
return (Splitter) splitter;
diff --git a/user/test/com/google/gwt/user/client/ui/SplitLayoutPanelTest.java b/user/test/com/google/gwt/user/client/ui/SplitLayoutPanelTest.java
index 0f59fdd..391ca0a 100644
--- a/user/test/com/google/gwt/user/client/ui/SplitLayoutPanelTest.java
+++ b/user/test/com/google/gwt/user/client/ui/SplitLayoutPanelTest.java
@@ -47,6 +47,18 @@
assertEquals(l2, p.getCenter());
}
+ public void testSetWidgetMinSizeCenter() {
+ SplitLayoutPanel p = new SplitLayoutPanel();
+ Label west = new Label("west");
+ Label center = new Label("center");
+
+ p.addWest(west, 100);
+ p.add(center);
+
+ // Should be ignored gracefully.
+ p.setWidgetMinSize(center, 10);
+ }
+
public void testSplitterOrder() {
SplitLayoutPanel p = new SplitLayoutPanel();
WidgetCollection children = p.getChildren();
@@ -97,4 +109,27 @@
assertEquals(SplitLayoutPanel.HSplitter.class, children.get(1).getClass());
assertEquals(l1, children.get(2));
}
+
+ public void testRemoveOutOfOrder() {
+ SplitLayoutPanel p = new SplitLayoutPanel();
+ WidgetCollection children = p.getChildren();
+
+ Label l0 = new Label("foo");
+ Label l1 = new Label("bar");
+
+ p.addWest(l0, 64);
+ p.addWest(l1, 64);
+ assertEquals(l0, children.get(0));
+ assertEquals(SplitLayoutPanel.HSplitter.class, children.get(1).getClass());
+ assertEquals(l1, children.get(2));
+ assertEquals(SplitLayoutPanel.HSplitter.class, children.get(3).getClass());
+
+ SplitLayoutPanel.HSplitter splitter0 = (SplitLayoutPanel.HSplitter) children.get(1);
+
+ // Remove the second element and make sure the correct splitter is removed.
+ p.remove(l1);
+ assertEquals(2, children.size());
+ assertEquals(l0, children.get(0));
+ assertEquals(splitter0, children.get(1));
+ }
}
