Removing more than pageful in Escalator doesn't break anymore (#13334)

Change-Id: I4872766d9ed49d4a0765d28ed00e84cecf24dda4

client/src/com/vaadin/client/ui/grid/Escalator.java

@@ -2150,7 +2150,9 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker
                         "topRowLogicalIndex: " + this.topRowLogicalIndex
                                 + " -> " + topRowLogicalIndex);
             }
-            assert topRowLogicalIndex >= 0 : "topRowLogicalIndex became negative";
+            assert topRowLogicalIndex >= 0 : "topRowLogicalIndex became negative (top left cell contents: "
+                    + visualRowOrder.getFirst().getCells().getItem(0)
+                            .getInnerText() + ") ";
             /*
              * if there's a smart way of evaluating and asserting the max index,
              * this would be a nice place to put it. I haven't found out an
@@ -2882,19 +2884,52 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker
                                 removedVisualInside, 0);
                     }
 
+                    else if (removedVisualInside.contains(0)
+                            && numberOfRows >= visualRowOrder.size()) {
+                        /*
+                         * We're removing so many rows that the viewport is
+                         * pushed up more than a screenful. This means we can
+                         * simply scroll up and everything will work without a
+                         * sweat.
+                         */
+
+                        double left = horizontalScrollbar.getScrollPos();
+                        int top = contentBottom - visualRowOrder.size()
+                                * getDefaultRowHeight();
+                        setBodyScrollPosition(left, top);
+
+                        Range allEscalatorRows = Range.withLength(0,
+                                visualRowOrder.size());
+                        int logicalTargetIndex = getRowCount()
+                                - allEscalatorRows.length();
+                        moveAndUpdateEscalatorRows(allEscalatorRows, 0,
+                                logicalTargetIndex);
+
+                        /*
+                         * Scrolling the body to the correct location will be
+                         * fixed automatically. Because the amount of rows is
+                         * decreased, the viewport is pushed up as the scrollbar
+                         * shrinks. So no need to do anything there.
+                         * 
+                         * TODO [[optimize]]: This might lead to a double body
+                         * refresh. Needs investigation.
+                         */
+                    }
+
                     else if (contentBottom
                             + (numberOfRows * getDefaultRowHeight())
                             - viewportBottom < getDefaultRowHeight()) {
                         /*
-                         * FIXME [[rowheight]]: above coded to work only with
-                         * default row heights - will not work with variable row
-                         * heights
+                         * We're at the end of the row container, everything is
+                         * added to the top.
                          */
 
                         /*
-                         * We're at the end of the row container, everything is
-                         * added to the top.
+                         * FIXME [[rowheight]]: above if-clause is coded to only
+                         * work with default row heights - will not work with
+                         * variable row heights
                          */
+
                         paintRemoveRowsAtBottom(removedLogicalInside,
                                 removedVisualInside);
                         updateTopRowLogicalIndex(-removedLogicalInside.length());

uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java

@@ -38,6 +38,8 @@ 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 REMOVE_50_ROWS_FROM_BOTTOM = "Remove 50 rows from bottom";
+    protected static final String REMOVE_50_ROWS_FROM_ALMOST_BOTTOM = "Remove 50 rows from almost bottom";
     protected static final String ADD_ONE_OF_EACH_ROW = "Add one of each row";
     protected static final String RESIZE_FIRST_COLUMN_TO_MAX_WIDTH = "Resize first column to max width";
 
@@ -75,30 +77,72 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest
         }
     }
 
+    /**
+     * @param row
+     *            the index of the row element in the section. If negative, the
+     *            calculation starts from the end (-1 is the last, -2 is the
+     *            second-to-last etc)
+     */
     protected WebElement getHeaderRow(int row) {
         return getRow("thead", row);
     }
 
+    /**
+     * @param row
+     *            the index of the row element in the section. If negative, the
+     *            calculation starts from the end (-1 is the last, -2 is the
+     *            second-to-last etc)
+     */
     protected WebElement getBodyRow(int row) {
         return getRow("tbody", row);
     }
 
+    /**
+     * @param row
+     *            the index of the row element in the section. If negative, the
+     *            calculation starts from the end (-1 is the last, -2 is the
+     *            second-to-last etc)
+     */
     protected WebElement getFooterRow(int row) {
         return getRow("tfoot", row);
     }
 
+    /**
+     * @param row
+     *            the index of the row element in the section. If negative, the
+     *            calculation starts from the end (-1 is the last, -2 is the
+     *            second-to-last etc)
+     */
     protected WebElement getHeaderCell(int row, int col) {
         return getCell("thead", row, col);
     }
 
+    /**
+     * @param row
+     *            the index of the row element in the section. If negative, the
+     *            calculation starts from the end (-1 is the last, -2 is the
+     *            second-to-last etc)
+     */
     protected WebElement getBodyCell(int row, int col) {
         return getCell("tbody", row, col);
     }
 
+    /**
+     * @param row
+     *            the index of the row element in the section. If negative, the
+     *            calculation starts from the end (-1 is the last, -2 is the
+     *            second-to-last etc)
+     */
     protected WebElement getFooterCell(int row, int col) {
         return getCell("tfoot", row, col);
     }
 
+    /**
+     * @param row
+     *            the index of the row element in the section. If negative, the
+     *            calculation starts from the end (-1 is the last, -2 is the
+     *            second-to-last etc)
+     */
     private WebElement getCell(String sectionTag, int row, int col) {
         WebElement rowElement = getRow(sectionTag, row);
         if (rowElement != null) {
@@ -112,12 +156,26 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest
         }
     }
 
