Fix occasional empty rows in Table and TreeTable (#9551)

There's an intermittently happening issue with both Table and TreeTable, which results in row data disappearing.
This change removes a method which is probably a vestigial one from over five years ago and other changes are handling the things the method used to perform. Currently the method removes rows deemed unnecessary from the row buffer. The problem is, those rows are visible to the user and removing causes row contents to be lost.
Also included are manually runnable test cases which demonstrate that this removal actually prevents the issue from happening.

Fixes #7964
Fixes #5030

diff --git a/server/src/main/java/com/vaadin/ui/Table.java b/server/src/main/java/com/vaadin/ui/Table.java
index 15f0672fb9af743e28facdce0f388ad88ca39bb3..e59ac47c8493bbd99cd495a0407699d001ce4a7a 100644 (file)
--- a/server/src/main/java/com/vaadin/ui/Table.java
+++ b/server/src/main/java/com/vaadin/ui/Table.java
@@ -1749,9 +1749,6 @@ public class Table extends AbstractSelect implements Action.Container,
         if (rows > 0) {
             pageBufferFirstIndex = firstIndex;
         }
-        if (getPageLength() != 0) {
-            removeUnnecessaryRows();
-        }
 
         setRowCacheInvalidated(true);
         markAsDirty();
@@ -1817,48 +1814,6 @@ public class Table extends AbstractSelect implements Action.Container,
 
     }
 
-    /**
-     * Removes rows that fall outside the required cache.
-     */
-    private void removeUnnecessaryRows() {
-        int minPageBufferIndex = getMinPageBufferIndex();
-        int maxPageBufferIndex = getMaxPageBufferIndex();
-
-        int maxBufferSize = maxPageBufferIndex - minPageBufferIndex + 1;
-
-        /*
-         * Number of rows that were previously cached. This is not necessarily
-         * the same as pageLength if we do not have enough rows in the
-         * container.
-         */
-        int currentlyCachedRowCount = pageBuffer[CELL_ITEMID].length;
-
-        if (currentlyCachedRowCount <= maxBufferSize) {
-            // removal unnecessary
-            return;
-        }
-
-        /* Figure out which rows to get rid of. */
-        int firstCacheRowToRemoveInPageBuffer = -1;
-        if (minPageBufferIndex > pageBufferFirstIndex) {
-            firstCacheRowToRemoveInPageBuffer = pageBufferFirstIndex;
-        } else if (maxPageBufferIndex < pageBufferFirstIndex
-                + currentlyCachedRowCount) {
-            firstCacheRowToRemoveInPageBuffer = maxPageBufferIndex + 1;
-        }
-
-        if (firstCacheRowToRemoveInPageBuffer
-                - pageBufferFirstIndex < currentlyCachedRowCount) {
-            /*
-             * Unregister all components that fall beyond the cache limits after
-             * inserting the new rows.
-             */
-            unregisterComponentsAndPropertiesInRows(
-                    firstCacheRowToRemoveInPageBuffer, currentlyCachedRowCount
-                            - firstCacheRowToRemoveInPageBuffer);
-        }
-    }
-
     /**
      * Requests that the Table should be repainted as soon as possible.
      *

diff --git a/uitest/src/main/java/com/vaadin/tests/components/table/ComponentsDisappearWhenScrolling.java b/uitest/src/main/java/com/vaadin/tests/components/table/ComponentsDisappearWhenScrolling.java
new file mode 100644 (file)
index 0000000..234a7c9
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/table/ComponentsDisappearWhenScrolling.java
@@ -0,0 +1,40 @@
+package com.vaadin.tests.components.table;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.shared.ui.label.ContentMode;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.Table;
+
+public class ComponentsDisappearWhenScrolling extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        Label label = new Label(getDescription(), ContentMode.HTML);
+        addComponent(label);
+
+        Table table = new Table();
+        table.setSizeFull();
+        table.setCacheRate(1.1);
+        table.addContainerProperty(0, Button.class, null);
+        for (int i = 0; i < 100; i++) {
+            table.addItem(new Object[] { new Button() }, i);
+        }
+        addComponent(table);
+    }
+
+    @Override
+    public String getDescription() {
+        return "Scroll all the way down, then slowly back up. More often than" +
+                " not this results in 'flattening' of several rows. This is " +
+                "due to component connectors being unregistered on " +
+                "components, which are on visible table rows.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 7964;
+    }
+
+}

diff --git a/uitest/src/main/java/com/vaadin/tests/components/treetable/TreeTableComponentsDisappearWhenScrolling.java b/uitest/src/main/java/com/vaadin/tests/components/treetable/TreeTableComponentsDisappearWhenScrolling.java
new file mode 100644 (file)
index 0000000..33e6271
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/treetable/TreeTableComponentsDisappearWhenScrolling.java
@@ -0,0 +1,47 @@
+package com.vaadin.tests.components.treetable;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.shared.ui.label.ContentMode;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.TreeTable;
+
+public class TreeTableComponentsDisappearWhenScrolling extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        Label label = new Label(getDescription(), ContentMode.HTML);
+        addComponent(label);
+
+        TreeTable table = new TreeTable();
+        table.setSizeFull();
+        table.setCacheRate(1.1);
+        table.addContainerProperty(0, Button.class, null);
+        for (int i = 0; i < 100; i++) {
+            table.addItem(new Object[] { new Button(Integer.toString(i)) }, i);
+            if ((i + 1) % 5 == 0) {
+                for (int j = 0; j < 4; j++) {
+                    table.setParent(i - j, i - 4);
+                }
+            }
+        }
+
+        addComponent(table);
+    }
+
+    @Override
+    public String getDescription() {
+        return "Expand nodes and scroll all the way down, then slowly back up" +
+                ". More often than" +
+                " not this results in 'flattening' of several rows. This is " +
+                "due to component connectors being unregistered on " +
+                "components, which are on visible table rows.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 7964;
+    }
+
+}