Fixes an issue with onUnload() (#13334)

Change-Id: I7ed22d97ee861208822f845068b7eee856392c07

4 files changed, 110 insertions, 11 deletions

diff --git a/client/src/com/vaadin/client/ui/grid/Escalator.java b/client/src/com/vaadin/client/ui/grid/Escalator.java
index be831b5b61..c054331c00 100644
--- a/client/src/com/vaadin/client/ui/grid/Escalator.java
+++ b/client/src/com/vaadin/client/ui/grid/Escalator.java
@@ -2646,10 +2646,7 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker
                 return;
             }
 
-            final Range viewportRange = Range.withLength(
-                    getLogicalRowIndex(visualRowOrder.getFirst()),
-                    visualRowOrder.size());
-
+            final Range viewportRange = getVisibleRowRange();
             final Range removedRowsRange = Range
                     .withLength(index, numberOfRows);
 
@@ -2721,12 +2718,12 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker
                 int escalatorRowCount = bodyElem.getChildCount();
 
                 /*
-                 * If we're left with less rows than the number of escalators,
-                 * remove the unused ones.
+                 * remember: the rows have already been subtracted from the row
+                 * count at this point
                  */
-                final int escalatorRowsToRemove = escalatorRowCount
-                        - getRowCount();
-                if (escalatorRowsToRemove > 0) {
+                int rowsLeft = getRowCount();
+                if (rowsLeft < escalatorRowCount) {
+                    int escalatorRowsToRemove = escalatorRowCount - rowsLeft;
                     for (int i = 0; i < escalatorRowsToRemove; i++) {
                         final TableRowElement tr = visualRowOrder
                                 .remove(removedVisualInside.getStart());
@@ -4056,9 +4053,27 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker
         scroller.detachMousewheelListener(getElement());
         scroller.detachTouchListeners(getElement());
 
+        /*
+         * We can call paintRemoveRows here, because static ranges are simple to
+         * remove.
+         */
         header.paintRemoveRows(0, header.getRowCount());
         footer.paintRemoveRows(0, footer.getRowCount());
-        body.paintRemoveRows(0, body.getRowCount());
+
+        /*
+         * We can't call body.paintRemoveRows since it relies on rowCount to be
+         * updated correctly. Since it isn't, we'll simply and brutally rip out
+         * the DOM elements (in an elegant way, of course).
+         */
+        int rowsToRemove = bodyElem.getChildCount();
+        for (int i = 0; i < rowsToRemove; i++) {
+            int index = rowsToRemove - i - 1;
+            TableRowElement tr = bodyElem.getRows().getItem(index);
+            body.paintRemoveRow(tr, index);
+            body.removeRowPosition(tr);
+        }
+        body.visualRowOrder.clear();
+        body.setTopRowLogicalIndex(0);
 
         super.onUnload();
     }
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java
index 8f29a71536..0c58b01062 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java
@@ -38,6 +38,7 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest
     protected static final String ADD_ONE_ROW_TO_BEGINNING = "Add one row to beginning";
     protected static final String REMOVE_ONE_COLUMN_FROM_BEGINNING = "Remove one column from beginning";
     protected static final String REMOVE_ONE_ROW_FROM_BEGINNING = "Remove one row from beginning";
+    protected static final String ADD_ONE_OF_EACH_ROW = "Add one of each row";
 
     protected static final String HEADER_ROWS = "Header Rows";
     protected static final String BODY_ROWS = "Body Rows";
@@ -46,6 +47,7 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest
     protected static final String REMOVE_ALL_INSERT_SCROLL = "Remove all, insert 30 and scroll 40px";
 
     protected static final String GENERAL = "General";
+    protected static final String DETACH_ESCALATOR = "Detach Escalator";
     protected static final String POPULATE_COLUMN_ROW = "Populate Escalator (columns, then rows)";
     protected static final String POPULATE_ROW_COLUMN = "Populate Escalator (rows, then columns)";
     protected static final String CLEAR_COLUMN_ROW = "Clear (columns, then rows)";
@@ -65,7 +67,11 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest
     }
 
     protected WebElement getEscalator() {
-        return getDriver().findElement(By.className("v-escalator"));
+        try {
+            return getDriver().findElement(By.className("v-escalator"));
+        } catch (NoSuchElementException e) {
+            return null;
+        }
     }
 
     protected WebElement getHeaderRow(int row) {
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicsTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicsTest.java
new file mode 100644
index 0000000000..bac5c24fbe
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicsTest.java
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * 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
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.grid.basicfeatures;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.NotificationElement;
+
+public class EscalatorBasicsTest extends EscalatorBasicClientFeaturesTest {
+
+    @Test
+    public void testDetachingAnEmptyEscalator() {
+        setDebug(true);
+        openTestURL();
+
+        selectMenuPath(GENERAL, DETACH_ESCALATOR);
+        assertEscalatorIsRemovedCorrectly();
+    }
+
+    @Test
+    public void testDetachingASemiPopulatedEscalator() {
+        setDebug(true);
+        openTestURL();
+
+        selectMenuPath(COLUMNS_AND_ROWS, ADD_ONE_OF_EACH_ROW);
+        selectMenuPath(COLUMNS_AND_ROWS, COLUMNS, ADD_ONE_COLUMN_TO_BEGINNING);
+        selectMenuPath(GENERAL, DETACH_ESCALATOR);
+        assertEscalatorIsRemovedCorrectly();
+    }
+
+    @Test
+    public void testDetachingAPopulatedEscalator() {
+        setDebug(true);
+        openTestURL();
+
+        selectMenuPath(GENERAL, POPULATE_COLUMN_ROW);
+        selectMenuPath(GENERAL, DETACH_ESCALATOR);
+        assertEscalatorIsRemovedCorrectly();
+    }
+
+    private void assertEscalatorIsRemovedCorrectly() {
+        assertFalse($(NotificationElement.class).exists());
+        assertNull(getEscalator());
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java
index 987dcc1bb7..60b9d5cc7d 100644
--- a/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java
+++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java
@@ -368,6 +368,23 @@ public class EscalatorBasicClientFeaturesWidget extends
 
     private void createGeneralMenu() {
         String[] menupath = { GENERAL_MENU };
+
+        addMenuCommand("Detach Escalator", new ScheduledCommand() {
+            @Override
+            public void execute() {
+                escalator.removeFromParent();
+            }
+        }, menupath);
+
+        addMenuCommand("Attach Escalator", new ScheduledCommand() {
+            @Override
+            public void execute() {
+                if (!escalator.isAttached()) {
+                    addNorth(escalator, 500);
+                }
+            }
+        }, menupath);
+
         addMenuCommand("Clear (columns, then rows)", new ScheduledCommand() {
             @Override
             public void execute() {