+    /**
+     * @param row
+     *            the index of the row element in the section. If negative, the
+     *            calculation starts from the end (-1 is the last, -2 is the
+     *            second-to-last etc)
+     */
     private WebElement getRow(String sectionTag, int row) {
         WebElement escalator = getEscalator();
         WebElement tableSection = escalator.findElement(By.tagName(sectionTag));
 
         try {
-            return tableSection.findElement(By.xpath("tr[" + (row + 1) + "]"));
+            if (row >= 0) {
+                int fromFirst = row + 1;
+                return tableSection.findElement(By.xpath("tr[" + fromFirst
+                        + "]"));
+            } else {
+                int fromLast = Math.abs(row + 1);
+                return tableSection.findElement(By.xpath("tr[last() - "
+                        + fromLast + "]"));
+            }
         } catch (NoSuchElementException e) {
             return null;
         }

uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorRowColumnTest.java

@@ -15,6 +15,7 @@
  */
 package com.vaadin.tests.components.grid.basicfeatures;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -24,6 +25,12 @@ import org.openqa.selenium.By;
 
 public class EscalatorRowColumnTest extends EscalatorBasicClientFeaturesTest {
 
+    /**
+     * The scroll position of the Escalator when scrolled all the way down, to
+     * reveal the 100:th row.
+     */
+    private static final int BOTTOM_SCROLL_POSITION = 1857;
+
     @Test
     public void testInit() {
         openTestURL();
@@ -214,4 +221,87 @@ public class EscalatorRowColumnTest extends EscalatorBasicClientFeaturesTest {
         assertNotEquals("Column width should've changed", originalWidth,
                 newWidth);
     }
+
+    @Test
+    public void testRemoveMoreThanPagefulAtBottomWhileScrolledToBottom()
+            throws Exception {
+        openTestURL();
+        selectMenuPath(GENERAL, POPULATE_COLUMN_ROW);
+
+        scrollVerticallyTo(BOTTOM_SCROLL_POSITION);
+        selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, REMOVE_50_ROWS_FROM_BOTTOM);
+        assertEquals("Row 49: 0,49", getBodyCell(-1, 0).getText());
+
+        scrollVerticallyTo(0);
+
+        // let the DOM organize itself
+        Thread.sleep(500);
+
+        // if something goes wrong, it'll explode before this.
+        assertEquals("Row 0: 0,0", getBodyCell(0, 0).getText());
+    }
+
+    @Test
+    public void testRemoveMoreThanPagefulAtBottomWhileScrolledAlmostToBottom()
+            throws Exception {
+        openTestURL();
+        selectMenuPath(GENERAL, POPULATE_COLUMN_ROW);
+
+        // bottom minus 15 rows.
+        scrollVerticallyTo(BOTTOM_SCROLL_POSITION - 15 * 20);
+        selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, REMOVE_50_ROWS_FROM_BOTTOM);
+        assertEquals("Row 49: 0,49", getBodyCell(-1, 0).getText());
+
+        scrollVerticallyTo(0);
+
+        // let the DOM organize itself
+        Thread.sleep(500);
+
+        // if something goes wrong, it'll explode before this.
+        assertEquals("Row 0: 0,0", getBodyCell(0, 0).getText());
+    }
+
+    @Test
+    public void testRemoveMoreThanPagefulNearBottomWhileScrolledToBottom()
+            throws Exception {
+        openTestURL();
+        selectMenuPath(GENERAL, POPULATE_COLUMN_ROW);
+
+        scrollVerticallyTo(BOTTOM_SCROLL_POSITION);
+        selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS,
+                REMOVE_50_ROWS_FROM_ALMOST_BOTTOM);
+        assertEquals("Row 49: 0,99", getBodyCell(-1, 0).getText());
+
+        scrollVerticallyTo(0);
+
+        // let the DOM organize itself
+        Thread.sleep(500);
+
+        // if something goes wrong, it'll explode before this.
+        assertEquals("Row 0: 0,0", getBodyCell(0, 0).getText());
+    }
+
+    @Test
+    public void testRemoveMoreThanPagefulNearBottomWhileScrolledAlmostToBottom()
+            throws Exception {
+        openTestURL();
+        selectMenuPath(GENERAL, POPULATE_COLUMN_ROW);
+
+        // bottom minus 15 rows.
+        scrollVerticallyTo(BOTTOM_SCROLL_POSITION - 15 * 20);
+        selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS,
+                REMOVE_50_ROWS_FROM_ALMOST_BOTTOM);
+
+        // let the DOM organize itself
+        Thread.sleep(500);
+        assertEquals("Row 49: 0,99", getBodyCell(-1, 0).getText());
+
+        scrollVerticallyTo(0);
+
+        // let the DOM organize itself
+        Thread.sleep(500);
+
+        // if something goes wrong, it'll explode before this.
+        assertEquals("Row 0: 0,0", getBodyCell(0, 0).getText());
+    }
 }

uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java

@@ -514,6 +514,14 @@ public class EscalatorBasicClientFeaturesWidget extends
                         .getRowCount() - 50, 50);
             }
         }, menupath);
+        addMenuCommand("Remove 50 rows from almost bottom",
+                new ScheduledCommand() {
+                    @Override
+                    public void execute() {
+                        removeRows(escalator.getBody(), escalator.getBody()
+                                .getRowCount() - 60, 50);
+                    }
+                }, menupath);
         addMenuCommand("Remove all, insert 30 and scroll 40px",
                 new ScheduledCommand() {
                     @Override