Fix displaying Details when Grid is attached to DOM (#18390)

Change-Id: I74360c7a3f0c5798dbaa44ea06ea1585c4289449

5 files changed, 81 insertions, 14 deletions

diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java
index b720dc442f..ef52a429e7 100644
--- a/client/src/com/vaadin/client/connectors/GridConnector.java
+++ b/client/src/com/vaadin/client/connectors/GridConnector.java
@@ -735,13 +735,20 @@ public class GridConnector extends AbstractHasComponentsConnector implements
 
     private final DetailsListener detailsListener = new DetailsListener() {
         @Override
-        public void reapplyDetailsVisibility(int rowIndex, JsonObject row) {
-            if (hasDetailsOpen(row)) {
-                getWidget().setDetailsVisible(rowIndex, true);
-                detailsConnectorFetcher.schedule();
-            } else {
-                getWidget().setDetailsVisible(rowIndex, false);
-            }
+        public void reapplyDetailsVisibility(final int rowIndex,
+                final JsonObject row) {
+            Scheduler.get().scheduleDeferred(new ScheduledCommand() {
+
+                @Override
+                public void execute() {
+                    if (hasDetailsOpen(row)) {
+                        getWidget().setDetailsVisible(rowIndex, true);
+                        detailsConnectorFetcher.schedule();
+                    } else {
+                        getWidget().setDetailsVisible(rowIndex, false);
+                    }
+                }
+            });
         }
 
         private boolean hasDetailsOpen(JsonObject row) {
@@ -912,6 +919,13 @@ public class GridConnector extends AbstractHasComponentsConnector implements
     }
 
     @Override
+    public void onUnregister() {
+        customDetailsGenerator.indexToDetailsMap.clear();
+
+        super.onUnregister();
+    }
+
+    @Override
     public void onStateChanged(final StateChangeEvent stateChangeEvent) {
         super.onStateChanged(stateChangeEvent);
 
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index 6d9e5d18ec..53ccd432c4 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -7649,7 +7649,8 @@ public class Grid<T> extends ResizeComposite implements
 
     @Override
     protected void onDetach() {
-        for (int row : visibleDetails) {
+        Set<Integer> details = new HashSet<Integer>(visibleDetails);
+        for (int row : details) {
             setDetailsVisible(row, false);
         }
 
diff --git a/server/src/com/vaadin/data/RpcDataProviderExtension.java b/server/src/com/vaadin/data/RpcDataProviderExtension.java
index d291d509b2..b3c7972b52 100644
--- a/server/src/com/vaadin/data/RpcDataProviderExtension.java
+++ b/server/src/com/vaadin/data/RpcDataProviderExtension.java
@@ -1475,4 +1475,13 @@ public class RpcDataProviderExtension extends AbstractExtension {
     public DetailComponentManager getDetailComponentManager() {
         return detailComponentManager;
     }
+
+    @Override
+    public void detach() {
+        for (Object itemId : ImmutableSet.copyOf(visibleDetails)) {
+            detailComponentManager.destroyDetails(itemId);
+        }
+
+        super.detach();
+    }
 }
diff --git a/uitest/src/com/vaadin/tests/components/grid/GridDetailsDetach.java b/uitest/src/com/vaadin/tests/components/grid/GridDetailsDetach.java
index c25ad287b7..1032378a2d 100644
--- a/uitest/src/com/vaadin/tests/components/grid/GridDetailsDetach.java
+++ b/uitest/src/com/vaadin/tests/components/grid/GridDetailsDetach.java
@@ -35,7 +35,7 @@ public class GridDetailsDetach extends AbstractTestUI {
 
     @Override
     protected void setup(VaadinRequest request) {
-        final VerticalLayout layout = new VerticalLayout();
+        VerticalLayout layout = new VerticalLayout();
         layout.setSizeFull();
 
         Button button = new Button("Test");
@@ -43,19 +43,30 @@ public class GridDetailsDetach extends AbstractTestUI {
         layout.setExpandRatio(button, 0f);
 
         currentGrid = generateGrid();
-        layout.addComponent(currentGrid);
-        layout.setExpandRatio(currentGrid, 1f);
+        final VerticalLayout gridContainer = new VerticalLayout();
+        gridContainer.addComponent(currentGrid);
 
         button.addClickListener(new Button.ClickListener() {
 
             @Override
             public void buttonClick(ClickEvent event) {
-                Grid newGrid = generateGrid();
-                layout.replaceComponent(currentGrid, newGrid);
-                currentGrid = newGrid;
+                gridContainer.replaceComponent(currentGrid, new Label("Foo"));
             }
         });
 
+        layout.addComponent(new Button("Reattach Grid",
+                new Button.ClickListener() {
+
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        gridContainer.removeAllComponents();
+                        gridContainer.addComponent(currentGrid);
+                    }
+                }));
+
+        layout.addComponent(gridContainer);
+        layout.setExpandRatio(gridContainer, 1f);
+
         addComponent(layout);
     }
 
diff --git a/uitest/src/com/vaadin/tests/components/grid/GridDetailsDetachTest.java b/uitest/src/com/vaadin/tests/components/grid/GridDetailsDetachTest.java
index f9071dd3da..fc79fd1b68 100644
--- a/uitest/src/com/vaadin/tests/components/grid/GridDetailsDetachTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/GridDetailsDetachTest.java
@@ -15,7 +15,12 @@
  */
 package com.vaadin.tests.components.grid;
 
+import java.util.List;
+
+import org.junit.Assert;
 import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
 
 import com.vaadin.testbench.elements.ButtonElement;
 import com.vaadin.testbench.elements.GridElement;
@@ -31,11 +36,38 @@ public class GridDetailsDetachTest extends MultiBrowserTest {
         openTestURL();
 
         $(GridElement.class).first().getCell(3, 0).click();
+        $(GridElement.class).first().getCell(5, 0).click();
+
+        assertNoErrorNotifications();
+
+        $(ButtonElement.class).first().click();
+
+        assertNoErrorNotifications();
+    }
+
+    @Test
+    public void testDetachAndReattachGridWithDetailsOpen() {
+        setDebug(true);
+        openTestURL();
+
+        $(GridElement.class).first().getCell(3, 0).click();
+        $(GridElement.class).first().getCell(5, 0).click();
 
         assertNoErrorNotifications();
 
         $(ButtonElement.class).first().click();
 
         assertNoErrorNotifications();
+
+        $(ButtonElement.class).get(1).click();
+
+        assertNoErrorNotifications();
+
+        List<WebElement> spacers = findElements(By.className("v-grid-spacer"));
+        Assert.assertEquals("Not enough spacers in DOM", 2, spacers.size());
+        Assert.assertEquals("Spacer content not visible",
+                "Extra data for Bean 3", spacers.get(0).getText());
+        Assert.assertEquals("Spacer content not visible",
+                "Extra data for Bean 5", spacers.get(1).getText());
     }
 }