Fixes Issues #596 & #982.
Includes the following changes:
1 - Removes the children field from within TabPanel to rely exclusively on
the contained DeckPanel to maintain the child widgets.
2 - Introduces nested unmodifiable classes for DeckPanel and TabBar to
prevent invariant violating changes from being made though getTabBar()
and getDeckPanel().
3 - Expands TabPanelTest to verify the behavior introduced in 2.
Patch by: bobv
Review by: knorton
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@1010 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/client/ui/TabPanel.java b/user/src/com/google/gwt/user/client/ui/TabPanel.java
index 2ddee07..fae85e8 100644
--- a/user/src/com/google/gwt/user/client/ui/TabPanel.java
+++ b/user/src/com/google/gwt/user/client/ui/TabPanel.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
@@ -48,9 +48,121 @@
public class TabPanel extends Composite implements TabListener,
SourcesTabEvents, HasWidgets, IndexedPanel {
- private WidgetCollection children = new WidgetCollection(this);
- private DeckPanel deck = new DeckPanel();
- private TabBar tabBar = new TabBar();
+ /**
+ * This extension of DeckPanel overrides the public mutator methods to
+ * prevent external callers from adding to the state of the DeckPanel.
+ * <p>
+ * Removal of Widgets is supported so that WidgetCollection.WidgetIterator
+ * operates as expected.
+ * </p><p>
+ * We ensure that the DeckPanel cannot become of of sync with its
+ * associated TabBar by delegating all mutations to the TabBar to this
+ * implementation of DeckPanel.
+ * </p>
+ */
+ private static class TabbedDeckPanel extends DeckPanel {
+ private UnmodifiableTabBar tabBar;
+
+ public TabbedDeckPanel(UnmodifiableTabBar tabBar) {
+ this.tabBar = tabBar;
+ }
+
+ public void clear() {
+ throw new UnsupportedOperationException(
+ "Use TabPanel.clear() to alter the DeckPanel");
+ }
+
+ public void insert(Widget w, int beforeIndex) {
+ throw new UnsupportedOperationException(
+ "Use TabPanel.insert() to alter the DeckPanel");
+ }
+
+ public boolean remove(Widget w) {
+ // Removal of items from the TabBar is delegated to the DeckPanel
+ // to ensure consistency
+ int idx = getWidgetIndex(w);
+ if (idx != -1) {
+ tabBar.removeTabProtected(idx);
+ return super.remove(w);
+ }
+
+ return false;
+ }
+
+ protected void insertProtected(Widget w, String tabText, boolean asHTML,
+ int beforeIndex) {
+
+ // Check to see if the TabPanel already contains the Widget. If so,
+ // remove it and see if we need to shift the position to the left.
+ int idx = getWidgetIndex(w);
+ if (idx != -1) {
+ remove(w);
+ if (idx < beforeIndex) {
+ beforeIndex--;
+ }
+ }
+
+ tabBar.insertTabProtected(tabText, asHTML, beforeIndex);
+ super.insert(w, beforeIndex);
+ }
+
+ protected void insertProtected(Widget w, Widget tabWidget,
+ int beforeIndex) {
+
+ // Check to see if the TabPanel already contains the Widget. If so,
+ // remove it and see if we need to shift the position to the left.
+ int idx = getWidgetIndex(w);
+ if (idx != -1) {
+ remove(w);
+ if (idx < beforeIndex) {
+ beforeIndex--;
+ }
+ }
+
+ tabBar.insertTabProtected(tabWidget, beforeIndex);
+ super.insert(w, beforeIndex);
+ }
+ };
+
+ /**
+ * This extension of TabPanel overrides the public mutator methods to
+ * prevent external callers from modifying the state of the TabBar.
+ */
+ private static class UnmodifiableTabBar extends TabBar {
+ public void insertTab(String text, boolean asHTML, int beforeIndex) {
+ throw new UnsupportedOperationException(
+ "Use TabPanel.insert() to alter the TabBar");
+ }
+
+ public void insertTab(Widget widget, int beforeIndex) {
+ throw new UnsupportedOperationException(
+ "Use TabPanel.insert() to alter the TabBar");
+ }
+
+ public void removeTab(int index) {
+ // It's possible for removeTab() to function correctly, but it's
+ // preferable to have only TabbedDeckPanel.remove() be operable,
+ // especially since TabBar does not export an Iterator over its values.
+ throw new UnsupportedOperationException(
+ "Use TabPanel.remove() to alter the TabBar");
+ }
+
+ protected void insertTabProtected(String text, boolean asHTML,
+ int beforeIndex) {
+ super.insertTab(text, asHTML, beforeIndex);
+ }
+
+ protected void insertTabProtected(Widget widget, int beforeIndex) {
+ super.insertTab(widget, beforeIndex);
+ }
+
+ protected void removeTabProtected(int index) {
+ super.removeTab(index);
+ }
+ }
+
+ private UnmodifiableTabBar tabBar = new UnmodifiableTabBar();
+ private TabbedDeckPanel deck = new TabbedDeckPanel(tabBar);
private TabListenerCollection tabListeners;
/**
@@ -76,7 +188,8 @@
}
/**
- * Adds a widget to the tab panel.
+ * Adds a widget to the tab panel. If the Widget is already attached to
+ * the TabPanel, it will be moved to the right-most index.
*
* @param w the widget to be added
* @param tabText the text to be shown on its tab
@@ -86,7 +199,8 @@
}
/**
- * Adds a widget to the tab panel.
+ * Adds a widget to the tab panel. If the Widget is already attached to
+ * the TabPanel, it will be moved to the right-most index.
*
* @param w the widget to be added
* @param tabText the text to be shown on its tab
@@ -97,7 +211,8 @@
}
/**
- * Adds a widget to the tab panel.
+ * Adds a widget to the tab panel. If the Widget is already attached to
+ * the TabPanel, it will be moved to the right-most index.
*
* @param w the widget to be added
* @param tabWidget the widget to be shown in the tab
@@ -120,7 +235,9 @@
}
/**
- * Gets the deck panel within this tab panel.
+ * Gets the deck panel within this tab panel. Adding or removing Widgets
+ * from the DeckPanel is not supported and will throw
+ * UnsupportedOperationExceptions.
*
* @return the deck panel
*/
@@ -129,7 +246,9 @@
}
/**
- * Gets the tab bar within this tab panel.
+ * Gets the tab bar within this tab panel. Adding or removing tabs from
+ * from the TabBar is not supported and will throw
+ * UnsupportedOperationExceptions.
*
* @return the tab bar
*/
@@ -138,19 +257,20 @@
}
public Widget getWidget(int index) {
- return children.get(index);
+ return deck.getWidget(index);
}
public int getWidgetCount() {
- return children.size();
+ return deck.getWidgetCount();
}
public int getWidgetIndex(Widget widget) {
- return children.indexOf(widget);
+ return deck.getWidgetIndex(widget);
}
/**
- * Inserts a widget into the tab panel.
+ * Inserts a widget into the tab panel. If the Widget is already attached
+ * to the TabPanel, it will be moved to the requested index.
*
* @param widget the widget to be inserted
* @param tabText the text to be shown on its tab
@@ -159,26 +279,26 @@
*/
public void insert(Widget widget, String tabText, boolean asHTML,
int beforeIndex) {
- children.insert(widget, beforeIndex);
- tabBar.insertTab(tabText, asHTML, beforeIndex);
- deck.insert(widget, beforeIndex);
+ // Delegate updates to the TabBar to our DeckPanel implementation
+ deck.insertProtected(widget, tabText, asHTML, beforeIndex);
}
/**
- * Inserts a widget into the tab panel.
+ * Inserts a widget into the tab panel. If the Widget is already attached
+ * to the TabPanel, it will be moved to the requested index.
*
* @param widget the widget to be inserted.
* @param tabWidget the widget to be shown on its tab.
* @param beforeIndex the index before which it will be inserted.
*/
public void insert(Widget widget, Widget tabWidget, int beforeIndex) {
- children.insert(widget, beforeIndex);
- tabBar.insertTab(tabWidget, beforeIndex);
- deck.insert(widget, beforeIndex);
+ // Delegate updates to the TabBar to our DeckPanel implementation
+ deck.insertProtected(widget, tabWidget, beforeIndex);
}
/**
- * Inserts a widget into the tab panel.
+ * Inserts a widget into the tab panel. If the Widget is already attached
+ * to the TabPanel, it will be moved to the requested index.
*
* @param widget the widget to be inserted
* @param tabText the text to be shown on its tab
@@ -189,7 +309,9 @@
}
public Iterator iterator() {
- return children.iterator();
+ // The Iterator returned by DeckPanel supports removal and will invoke
+ // TabbedDeckPanel.remove(), which is an active function.
+ return deck.iterator();
}
public boolean onBeforeTabSelected(SourcesTabEvents sender, int tabIndex) {
@@ -207,7 +329,8 @@
}
public boolean remove(int index) {
- return remove(getWidget(index));
+ // Delegate updates to the TabBar to our DeckPanel implementation
+ return deck.remove(index);
}
/**
@@ -216,15 +339,8 @@
* @param widget the widget to be removed
*/
public boolean remove(Widget widget) {
- int index = getWidgetIndex(widget);
- if (index == -1) {
- return false;
- }
-
- children.remove(widget);
- tabBar.removeTab(index);
- deck.remove(widget);
- return true;
+ // Delegate updates to the TabBar to our DeckPanel implementation
+ return deck.remove(widget);
}
public void removeTabListener(TabListener listener) {
diff --git a/user/test/com/google/gwt/user/client/ui/TabPanelTest.java b/user/test/com/google/gwt/user/client/ui/TabPanelTest.java
index b54050d..5584b93 100644
--- a/user/test/com/google/gwt/user/client/ui/TabPanelTest.java
+++ b/user/test/com/google/gwt/user/client/ui/TabPanelTest.java
@@ -27,6 +27,125 @@
public String getModuleName() {
return "com.google.gwt.user.User";
}
+
+ public void testUnmodifiableDeckPanelSubclasses() {
+ TabPanel p = new TabPanel();
+ DeckPanel d = p.getDeckPanel();
+
+ try {
+ d.add(new Label("No"));
+ fail("Internal DeckPanel should not allow add() method");
+ } catch (UnsupportedOperationException e) {
+ // Expected behavior
+ }
+
+ try {
+ d.insert(new Label("No"), 0);
+ fail("Internal DeckPanel should not allow insert() method");
+ } catch (UnsupportedOperationException e) {
+ // Expected behavior
+ }
+
+ try {
+ d.clear();
+ fail("Internal DeckPanel should not allow clear() method");
+ } catch (UnsupportedOperationException e) {
+ // Expected behavior
+ }
+ }
+
+ public void testUnmodifiableTabBarSubclasses() {
+ TabPanel p = new TabPanel();
+ TabBar b = p.getTabBar();
+
+ try {
+ b.addTab("no");
+ fail("Internal TabBar should not allow addTab() method");
+ } catch (UnsupportedOperationException e) {
+ // Expected behavior
+ }
+
+ try {
+ b.addTab("no", true);
+ fail("Internal TabBar should not allow addTab() method");
+ } catch (UnsupportedOperationException e) {
+ // Expected behavior
+ }
+
+ try {
+ b.addTab(new Label("no"));
+ fail("Internal TabBar should not allow addTab() method");
+ } catch (UnsupportedOperationException e) {
+ // Expected behavior
+ }
+
+ try {
+ b.insertTab("no", 0);
+ fail("Internal TabBar should not allow insertTab() method");
+ } catch (UnsupportedOperationException e) {
+ // Expected behavior
+ }
+
+ try {
+ b.insertTab("no", true, 0);
+ fail("Internal TabBar should not allow insertTab() method");
+ } catch (UnsupportedOperationException e) {
+ // Expected behavior
+ }
+
+ try {
+ b.insertTab(new Label("no"), 0);
+ fail("Internal TabBar should not allow insertTab() method");
+ } catch (UnsupportedOperationException e) {
+ // Expected behavior
+ }
+
+ try {
+ b.removeTab(0);
+ fail("Internal TabBar should not allow removeTab() method");
+ } catch (UnsupportedOperationException e) {
+ // Expected behavior
+ }
+ }
+
+ public void testInsertMultipleTimes() {
+ TabPanel p = new TabPanel();
+
+ TextBox tb = new TextBox();
+ p.add(tb, "Title");
+ p.add(tb, "Title");
+ p.add(tb, "Title3");
+
+ assertEquals(1, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(tb));
+ Iterator i = p.iterator();
+ assertTrue(i.hasNext());
+ assertTrue(tb.equals((Widget)i.next()));
+ assertFalse(i.hasNext());
+
+ Label l = new Label();
+ p.add(l, "Title");
+ p.add(l, "Title");
+ p.add(l, "Title3");
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(tb));
+ assertEquals(1, p.getWidgetIndex(l));
+
+ p.insert(l, "Title", 0);
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(l));
+ assertEquals(1, p.getWidgetIndex(tb));
+
+ p.insert(l, "Title", 1);
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(l));
+ assertEquals(1, p.getWidgetIndex(tb));
+
+ p.insert(l, "Title", 2);
+ assertEquals(2, p.getWidgetCount());
+ assertEquals(0, p.getWidgetIndex(tb));
+ assertEquals(1, p.getWidgetIndex(l));
+ }
/**
* Tests to ensure that arbitrary widgets can be added/inserted effectively.
