Fixes removing Widgets from Grid header/footer

Change-Id: Ic2872fec49851ea3c8ed32ca2a77dcbd27e739a8

3 files changed, 107 insertions, 6 deletions

diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index e50fcb8ba6..bbdaec8994 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -54,6 +54,7 @@ import com.google.gwt.user.client.Event;
 import com.google.gwt.user.client.Timer;
 import com.google.gwt.user.client.ui.Button;
 import com.google.gwt.user.client.ui.CheckBox;
+import com.google.gwt.user.client.ui.HasWidgets;
 import com.google.gwt.user.client.ui.ResizeComposite;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.DeferredWorker;
@@ -168,7 +169,7 @@ import com.vaadin.shared.util.SharedUtil;
  * @author Vaadin Ltd
  */
 public class Grid<T> extends ResizeComposite implements
-        HasSelectionHandlers<T>, SubPartAware, DeferredWorker {
+        HasSelectionHandlers<T>, SubPartAware, DeferredWorker, HasWidgets {
 
     /**
      * Enum describing different sections of Grid.
@@ -5545,4 +5546,80 @@ public class Grid<T> extends ResizeComposite implements
             setEscalatorSizeFromDataSource();
         }
     }
+
+    /**
+     * Grid does not support adding Widgets this way.
+     * <p>
+     * This method is implemented only because removing widgets from Grid (added
+     * via e.g. {@link Renderer}s) requires the {@link HasWidgets} interface.
+     * 
+     * @param w
+     *            irrelevant
+     * @throws UnsupportedOperationException
+     *             always
+     */
+    @Override
+    @Deprecated
+    public void add(Widget w) {
+        throw new UnsupportedOperationException(
+                "Cannot add widgets to Grid with this method");
+    }
+
+    /**
+     * Grid does not support clearing Widgets this way.
+     * <p>
+     * This method is implemented only because removing widgets from Grid (added
+     * via e.g. {@link Renderer}s) requires the {@link HasWidgets} interface.
+     * 
+     * @throws UnsupportedOperationException
+     *             always
+     */
+    @Override
+    @Deprecated
+    public void clear() {
+        throw new UnsupportedOperationException(
+                "Cannot clear widgets from Grid this way");
+    }
+
+    /**
+     * Grid does not support iterating through Widgets this way.
+     * <p>
+     * This method is implemented only because removing widgets from Grid (added
+     * via e.g. {@link Renderer}s) requires the {@link HasWidgets} interface.
+     * 
+     * @return never
+     * @throws UnsupportedOperationException
+     *             always
+     */
+    @Override
+    @Deprecated
+    public Iterator<Widget> iterator() {
+        throw new UnsupportedOperationException(
+                "Cannot iterate through widgets in Grid this way");
+    }
+
+    /**
+     * Grid does not support removing Widgets this way.
+     * <p>
+     * This method is implemented only because removing widgets from Grid (added
+     * via e.g. {@link Renderer}s) requires the {@link HasWidgets} interface.
+     * 
+     * @return always <code>false</code>
+     */
+    @Override
+    @Deprecated
+    public boolean remove(Widget w) {
+        /*
+         * This is the method that is the sole reason to have Grid implement
+         * HasWidget - when Vaadin removes a Component from the hierarchy, the
+         * corresponding Widget will call removeFromParent() on itself. GWT will
+         * check there that its parent (i.e. Grid) implements HasWidgets, and
+         * will call this remove(Widget) method.
+         * 
+         * tl;dr: all this song and dance to make sure GWT's sanity checks
+         * aren't triggered, even though they effectively do nothing interesting
+         * from Grid's perspective.
+         */
+        return false;
+    }
 }
diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index a482f819b7..4166df6c71 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -48,7 +48,6 @@ import com.vaadin.data.Property;
 import com.vaadin.data.RpcDataProviderExtension;
 import com.vaadin.data.RpcDataProviderExtension.DataProviderKeyMapper;
 import com.vaadin.data.fieldgroup.FieldGroup;
-import com.vaadin.data.fieldgroup.FieldGroup.BindException;
 import com.vaadin.data.fieldgroup.FieldGroup.CommitException;
 import com.vaadin.data.fieldgroup.FieldGroupFieldFactory;
 import com.vaadin.data.sort.Sort;
@@ -1170,6 +1169,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
              *            a plain text caption
              */
             public void setText(String text) {
+                removeComponentIfPresent();
                 cellState.text = text;
                 cellState.type = GridStaticCellType.TEXT;
                 row.section.markAsDirty();
@@ -1211,6 +1211,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
              *            the html to set
              */
             public void setHtml(String html) {
+                removeComponentIfPresent();
                 cellState.html = html;
                 cellState.type = GridStaticCellType.HTML;
                 row.section.markAsDirty();
@@ -1237,6 +1238,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
              *            the component to set
              */
             public void setComponent(Component component) {
+                removeComponentIfPresent();
                 component.setParent(row.section.grid);
                 cellState.connector = component;
                 cellState.type = GridStaticCellType.WIDGET;
@@ -1264,6 +1266,13 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
                 row.section.markAsDirty();
             }
 
+            private void removeComponentIfPresent() {
+                Component component = (Component) cellState.connector;
+                if (component != null) {
+                    component.setParent(null);
+                    cellState.connector = null;
+                }
+            }
         }
 
         protected Grid grid;
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java
index 76382da035..bf1d1329aa 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridStaticSectionComponentTest.java
@@ -16,18 +16,18 @@
 package com.vaadin.tests.components.grid.basicfeatures.server;
 
 import static org.junit.Assert.assertEquals;
-
-import java.io.IOException;
+import static org.junit.Assert.assertTrue;
 
 import org.junit.Test;
 
 import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.NotificationElement;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
 
 public class GridStaticSectionComponentTest extends GridBasicFeaturesTest {
 
     @Test
-    public void testNativeButtonInHeader() throws IOException {
+    public void testNativeButtonInHeader() throws Exception {
         openTestURL();
 
         selectMenuPath("Component", "Columns", "Column 1", "Header Type",
@@ -39,7 +39,7 @@ public class GridStaticSectionComponentTest extends GridBasicFeaturesTest {
     }
 
     @Test
-    public void testNativeButtonInFooter() throws IOException {
+    public void testNativeButtonInFooter() throws Exception {
         openTestURL();
 
         selectMenuPath("Component", "Footer", "Visible");
@@ -51,4 +51,19 @@ public class GridStaticSectionComponentTest extends GridBasicFeaturesTest {
 
         assertEquals("5. Button clicked!", getLogRow(0));
     }
+
+    @Test
+    public void testRemoveComponentFromHeader() throws Exception {
+        openTestURL();
+        selectMenuPath("Component", "Columns", "Column 1", "Header Type",
+                "Widget Header");
+        selectMenuPath("Component", "Columns", "Column 1", "Header Type",
+                "Text Header");
+        assertTrue("No notifications should've been shown",
+                !$(NotificationElement.class).exists());
+        assertEquals("Header should've been reverted back to text header",
+                "text header", getGridElement().getHeaderCell(0, 1).getText()
+                        .toLowerCase());
+    }
+
 }