Fix TabSheet attaching and detaching components (#10988)

This patch reverts the fix #10557 and replaces it with a
proper solution from Grid perspective.

Fixes #10987
Fixes #10985

4 files changed, 28 insertions, 36 deletions

diff --git a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java
index 574ed99045..3df69d45f3 100644
--- a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java
+++ b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java
@@ -220,11 +220,6 @@ public class DataCommunicator<T> extends AbstractExtension {
     public void attach() {
         super.attach();
         attachDataProviderListener();
-
-        if (getPushRows().isEmpty()) {
-            // Make sure rows are pushed when component is attached.
-            setPushRows(Range.withLength(0, getMinPushSize()));
-        }
     }
 
     @Override
@@ -313,6 +308,11 @@ public class DataCommunicator<T> extends AbstractExtension {
     public void beforeClientResponse(boolean initial) {
         super.beforeClientResponse(initial);
 
+        if (initial && getPushRows().isEmpty()) {
+            // Make sure rows are pushed when component is attached.
+            setPushRows(Range.withLength(0, getMinPushSize()));
+        }
+
         sendDataToClient(initial);
     }
 
diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java
index 956b81c3eb..40618074d8 100644
--- a/server/src/main/java/com/vaadin/ui/Grid.java
+++ b/server/src/main/java/com/vaadin/ui/Grid.java
@@ -2507,6 +2507,16 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
                 .forEach(this::addColumn);
     }
 
+    @Override
+    public void beforeClientResponse(boolean initial) {
+        super.beforeClientResponse(initial);
+
+        if (initial && editor.isOpen()) {
+            // Re-attaching grid. Any old editor should be closed.
+            editor.cancel();
+        }
+    }
+
     /**
      * Sets the property set to use for this grid. Does not create or update
      * columns in any way but will delete and re-create the editor.
@@ -4776,20 +4786,4 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
             column.updateSortable();
         }
     }
-
-    @Override
-    public void setVisible(boolean visible) {
-        if (getEditor().isOpen() && !visible) {
-            getEditor().cancel();
-        }
-        super.setVisible(visible);
-    }
-
-    @Override
-    public void detach() {
-        if (getEditor().isOpen()) {
-            getEditor().cancel();
-        }
-        super.detach();
-    }
 }
diff --git a/server/src/main/java/com/vaadin/ui/TabSheet.java b/server/src/main/java/com/vaadin/ui/TabSheet.java
index 8cb7243473..75b08bf5dc 100644
--- a/server/src/main/java/com/vaadin/ui/TabSheet.java
+++ b/server/src/main/java/com/vaadin/ui/TabSheet.java
@@ -585,12 +585,6 @@ public class TabSheet extends AbstractComponentContainer
      */
     private void setSelected(Component component) {
         Tab tab = tabs.get(selected);
-        if (tab != null && !Objects.equals(tab.getComponent(), component)
-                && tab.getComponent() != null
-                && tab.getComponent().isAttached()) {
-            tab.getComponent().detach();
-            tab.getComponent().attach(); // ugly hack
-        }
 
         selected = component;
         // Repaint of the selected component is needed as only the selected
diff --git a/server/src/test/java/com/vaadin/ui/AbstractListingTest.java b/server/src/test/java/com/vaadin/ui/AbstractListingTest.java
index 86f6929d38..77cd7779c3 100644
--- a/server/src/test/java/com/vaadin/ui/AbstractListingTest.java
+++ b/server/src/test/java/com/vaadin/ui/AbstractListingTest.java
@@ -30,9 +30,13 @@ public class AbstractListingTest {
 
         /**
          * Used to execute data generation
+         * 
+         * @param initial
+         *            {@code true} to mock initial data request; {@code false}
+         *            for follow-up request.
          */
-        public void runDataGeneration() {
-            super.getDataCommunicator().beforeClientResponse(true);
+        public void runDataGeneration(boolean initial) {
+            super.getDataCommunicator().beforeClientResponse(initial);
         }
 
         @Override
@@ -129,7 +133,7 @@ public class AbstractListingTest {
         CountGenerator generator = new CountGenerator();
         generator.extend(listing);
         listing.setItems("Foo");
-        listing.runDataGeneration();
+        listing.runDataGeneration(true);
         assertEquals("Generator should have been called once", 1,
                 generator.callCount);
     }
@@ -139,7 +143,7 @@ public class AbstractListingTest {
         CountGenerator generator = new CountGenerator();
         listing.setItems("Foo");
         generator.extend(listing);
-        listing.runDataGeneration();
+        listing.runDataGeneration(true);
         assertEquals("Generator should have been called once", 1,
                 generator.callCount);
     }
@@ -149,10 +153,10 @@ public class AbstractListingTest {
         listing.setItems("Foo");
         CountGenerator generator = new CountGenerator();
         generator.extend(listing);
-        listing.runDataGeneration();
+        listing.runDataGeneration(true);
         assertEquals("Generator should have been called once", 1,
                 generator.callCount);
-        listing.runDataGeneration();
+        listing.runDataGeneration(false);
         assertEquals("Generator should not have been called again", 1,
                 generator.callCount);
     }
@@ -163,7 +167,7 @@ public class AbstractListingTest {
         CountGenerator generator = new CountGenerator();
         generator.extend(listing);
         generator.remove();
-        listing.runDataGeneration();
+        listing.runDataGeneration(true);
         assertEquals("Generator should not have been called", 0,
                 generator.callCount);
     }
@@ -173,11 +177,11 @@ public class AbstractListingTest {
         listing.setItems("Foo");
         CountGenerator generator = new CountGenerator();
         generator.extend(listing);
-        listing.runDataGeneration();
+        listing.runDataGeneration(true);
         assertEquals("Generator should have been called once", 1,
                 generator.callCount);
         generator.refresh("Foo");
-        listing.runDataGeneration();
+        listing.runDataGeneration(false);
         assertEquals("Generator should have been called again", 2,
                 generator.callCount);
     }