Re-adding content in Table causes table to lose scroll position (#14581)

At this moment behavior of restoring scroll position more like as in
7.2.6. But restoring is only in case of removeAll() - addAll() (or add()
one time - then restore index is reset).

In 7.2.6 restoring of scroll position was the result of client defect
(scrolling in lazyScroller was not changed if variable "firstvisible"
fromserver was 0). This "defect" was fixed in one of the patches.

Change-Id: I2e2fb8749ec95f3409caeacafff46c4c29159e74

2 files changed, 228 insertions, 21 deletions

diff --git a/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRows.java b/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRows.java
index d9cbf007df..df06580dae 100644
--- a/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRows.java
+++ b/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRows.java
@@ -11,6 +11,12 @@ import com.vaadin.ui.Button;
 import com.vaadin.ui.Button.ClickListener;
 import com.vaadin.ui.Table;
 
+/**
+ * Scroll position should be restored when removing and re-adding all rows in
+ * Table.
+ * 
+ * @author Vaadin Ltd
+ */
 public class TableRepairsScrollPositionOnReAddingAllRows extends AbstractTestUI {
 
     private static final long serialVersionUID = 1L;
@@ -19,32 +25,130 @@ public class TableRepairsScrollPositionOnReAddingAllRows extends AbstractTestUI
     protected void setup(VaadinRequest request) {
         final BeanItemContainer<TableItem> cont = new BeanItemContainer<TableItem>(
                 TableItem.class);
-        final List<TableItem> itemList = new ArrayList<TableItem>();
+        final List<TableItem> restoringItemList = new ArrayList<TableItem>();
 
-        Button button1 = new Button("ReAdd rows");
-        button1.setId("button1");
-        button1.addClickListener(new ClickListener() {
+        final Table table = new Table();
+        table.setWidth("400px");
+        table.setPageLength(-1);
+        table.setContainerDataSource(cont);
+        table.setSelectable(true);
+
+        Button buttonRestore = new Button("Restore table rows");
+        buttonRestore.setId("buttonRestore");
+        buttonRestore.addClickListener(new ClickListener() {
 
             @Override
             public void buttonClick(com.vaadin.ui.Button.ClickEvent event) {
                 cont.removeAllItems();
-                cont.addAll(itemList);
+                cont.addAll(restoringItemList);
             }
         });
 
+        Button buttonReAddAllViaAddAll = new Button("Re-add rows all at once");
+        buttonReAddAllViaAddAll.setId("buttonReAddAllViaAddAll");
+        buttonReAddAllViaAddAll.addClickListener(new ClickListener() {
+
+            @Override
+            public void buttonClick(com.vaadin.ui.Button.ClickEvent event) {
+                List<TableItem> originalItemIds = new ArrayList<TableItem>(cont
+                        .getItemIds());
+                cont.removeAllItems();
+                cont.addAll(originalItemIds);
+            }
+        });
+
+        Button buttonReplaceByAnotherCollectionViaAddAll = new Button(
+                "Replace by another items (via addAll())");
+        buttonReplaceByAnotherCollectionViaAddAll
+                .setId("buttonReplaceByAnotherCollectionViaAddAll");
+        buttonReplaceByAnotherCollectionViaAddAll
+                .addClickListener(new ClickListener() {
+
+                    @Override
+                    public void buttonClick(
+                            com.vaadin.ui.Button.ClickEvent event) {
+                        cont.removeAllItems();
+                        // create new collection (of different items) with other
+                        // size
+                        List<TableItem> itemList = new ArrayList<TableItem>();
+                        for (int i = 0; i < 79; i++) {
+                            TableItem ti = new TableItem();
+                            ti.setName("AnotherItem1_" + i);
+                            itemList.add(ti);
+                        }
+                        cont.addAll(itemList);
+                    }
+                });
+
+        Button buttonReplaceByAnotherCollectionViaAdd = new Button(
+                "Replace by another items (via add(), add()..)");
+        buttonReplaceByAnotherCollectionViaAdd
+                .setId("buttonReplaceByAnotherCollectionViaAdd");
+        buttonReplaceByAnotherCollectionViaAdd
+                .addClickListener(new ClickListener() {
+
+                    @Override
+                    public void buttonClick(
+                            com.vaadin.ui.Button.ClickEvent event) {
+                        cont.removeAllItems();
+                        for (int i = 0; i < 81; i++) {
+                            TableItem ti = new TableItem();
+                            ti.setName("AnotherItem2_" + i);
+                            // add one by one in container
+                            cont.addBean(ti);
+                        }
+                    }
+                });
+
+        Button buttonReplaceBySubsetOfSmallerSize = new Button(
+                "Replace rows by sub-set of smaller size (size not enought for restoring scroll position)");
+        buttonReplaceBySubsetOfSmallerSize
+                .setId("buttonReplaceBySubsetOfSmallerSize");
+        buttonReplaceBySubsetOfSmallerSize
+                .addClickListener(new ClickListener() {
+
+                    @Override
+                    public void buttonClick(
+                            com.vaadin.ui.Button.ClickEvent event) {
+                        cont.removeAllItems();
+                        cont.addAll(restoringItemList.subList(0, 20));
+                    }
+                });
+
+        Button buttonReplaceByWholeSubsetPlusOnNew = new Button(
+                "Replace rows by whole subset plus one new item");
+        buttonReplaceByWholeSubsetPlusOnNew
+                .setId("buttonReplaceByWholeSubsetPlusOnNew");
+        buttonReplaceByWholeSubsetPlusOnNew
+                .addClickListener(new ClickListener() {
+
+                    @Override
+                    public void buttonClick(
+                            com.vaadin.ui.Button.ClickEvent event) {
+                        cont.removeAllItems();
+
+                        List<TableItem> list = new ArrayList<TableItem>(
+                                restoringItemList);
+                        TableItem ti = new TableItem();
+                        ti.setName("AnotherItem3_" + 80);
+                        list.add(ti);
+                        cont.addAll(list);
+                    }
+                });
+
         for (int i = 0; i < 80; i++) {
             TableItem ti = new TableItem();
-            ti.setName("Name_" + i);
-            itemList.add(ti);
+            ti.setName("Item_" + i);
+            restoringItemList.add(ti);
             cont.addBean(ti);
         }
 
-        final Table table = new Table();
-        table.setPageLength(-1);
-        table.setContainerDataSource(cont);
-        table.setSelectable(true);
-
-        getLayout().addComponent(button1);
+        getLayout().addComponent(buttonReAddAllViaAddAll);
+        getLayout().addComponent(buttonReplaceByAnotherCollectionViaAddAll);
+        getLayout().addComponent(buttonReplaceByAnotherCollectionViaAdd);
+        getLayout().addComponent(buttonReplaceBySubsetOfSmallerSize);
+        getLayout().addComponent(buttonReplaceByWholeSubsetPlusOnNew);
+        getLayout().addComponent(buttonRestore);
         getLayout().addComponent(table);
     }
 
diff --git a/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRowsTest.java b/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRowsTest.java
index 2fbdd8b581..a3e7f29dfe 100644
--- a/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRowsTest.java
+++ b/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRowsTest.java
@@ -31,6 +31,12 @@ import com.vaadin.testbench.screenshot.ImageComparison;
 import com.vaadin.testbench.screenshot.ReferenceNameGenerator;
 import com.vaadin.tests.tb3.MultiBrowserTest;
 
+/**
+ * Scroll position should be restored when removing and re-adding all rows in
+ * Table.
+ * 
+ * @author Vaadin Ltd
+ */
 public class TableRepairsScrollPositionOnReAddingAllRowsTest extends
         MultiBrowserTest {
 
@@ -39,29 +45,126 @@ public class TableRepairsScrollPositionOnReAddingAllRowsTest extends
             throws InterruptedException {
         openTestURL();
 
-        WebElement buttonReAddRows = findElement(By.id("button1"));
+        WebElement row0 = $(TableElement.class).first().getCell(0, 0);
+        int rowLocation0 = row0.getLocation().getY();
 
+        // case 1
         scrollUp();
 
         waitUntilNot(new ExpectedCondition<Boolean>() {
             @Override
             public Boolean apply(WebDriver input) {
-                return $(TableElement.class).first().getCell(49, 0) == null;
+                return $(TableElement.class).first().getCell(48, 0) == null;
             }
         }, 10);
 
-        WebElement row = $(TableElement.class).first().getCell(49, 0);
+        WebElement row = $(TableElement.class).first().getCell(48, 0);
         int rowLocation = row.getLocation().getY();
 
-        buttonReAddRows.click();
+        // This button is for re-adding all rows (original itemIds) at once
+        // (removeAll() + addAll())
+        hitButton("buttonReAddAllViaAddAll");
 
-        row = $(TableElement.class).first().getCell(49, 0);
+        row = $(TableElement.class).first().getCell(48, 0);
         int newRowLocation = row.getLocation().getY();
 
         // ranged check because IE9 consistently misses the mark by 1 pixel
         assertThat(
-                "Scroll position should be the same as before Re-Adding all rows",
-                (double) newRowLocation, closeTo(rowLocation, 1));
+                "Scroll position should be the same as before Re-Adding rows via addAll()",
+                (double) newRowLocation,
+                closeTo(rowLocation, row.getSize().height + 1));
+
+        // case 2
+        scrollUp();
+
+        waitUntilNot(new ExpectedCondition<Boolean>() {
+            @Override
+            public Boolean apply(WebDriver input) {
+                return $(TableElement.class).first().getCell(48, 0) == null;
+            }
+        }, 10);
+
+        row = $(TableElement.class).first().getCell(48, 0);
+        rowLocation = row.getLocation().getY();
+
+        // This button is for replacing all rows at once (removeAll() +
+        // addAll())
+        hitButton("buttonReplaceByAnotherCollectionViaAddAll");
+
+        row = $(TableElement.class).first().getCell(48, 0);
+        newRowLocation = row.getLocation().getY();
+
+        // ranged check because IE9 consistently misses the mark by 1 pixel
+        assertThat(
+                "Scroll position should be the same as before Replacing rows via addAll()",
+                (double) newRowLocation,
+                closeTo(rowLocation, row.getSize().height + 1));
+
+        // case 3
+        // This button is for replacing all rows one by one (removeAll() + add()
+        // + add()..)
+        hitButton("buttonReplaceByAnotherCollectionViaAdd");
+
+        row = $(TableElement.class).first().getCell(0, 0);
+        newRowLocation = row.getLocation().getY();
+
+        // ranged check because IE9 consistently misses the mark by 1 pixel
+        assertThat("Scroll position should be 0", (double) newRowLocation,
+                closeTo(rowLocation0, 1));
+
+        // case 4
+        // This button is for restoring initial list and for scrolling to 0
+        // position
+        hitButton("buttonRestore");
+        scrollUp();
+
+        waitUntilNot(new ExpectedCondition<Boolean>() {
+            @Override
+            public Boolean apply(WebDriver input) {
+                return $(TableElement.class).first().getCell(48, 0) == null;
+            }
+        }, 10);
+
+        // This button is for replacing all rows at once but the count of rows
+        // is less then first index to scroll
+        hitButton("buttonReplaceBySubsetOfSmallerSize");
+
+        row = $(TableElement.class).first().getCell(5, 0);
+
+        newRowLocation = row.getLocation().getY();
+
+        // ranged check because IE9 consistently misses the mark by 1 pixel
+        assertThat("Scroll position should be 0", (double) newRowLocation,
+                closeTo(rowLocation0, 1));
+
+        // case 5
+        // This button is for restoring initial list and for scrolling to 0
+        // position
+        hitButton("buttonRestore");
+        scrollUp();
+
+        waitUntilNot(new ExpectedCondition<Boolean>() {
+            @Override
+            public Boolean apply(WebDriver input) {
+                return $(TableElement.class).first().getCell(48, 0) == null;
+            }
+        }, 10);
+
+        row = $(TableElement.class).first().getCell(48, 0);
+        rowLocation = row.getLocation().getY();
+
+        // This button is for replacing by whole original sub-set of items plus
+        // one new
+        hitButton("buttonReplaceByWholeSubsetPlusOnNew");
+
+        row = $(TableElement.class).first().getCell(48, 0);
+        newRowLocation = row.getLocation().getY();
+
+        // ranged check because IE9 consistently misses the mark by 1 pixel
+        assertThat("Scroll position should be the same as before Replacing",
+                (double) newRowLocation,
+                closeTo(rowLocation, row.getSize().height + 1));
+
     }
 
     private void scrollUp() {
@@ -69,6 +172,6 @@ public class TableRepairsScrollPositionOnReAddingAllRowsTest extends
                 By.className("v-table-body-wrapper"));
         JavascriptExecutor js = new TestBenchCommandExecutor(getDriver(),
                 new ImageComparison(), new ReferenceNameGenerator());
-        js.executeScript("arguments[0].scrollTop = " + 1200, actualElement);
+        js.executeScript("arguments[0].scrollTop = " + 1205, actualElement);
     }
 }