Updated row and spacer handling for Escalator (#11438)

Updated row and spacer handling for Escalator.

Main changes:
- Spacers are only maintained and checked for rows that have DOM
representation, and not at all if there is no details generator. This
gives notable performance improvements to some particularly large Grids
- Escalator no longer tries to trim away any rows that don't fit within
the viewport just because a details row gets opened in Grid. This leads
to some increase in simultaneous DOM elements, but simplifies the logic
considerably. For example opening or closing details rows doesn't
require checking the overall content validity beyond the details row
itself anymore, but some repositioning at most. There are also no longer
any orphaned spacers without corresponding DOM rows.
- Spacers are better integrated into the overall position calculations.
- Some public methods that are no longer used by Escalator or have
changed functionality or order of operations. Any extending classes that
tap into row, spacer, or scroll position handling are likely to need
reworking after this update.
- Auto-detecting row height is delayed until Escalator is both attached
and displayed.

7 files changed, 561 insertions, 97 deletions

diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java
index 3ff3d61809..d0122cd9df 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridComponentsTest.java
@@ -1,5 +1,9 @@
 package com.vaadin.tests.components.grid;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 import java.util.Locale;
 import java.util.stream.IntStream;
 import java.util.stream.Stream;
@@ -21,10 +25,6 @@ import com.vaadin.testbench.elements.TextFieldElement;
 import com.vaadin.testbench.parallel.BrowserUtil;
 import com.vaadin.tests.tb3.MultiBrowserTest;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
 public class GridComponentsTest extends MultiBrowserTest {
 
     @Test
@@ -219,19 +219,14 @@ public class GridComponentsTest extends MultiBrowserTest {
                 getScrollLeft(grid));
 
         // Navigate back to fully visible TextField
-        new Actions(getDriver()).sendKeys(Keys.chord(Keys.SHIFT, Keys.TAB))
-                .perform();
+        pressKeyWithModifier(Keys.SHIFT, Keys.TAB);
         assertEquals(
                 "Grid should not scroll when focusing the text field again. ",
                 scrollMax, getScrollLeft(grid));
 
         // Navigate to out of viewport TextField in Header
-        new Actions(getDriver()).sendKeys(Keys.chord(Keys.SHIFT, Keys.TAB))
-                .perform();
-        // After Chrome 75, sendkeys issues
-        if (BrowserUtil.isChrome(getDesiredCapabilities())) {
-            grid.getHeaderCell(1, 0).findElement(By.id("headerField")).click();
-        }
+        pressKeyWithModifier(Keys.SHIFT, Keys.TAB);
+
         assertEquals("Focus should be in TextField in Header", "headerField",
                 getFocusedElement().getAttribute("id"));
         assertEquals("Grid should've scrolled back to start.", 0,
@@ -248,10 +243,30 @@ public class GridComponentsTest extends MultiBrowserTest {
 
         // Navigate to currently out of viewport TextField on Row 8
         new Actions(getDriver()).sendKeys(Keys.TAB, Keys.TAB).perform();
-        assertTrue("Grid should be scrolled to show row 7",
+        assertTrue("Grid should be scrolled to show row 8",
                 Integer.parseInt(grid.getVerticalScroller()
                         .getAttribute("scrollTop")) > scrollTopRow7);
 
+        // Focus button in first visible row of Grid
+        grid.getCell(2, 2).findElement(By.id("row_2")).click();
+        int scrollTopRow2 = Integer
+                .parseInt(grid.getVerticalScroller().getAttribute("scrollTop"));
+
+        // Navigate to currently out of viewport Button on Row 1
+        pressKeyWithModifier(Keys.SHIFT, Keys.TAB);
+        pressKeyWithModifier(Keys.SHIFT, Keys.TAB);
+        int scrollTopRow1 = Integer
+                .parseInt(grid.getVerticalScroller().getAttribute("scrollTop"));
+        assertTrue("Grid should be scrolled to show row 1",
+                scrollTopRow1 < scrollTopRow2);
+
+        // Continue further to the very first row
+        pressKeyWithModifier(Keys.SHIFT, Keys.TAB);
+        pressKeyWithModifier(Keys.SHIFT, Keys.TAB);
+        assertTrue("Grid should be scrolled to show row 0",
+                Integer.parseInt(grid.getVerticalScroller()
+                        .getAttribute("scrollTop")) < scrollTopRow1);
+
         // Focus button in last row of Grid
         grid.getCell(999, 2).findElement(By.id("row_999")).click();
         // Navigate to out of viewport TextField in Footer
@@ -288,4 +303,12 @@ public class GridComponentsTest extends MultiBrowserTest {
         assertFalse("Row " + i + " should not have a button",
                 row.getCell(2).isElementPresent(ButtonElement.class));
     }
+
+    // Workaround for Chrome 75, sendKeys(Keys.chord(Keys.SHIFT, Keys.TAB))
+    // doesn't work anymore
+    private void pressKeyWithModifier(Keys keyModifier, Keys key) {
+        new Actions(getDriver()).keyDown(keyModifier).perform();
+        new Actions(getDriver()).sendKeys(key).perform();
+        new Actions(getDriver()).keyUp(keyModifier).perform();
+    }
 }
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridScrolledToBottomTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridScrolledToBottomTest.java
index e4fd2fc2dd..a37e266d30 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridScrolledToBottomTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridScrolledToBottomTest.java
@@ -70,28 +70,27 @@ public class GridScrolledToBottomTest extends MultiBrowserTest {
         Actions actions = new Actions(driver);
         actions.clickAndHold(splitter).moveByOffset(0, -rowHeight / 2).release()
                 .perform();
-        // the last row is now only half visible, and in DOM tree it's actually
-        // the first row now but positioned to the bottom
+        // the last row is now only half visible
 
         // can't query grid.getRow(99) now or it moves the row position,
         // have to use element query instead
         List<WebElement> rows = grid.findElement(By.className("v-grid-body"))
                 .findElements(By.className("v-grid-row"));
-        WebElement firstRow = rows.get(0);
         WebElement lastRow = rows.get(rows.size() - 1);
+        WebElement secondToLastRow = rows.get(rows.size() - 2);
 
         // ensure the scrolling didn't jump extra
         assertEquals("Person 99",
-                firstRow.findElement(By.className("v-grid-cell")).getText());
-        assertEquals("Person 98",
                 lastRow.findElement(By.className("v-grid-cell")).getText());
+        assertEquals("Person 98", secondToLastRow
+                .findElement(By.className("v-grid-cell")).getText());
 
         // re-calculate current end position
         gridBottomY = grid.getLocation().getY() + grid.getSize().getHeight();
         // ensure the correct final row really is only half visible at the
         // bottom
-        assertThat(gridBottomY, greaterThan(firstRow.getLocation().getY()));
-        assertThat(firstRow.getLocation().getY() + rowHeight,
+        assertThat(gridBottomY, greaterThan(lastRow.getLocation().getY()));
+        assertThat(lastRow.getLocation().getY() + rowHeight,
                 greaterThan(gridBottomY));
     }
 }
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/GridColumnResizeModeTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/GridColumnResizeModeTest.java
index df51706ddb..812052a13c 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/GridColumnResizeModeTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/GridColumnResizeModeTest.java
@@ -103,12 +103,12 @@ public class GridColumnResizeModeTest extends GridBasicsTest {
 
         // ANIMATED resize mode
         drag(handle, 100);
-        assertTrue(
+        assertTrue("Expected width: " + cell.getSize().getWidth(),
                 getLogRow(0).contains("Column resized: caption=Column 1, width="
                         + cell.getSize().getWidth()));
 
         drag(handle, -100);
-        assertTrue(
+        assertTrue("Expected width: " + cell.getSize().getWidth(),
                 getLogRow(0).contains("Column resized: caption=Column 1, width="
                         + cell.getSize().getWidth()));
 
@@ -117,12 +117,12 @@ public class GridColumnResizeModeTest extends GridBasicsTest {
         sleep(250);
 
         drag(handle, 100);
-        assertTrue(
+        assertTrue("Expected width: " + cell.getSize().getWidth(),
                 getLogRow(0).contains("Column resized: caption=Column 1, width="
                         + cell.getSize().getWidth()));
 
         drag(handle, -100);
-        assertTrue(
+        assertTrue("Expected width: " + cell.getSize().getWidth(),
                 getLogRow(0).contains("Column resized: caption=Column 1, width="
                         + cell.getSize().getWidth()));
     }
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorBasicsTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorBasicsTest.java
index 3e8e7d65a3..87fbb358b9 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorBasicsTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorBasicsTest.java
@@ -3,12 +3,16 @@ package com.vaadin.tests.components.grid.basicfeatures.escalator;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
 
 import org.junit.Before;
 import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
 
+import com.vaadin.testbench.TestBenchElement;
 import com.vaadin.testbench.elements.NotificationElement;
 import com.vaadin.tests.components.grid.basicfeatures.EscalatorBasicClientFeaturesTest;
 
@@ -49,13 +53,29 @@ public class EscalatorBasicsTest extends EscalatorBasicClientFeaturesTest {
         scrollHorizontallyTo(50);
 
         selectMenuPath(GENERAL, DETACH_ESCALATOR);
+        waitForElementNotPresent(By.className("v-escalator"));
         selectMenuPath(GENERAL, ATTACH_ESCALATOR);
+        waitForElementPresent(By.className("v-escalator"));
 
         assertEquals("Vertical scroll position", 50, getScrollTop());
         assertEquals("Horizontal scroll position", 50, getScrollLeft());
 
+        TestBenchElement bodyCell = getBodyCell(2, 0);
+        WebElement viewport = findElement(
+                By.className("v-escalator-tablewrapper"));
+        WebElement header = findElement(By.className("v-escalator-header"));
+        // ensure this is the first (partially) visible cell
+        assertTrue(
+                viewport.getLocation().getX() > bodyCell.getLocation().getX());
+        assertTrue(viewport.getLocation().getX() < bodyCell.getLocation().getX()
+                + bodyCell.getSize().getWidth());
+        assertTrue(header.getLocation().getY()
+                + header.getSize().getHeight() > bodyCell.getLocation().getY());
+        assertTrue(header.getLocation().getY()
+                + header.getSize().getHeight() < bodyCell.getLocation().getY()
+                        + bodyCell.getSize().getHeight());
         assertEquals("First cell of first visible row", "Row 2: 0,2",
-                getBodyCell(0, 0).getText());
+                bodyCell.getText());
     }
 
     private void assertEscalatorIsRemovedCorrectly() {
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java
index 7924b67503..45e08f9683 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/basicfeatures/escalator/EscalatorSpacerTest.java
@@ -275,12 +275,17 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest {
         selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX);
 
         /*
-         * we check for row -3 instead of -1, because escalator has two rows
+         * we check for row -2 instead of -1, because escalator has one row
          * buffered underneath the footer
          */
         selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, SCROLL_TO, ROW_75);
         Thread.sleep(500);
-        assertEquals("Row 75: 0,75", getBodyCell(-3, 0).getText());
+        TestBenchElement cell75 = getBodyCell(-2, 0);
+        assertEquals("Row 75: 0,75", cell75.getText());
+        // confirm the scroll position
+        WebElement footer = findElement(By.className("v-escalator-footer"));
+        assertEquals(footer.getLocation().y,
+                cell75.getLocation().y + cell75.getSize().height);
 
         selectMenuPath(COLUMNS_AND_ROWS, BODY_ROWS, SCROLL_TO, ROW_25);
         Thread.sleep(500);
@@ -406,17 +411,52 @@ public class EscalatorSpacerTest extends EscalatorBasicClientFeaturesTest {
     }
 
     @Test
-    public void spacersAreInCorrectDomPositionAfterScroll() {
+    public void spacersAreInCorrectDomPositionAfterScroll()
+            throws InterruptedException {
         selectMenuPath(FEATURES, SPACERS, ROW_1, SET_100PX);
 
-        scrollVerticallyTo(32); // roughly one row's worth
+        scrollVerticallyTo(40); // roughly two rows' worth
 
+        // both rows should still be within DOM after this little scrolling, so
+        // the spacer should be the third element within the body (index: 2)
         WebElement tbody = getEscalator().findElement(By.tagName("tbody"));
-        WebElement spacer = getChild(tbody, 1);
+        WebElement spacer = getChild(tbody, 2);
         String cssClass = spacer.getAttribute("class");
         assertTrue(
-                "element index 1 was not a spacer (class=\"" + cssClass + "\")",
+                "element index 2 was not a spacer (class=\"" + cssClass + "\")",
                 cssClass.contains("-spacer"));
+
+        // Scroll to last DOM row (Row 20). The exact position varies a bit
+        // depending on the browser.
+        int scrollTo = 172;
+        while (scrollTo < 176) {
+            scrollVerticallyTo(scrollTo);
+            Thread.sleep(500);
+
+            // if spacer is still the third (index: 2) body element, i.e. not
+            // enough scrolling to re-purpose any rows, scroll a bit further
+            spacer = getChild(tbody, 2);
+            cssClass = spacer.getAttribute("class");
+            if (cssClass.contains("-spacer")) {
+                ++scrollTo;
+            } else {
+                break;
+            }
+        }
+        if (getChild(tbody, 20).getText().startsWith("Row 22:")) {
+            // Some browsers scroll too much, spacer should be out of visual
+            // range
+            assertNull("Element found where there should be none",
+                    getChild(tbody, 21));
+        } else {
+            // second row should still be within DOM but the first row out of
+            // it, so the spacer should be the second element within the body
+            // (index: 1)
+            spacer = getChild(tbody, 1);
+            cssClass = spacer.getAttribute("class");
+            assertTrue("element index 1 was not a spacer (class=\"" + cssClass
+                    + "\")", cssClass.contains("-spacer"));
+        }
     }
 
     @Test
diff --git a/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridBigDetailsManagerTest.java b/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridBigDetailsManagerTest.java
index 76c700ec8f..b145dc99bc 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridBigDetailsManagerTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridBigDetailsManagerTest.java
@@ -1,12 +1,16 @@
 package com.vaadin.tests.components.treegrid;
 
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.not;
 import static org.hamcrest.number.IsCloseTo.closeTo;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 
 import java.util.List;
 
+import org.junit.Before;
 import org.junit.Test;
 import org.openqa.selenium.StaleElementReferenceException;
 import org.openqa.selenium.WebDriver;
@@ -15,6 +19,7 @@ import org.openqa.selenium.support.ui.ExpectedCondition;
 import org.openqa.selenium.support.ui.ExpectedConditions;
 
 import com.vaadin.testbench.By;
+import com.vaadin.testbench.TestBenchElement;
 import com.vaadin.testbench.elements.ButtonElement;
 import com.vaadin.testbench.elements.TreeGridElement;
 import com.vaadin.tests.tb3.MultiBrowserTest;
@@ -33,13 +38,18 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
     private static final String HIDE_DETAILS = "hideDetails";
     private static final String ADD_GRID = "addGrid";
     private static final String SCROLL_TO_55 = "scrollTo55";
+    private static final String SCROLL_TO_3055 = "scrollTo3055";
+    private static final String SCROLL_TO_END = "scrollToEnd";
+    private static final String SCROLL_TO_START = "scrollToStart";
+    private static final String TOGGLE_15 = "toggle15";
+    private static final String TOGGLE_3000 = "toggle3000";
 
     private TreeGridElement treeGrid;
     private int expectedSpacerHeight = 0;
     private int expectedRowHeight = 0;
 
     private ExpectedCondition<Boolean> expectedConditionDetails(final int root,
-            final int branch, final int leaf) {
+            final Integer branch, final Integer leaf) {
         return new ExpectedCondition<Boolean>() {
             @Override
             public Boolean apply(WebDriver arg0) {
@@ -49,9 +59,14 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
             @Override
             public String toString() {
                 // waiting for...
+                if (leaf != null) {
+                    return String.format(
+                            "Leaf %s/%s/%s details row contents to be found",
+                            root, branch, leaf);
+                }
                 return String.format(
-                        "Leaf %s/%s/%s details row contents to be found", root,
-                        branch, leaf);
+                        "Branch %s/%s details row contents to be found", root,
+                        branch);
             }
         };
     }
@@ -87,6 +102,11 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
         return null;
     }
 
+    private WebElement getRow(int index) {
+        return treeGrid.getBody().findElements(By.className("v-treegrid-row"))
+                .get(index);
+    }
+
     private void ensureExpectedSpacerHeightSet() {
         if (expectedSpacerHeight == 0) {
             expectedSpacerHeight = treeGrid
@@ -94,9 +114,6 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
                     .getHeight();
             assertThat((double) expectedSpacerHeight, closeTo(27d, 2d));
         }
-        if (expectedRowHeight == 0) {
-            expectedRowHeight = treeGrid.getRow(0).getSize().getHeight();
-        }
     }
 
     private void assertSpacerCount(int expectedSpacerCount) {
@@ -129,10 +146,6 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
                 previousSpacer = spacer;
                 continue;
             }
-            if (spacer.getLocation().y == 0) {
-                // FIXME: find out why there are cases like this out of order
-                continue;
-            }
             // -1 should be enough, but increased tolerance to -3 for FireFox
             // and IE11 since a few pixels' discrepancy isn't relevant for this
             // fix
@@ -148,16 +161,24 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
                 treeGrid.findElements(By.className(CLASSNAME_ERROR)).size());
     }
 
-    @Test
-    public void expandAllOpenAllInitialDetails_toggleOneTwice_hideAll() {
-        openTestURL();
-        $(ButtonElement.class).id(EXPAND_ALL).click();
-        $(ButtonElement.class).id(SHOW_DETAILS).click();
+    private void addGrid() {
         $(ButtonElement.class).id(ADD_GRID).click();
-
         waitForElementPresent(By.className(CLASSNAME_TREEGRID));
 
         treeGrid = $(TreeGridElement.class).first();
+        expectedRowHeight = treeGrid.getRow(0).getSize().getHeight();
+    }
+
+    @Before
+    public void before() {
+        openTestURL();
+    }
+
+    @Test
+    public void expandAllOpenAllInitialDetails_toggleOneTwice_hideAll() {
+        $(ButtonElement.class).id(EXPAND_ALL).click();
+        $(ButtonElement.class).id(SHOW_DETAILS).click();
+        addGrid();
 
         waitUntil(expectedConditionDetails(0, 0, 0));
         ensureExpectedSpacerHeightSet();
@@ -205,14 +226,9 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
 
     @Test
     public void expandAllOpenAllInitialDetails_toggleAll() {
-        openTestURL();
         $(ButtonElement.class).id(EXPAND_ALL).click();
         $(ButtonElement.class).id(SHOW_DETAILS).click();
-        $(ButtonElement.class).id(ADD_GRID).click();
-
-        waitForElementPresent(By.className(CLASSNAME_TREEGRID));
-
-        treeGrid = $(TreeGridElement.class).first();
+        addGrid();
 
         waitUntil(expectedConditionDetails(0, 0, 0));
         ensureExpectedSpacerHeightSet();
@@ -231,9 +247,9 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
         assertSpacerPositions();
 
         // FIXME: TreeGrid fails to update cache correctly when you expand all
-        // and after a long, long wait you end up with 3321 open details rows
-        // and row 63/8/0 in view instead of 95 and 0/0/0 as expected.
-        // WaitUntil timeouts by then.
+        // and triggers client-side exceptions for rows that fall outside of the
+        // cache because they try to extend the cache with a range that isn't
+        // connected to the cached range
         if (true) {// remove this block after fixed
             return;
         }
@@ -241,7 +257,7 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
         $(ButtonElement.class).id(EXPAND_ALL).click();
 
         // State should have returned to what it was before collapsing.
-        waitUntil(expectedConditionDetails(0, 0, 0));
+        waitUntil(expectedConditionDetails(0, 0, 0), 15);
         assertSpacerCount(spacerCount);
         assertSpacerHeights();
         assertSpacerPositions();
@@ -251,13 +267,8 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
 
     @Test
     public void expandAllOpenNoInitialDetails_showSeveral_toggleOneByOne() {
-        openTestURL();
         $(ButtonElement.class).id(EXPAND_ALL).click();
-        $(ButtonElement.class).id(ADD_GRID).click();
-
-        waitForElementPresent(By.className(CLASSNAME_TREEGRID));
-
-        treeGrid = $(TreeGridElement.class).first();
+        addGrid();
 
         // open details for several rows, leave one out from the hierarchy that
         // is to be collapsed
@@ -310,17 +321,29 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
     }
 
     @Test
+    public void expandAllOpenAllInitialDetails_hideOne() {
+        $(ButtonElement.class).id(EXPAND_ALL).click();
+        $(ButtonElement.class).id(SHOW_DETAILS).click();
+        addGrid();
+
+        // check the position of a row
+        int oldY = treeGrid.getCell(2, 0).getLocation().getY();
+
+        // hide the spacer from previous row
+        treeGrid.getCell(1, 0).click();
+
+        // ensure the investigated row moved
+        assertNotEquals(oldY, treeGrid.getCell(2, 0).getLocation().getY());
+    }
+
+    @Test
     public void expandAllOpenAllInitialDetailsScrolled_toggleOne_hideAll() {
-        openTestURL();
         $(ButtonElement.class).id(EXPAND_ALL).click();
         $(ButtonElement.class).id(SHOW_DETAILS).click();
-        $(ButtonElement.class).id(ADD_GRID).click();
+        addGrid();
 
-        waitForElementPresent(By.className(CLASSNAME_TREEGRID));
         $(ButtonElement.class).id(SCROLL_TO_55).click();
 
-        treeGrid = $(TreeGridElement.class).first();
-
         waitUntil(expectedConditionDetails(1, 2, 0));
         ensureExpectedSpacerHeightSet();
         int spacerCount = treeGrid.findElements(By.className(CLASSNAME_SPACER))
@@ -333,8 +356,7 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
         waitUntil(ExpectedConditions.not(expectedConditionDetails(1, 2, 0)));
         assertSpacerHeights();
         assertSpacerPositions();
-        // FIXME: gives 128, not 90 as expected
-        // assertSpacerCount(spacerCount);
+        assertSpacerCount(spacerCount);
 
         treeGrid.expandWithClick(50);
 
@@ -342,8 +364,7 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
         waitUntil(expectedConditionDetails(1, 2, 0));
         assertSpacerHeights();
         assertSpacerPositions();
-        // FIXME: gives 131, not 90 as expected
-        // assertSpacerCount(spacerCount);
+        assertSpacerCount(spacerCount);
 
         // test that repeating the toggle still doesn't change anything
 
@@ -352,16 +373,14 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
         waitUntil(ExpectedConditions.not(expectedConditionDetails(1, 2, 0)));
         assertSpacerHeights();
         assertSpacerPositions();
-        // FIXME: gives 128, not 90 as expected
-        // assertSpacerCount(spacerCount);
+        assertSpacerCount(spacerCount);
 
         treeGrid.expandWithClick(50);
 
         waitUntil(expectedConditionDetails(1, 2, 0));
         assertSpacerHeights();
         assertSpacerPositions();
-        // FIXME: gives 131, not 90 as expected
-        // assertSpacerCount(spacerCount);
+        assertSpacerCount(spacerCount);
 
         // test that hiding all still won't break things
 
@@ -373,17 +392,13 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
 
     @Test
     public void expandAllOpenAllInitialDetailsScrolled_toggleAll() {
-        openTestURL();
         $(ButtonElement.class).id(EXPAND_ALL).click();
         $(ButtonElement.class).id(SHOW_DETAILS).click();
-        $(ButtonElement.class).id(ADD_GRID).click();
+        addGrid();
 
-        waitForElementPresent(By.className(CLASSNAME_TREEGRID));
         $(ButtonElement.class).id(SCROLL_TO_55).click();
 
-        treeGrid = $(TreeGridElement.class).first();
-
-        waitUntil(expectedConditionDetails(1, 1, 0));
+        waitUntil(expectedConditionDetails(1, 3, 0));
         ensureExpectedSpacerHeightSet();
 
         int spacerCount = treeGrid.findElements(By.className(CLASSNAME_SPACER))
@@ -400,7 +415,8 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
         assertSpacerHeights();
         assertSpacerPositions();
 
-        // FIXME: collapsing too many rows after scrolling still causes a chaos
+        // FIXME: collapsing and expanding too many rows after scrolling still
+        // fails to reset to the same state
         if (true) { // remove this block after fixed
             return;
         }
@@ -408,7 +424,7 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
         $(ButtonElement.class).id(EXPAND_ALL).click();
 
         // State should have returned to what it was before collapsing.
-        waitUntil(expectedConditionDetails(1, 1, 0));
+        waitUntil(expectedConditionDetails(1, 3, 0));
         assertSpacerCount(spacerCount);
         assertSpacerHeights();
         assertSpacerPositions();
@@ -418,27 +434,24 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
 
     @Test
     public void expandAllOpenNoInitialDetailsScrolled_showSeveral_toggleOneByOne() {
-        openTestURL();
         $(ButtonElement.class).id(EXPAND_ALL).click();
-        $(ButtonElement.class).id(ADD_GRID).click();
+        addGrid();
 
-        waitForElementPresent(By.className(CLASSNAME_TREEGRID));
         $(ButtonElement.class).id(SCROLL_TO_55).click();
 
-        treeGrid = $(TreeGridElement.class).first();
         assertSpacerCount(0);
 
         // open details for several rows, leave one out from the hierarchy that
         // is to be collapsed
-        treeGrid.getCell(50, 0).click();
-        treeGrid.getCell(51, 0).click();
-        treeGrid.getCell(52, 0).click();
-        // no click for cell (53, 0)
-        treeGrid.getCell(54, 0).click();
-        treeGrid.getCell(55, 0).click();
-        treeGrid.getCell(56, 0).click();
-        treeGrid.getCell(57, 0).click();
-        treeGrid.getCell(58, 0).click();
+        treeGrid.getCell(50, 0).click(); // Branch 1/2
+        treeGrid.getCell(51, 0).click(); // Leaf 1/2/0
+        treeGrid.getCell(52, 0).click(); // Leaf 1/2/1
+        // no click for cell (53, 0) // Leaf 1/2/2
+        treeGrid.getCell(54, 0).click(); // Branch 1/3
+        treeGrid.getCell(55, 0).click(); // Leaf 1/3/0
+        treeGrid.getCell(56, 0).click(); // Leaf 1/3/1
+        treeGrid.getCell(57, 0).click(); // Leaf 1/3/2
+        treeGrid.getCell(58, 0).click(); // Branch 1/4
         int spacerCount = 8;
 
         waitUntil(expectedConditionDetails(1, 2, 0));
@@ -507,4 +520,351 @@ public class TreeGridBigDetailsManagerTest extends MultiBrowserTest {
         assertNoErrors();
     }
 
+    @Test
+    public void expandAllOpenAllInitialDetailsScrolled_hideOne() {
+        $(ButtonElement.class).id(EXPAND_ALL).click();
+        $(ButtonElement.class).id(SHOW_DETAILS).click();
+        addGrid();
+
+        $(ButtonElement.class).id(SCROLL_TO_55).click();
+
+        // check the position of a row
+        int oldY = treeGrid.getCell(52, 0).getLocation().getY();
+
+        // hide the spacer from previous row
+        treeGrid.getCell(51, 0).click();
+
+        // ensure the investigated row moved
+        assertNotEquals(oldY, treeGrid.getCell(52, 0).getLocation().getY());
+    }
+
+    @Test
+    public void expandAllOpenAllInitialDetailsScrolledFar_toggleOne_hideAll() {
+        $(ButtonElement.class).id(EXPAND_ALL).click();
+        $(ButtonElement.class).id(SHOW_DETAILS).click();
+        addGrid();
+
+        $(ButtonElement.class).id(SCROLL_TO_3055).click();
+
+        waitUntil(expectedConditionDetails(74, 4, 0));
+        ensureExpectedSpacerHeightSet();
+        int spacerCount = treeGrid.findElements(By.className(CLASSNAME_SPACER))
+                .size();
+        assertSpacerPositions();
+
+        treeGrid.collapseWithClick(3051);
+
+        // collapsing one shouldn't affect spacer count, just update the cache
+        waitUntil(ExpectedConditions.not(expectedConditionDetails(1, 2, 0)));
+        assertSpacerHeights();
+        assertSpacerPositions();
+        assertSpacerCount(spacerCount);
+
+        treeGrid.expandWithClick(3051);
+
+        // expanding back shouldn't affect spacer count, just update the cache
+        waitUntil(expectedConditionDetails(74, 4, 0));
+        assertSpacerHeights();
+        assertSpacerPositions();
+        assertSpacerCount(spacerCount);
+
+        // test that repeating the toggle still doesn't change anything
+
+        treeGrid.collapseWithClick(3051);
+
+        waitUntil(ExpectedConditions.not(expectedConditionDetails(74, 4, 0)));
+        assertSpacerHeights();
+        assertSpacerPositions();
+        assertSpacerCount(spacerCount);
+
+        treeGrid.expandWithClick(3051);
+
+        waitUntil(expectedConditionDetails(74, 4, 0));
+        assertSpacerHeights();
+        assertSpacerPositions();
+        assertSpacerCount(spacerCount);
+
+        // test that hiding all still won't break things
+
+        $(ButtonElement.class).id(HIDE_DETAILS).click();
+        waitForElementNotPresent(By.className(CLASSNAME_SPACER));
+
+        assertNoErrors();
+    }
+
+    @Test
+    public void expandAllOpenAllInitialDetailsScrolledFar_toggleAll() {
+        $(ButtonElement.class).id(EXPAND_ALL).click();
+        $(ButtonElement.class).id(SHOW_DETAILS).click();
+        addGrid();
+
+        $(ButtonElement.class).id(SCROLL_TO_3055).click();
+
+        waitUntil(expectedConditionDetails(74, 4, 0));
+        ensureExpectedSpacerHeightSet();
+
+        int spacerCount = treeGrid.findElements(By.className(CLASSNAME_SPACER))
+                .size();
+        assertSpacerPositions();
+
+        $(ButtonElement.class).id(COLLAPSE_ALL).click();
+
+        waitForElementNotPresent(By.className(CLASSNAME_LEAF));
+
+        // There should still be a full cache's worth of details rows open,
+        // just not the same rows than before collapsing all.
+        assertSpacerCount(spacerCount);
+        assertSpacerHeights();
+        assertSpacerPositions();
+
+        // FIXME: collapsing and expanding too many rows after scrolling still
+        // fails to reset to the same state
+        if (true) { // remove this block after fixed
+            return;
+        }
+
+        $(ButtonElement.class).id(EXPAND_ALL).click();
+
+        // State should have returned to what it was before collapsing.
+        waitUntil(expectedConditionDetails(74, 4, 0));
+        assertSpacerCount(spacerCount);
+        assertSpacerHeights();
+        assertSpacerPositions();
+
+        assertNoErrors();
+    }
+
+    @Test
+    public void expandAllOpenNoInitialDetailsScrolledFar_showSeveral_toggleOneByOne() {
+        $(ButtonElement.class).id(EXPAND_ALL).click();
+        addGrid();
+
+        $(ButtonElement.class).id(SCROLL_TO_3055).click();
+
+        assertSpacerCount(0);
+
+        // open details for several rows, leave one out from the hierarchy that
+        // is to be collapsed
+        treeGrid.getCell(3051, 0).click(); // Branch 74/4
+        treeGrid.getCell(3052, 0).click(); // Leaf 74/4/0
+        treeGrid.getCell(3053, 0).click(); // Leaf 74/4/1
+        // no click for cell (3054, 0) // Leaf 74/4/2
+        treeGrid.getCell(3055, 0).click(); // Branch 74/5
+        treeGrid.getCell(3056, 0).click(); // Leaf 74/5/0
+        treeGrid.getCell(3057, 0).click(); // Leaf 74/5/1
+        treeGrid.getCell(3058, 0).click(); // Leaf 74/5/2
+        treeGrid.getCell(3059, 0).click(); // Branch 74/6
+        int spacerCount = 8;
+
+        waitUntil(expectedConditionDetails(74, 4, 0));
+        assertSpacerCount(spacerCount);
+        ensureExpectedSpacerHeightSet();
+        assertSpacerPositions();
+
+        // toggle the branch with partially open details rows
+        treeGrid.collapseWithClick(3051);
+
+        waitUntil(ExpectedConditions.not(expectedConditionDetails(74, 4, 0)));
+        assertSpacerCount(spacerCount - 2);
+        assertSpacerHeights();
+        assertSpacerPositions();
+
+        treeGrid.expandWithClick(3051);
+
+        waitUntil(expectedConditionDetails(74, 4, 0));
+        assertSpacerCount(spacerCount);
+        assertSpacerHeights();
+        assertSpacerPositions();
+
+        // toggle the branch with fully open details rows
+        treeGrid.collapseWithClick(3055);
+
+        waitUntil(ExpectedConditions.not(expectedConditionDetails(74, 5, 0)));
+        assertSpacerCount(spacerCount - 3);
+        assertSpacerHeights();
+        assertSpacerPositions();
+
+        treeGrid.expandWithClick(3055);
+
+        waitUntil(expectedConditionDetails(74, 5, 0));
+        assertSpacerCount(spacerCount);
+        assertSpacerHeights();
+        assertSpacerPositions();
+
+        // repeat both toggles to ensure still no errors
+        treeGrid.collapseWithClick(3051);
+
+        waitUntil(ExpectedConditions.not(expectedConditionDetails(74, 4, 0)));
+        assertSpacerCount(spacerCount - 2);
+        assertSpacerHeights();
+        assertSpacerPositions();
+
+        treeGrid.expandWithClick(3051);
+
+        waitUntil(expectedConditionDetails(74, 4, 0));
+        assertSpacerCount(spacerCount);
+        assertSpacerHeights();
+        assertSpacerPositions();
+        treeGrid.collapseWithClick(3055);
+
+        waitUntil(ExpectedConditions.not(expectedConditionDetails(74, 5, 0)));
+        assertSpacerCount(spacerCount - 3);
+        assertSpacerHeights();
+        assertSpacerPositions();
+
+        treeGrid.expandWithClick(3055);
+
+        waitUntil(expectedConditionDetails(74, 5, 0));
+        assertSpacerCount(spacerCount);
+        assertSpacerHeights();
+        assertSpacerPositions();
+
+        assertNoErrors();
+    }
+
+    @Test
+    public void expandAllOpenAllInitialDetailsScrolledFar_hideOne() {
+        $(ButtonElement.class).id(EXPAND_ALL).click();
+        $(ButtonElement.class).id(SHOW_DETAILS).click();
+        addGrid();
+
+        $(ButtonElement.class).id(SCROLL_TO_3055).click();
+
+        // check the position of a row
+        int oldY = treeGrid.getCell(3052, 0).getLocation().getY();
+
+        // hide the spacer from previous row
+        treeGrid.getCell(3051, 0).click();
+
+        // ensure the investigated row moved
+        assertNotEquals(oldY, treeGrid.getCell(52, 0).getLocation().getY());
+    }
+
+    @Test
+    public void expandAllOpenAllInitialDetails_checkScrollPositions() {
+        $(ButtonElement.class).id(EXPAND_ALL).click();
+        $(ButtonElement.class).id(SHOW_DETAILS).click();
+        addGrid();
+
+        TestBenchElement tableWrapper = treeGrid.getTableWrapper();
+
+        $(ButtonElement.class).id(SCROLL_TO_55).click();
+        waitUntil(expectedConditionDetails(1, 3, 0));
+
+        WebElement detailsRow = getSpacer(1, 3, 0);
+        assertNotNull("Spacer for row 55 not found", detailsRow);
+
+        int wrapperY = tableWrapper.getLocation().getY();
+        int wrapperHeight = tableWrapper.getSize().getHeight();
+
+        int detailsY = detailsRow.getLocation().getY();
+        int detailsHeight = detailsRow.getSize().getHeight();
+
+        assertThat("Scroll to 55 didn't scroll as expected",
+                (double) detailsY + detailsHeight,
+                closeTo(wrapperY + wrapperHeight, 1d));
+
+        $(ButtonElement.class).id(SCROLL_TO_3055).click();
+        waitUntil(expectedConditionDetails(74, 5, null));
+
+        detailsRow = getSpacer(74, 5, null);
+        assertNotNull("Spacer for row 3055 not found", detailsRow);
+
+        detailsY = detailsRow.getLocation().getY();
+        detailsHeight = detailsRow.getSize().getHeight();
+
+        assertThat("Scroll to 3055 didn't scroll as expected",
+                (double) detailsY + detailsHeight,
+                closeTo(wrapperY + wrapperHeight, 1d));
+
+        $(ButtonElement.class).id(SCROLL_TO_END).click();
+        waitUntil(expectedConditionDetails(99, 9, 2));
+
+        detailsRow = getSpacer(99, 9, 2);
+        assertNotNull("Spacer for last row not found", detailsRow);
+
+        detailsY = detailsRow.getLocation().getY();
+        detailsHeight = detailsRow.getSize().getHeight();
+
+        // the layout jumps sometimes, check again
+        wrapperY = tableWrapper.getLocation().getY();
+        wrapperHeight = tableWrapper.getSize().getHeight();
+
+        assertThat("Scroll to end didn't scroll as expected",
+                (double) detailsY + detailsHeight,
+                closeTo(wrapperY + wrapperHeight, 1d));
+
+        $(ButtonElement.class).id(SCROLL_TO_START).click();
+        waitUntil(expectedConditionDetails(0, 0, 0));
+
+        WebElement firstRow = getRow(0);
+        TestBenchElement header = treeGrid.getHeader();
+
+        assertThat("Scroll to start didn't scroll as expected",
+                (double) firstRow.getLocation().getY(),
+                closeTo(wrapperY + header.getSize().getHeight(), 1d));
+    }
+
+    @Test
+    public void expandAllOpenNoInitialDetails_testToggleScrolling() {
+        $(ButtonElement.class).id(EXPAND_ALL).click();
+        addGrid();
+
+        TestBenchElement tableWrapper = treeGrid.getTableWrapper();
+        int wrapperY = tableWrapper.getLocation().getY();
+
+        WebElement firstRow = getRow(0);
+        int firstRowY = firstRow.getLocation().getY();
+
+        TestBenchElement header = treeGrid.getHeader();
+        int headerHeight = header.getSize().getHeight();
+
+        assertThat("Unexpected initial scroll position", (double) firstRowY,
+                closeTo(wrapperY + headerHeight, 1d));
+
+        $(ButtonElement.class).id(TOGGLE_15).click();
+
+        firstRowY = firstRow.getLocation().getY();
+
+        assertThat(
+                "Toggling row 15's details open should have caused scrolling",
+                (double) firstRowY, not(closeTo(wrapperY + headerHeight, 1d)));
+
+        $(ButtonElement.class).id(SCROLL_TO_START).click();
+
+        firstRowY = firstRow.getLocation().getY();
+
+        assertThat("Scrolling to start failed", (double) firstRowY,
+                closeTo(wrapperY + headerHeight, 1d));
+
+        $(ButtonElement.class).id(TOGGLE_15).click();
+
+        firstRowY = firstRow.getLocation().getY();
+
+        assertThat(
+                "Toggling row 15's details closed should not have caused scrolling",
+                (double) firstRowY, closeTo(wrapperY + headerHeight, 1d));
+
+        $(ButtonElement.class).id(TOGGLE_3000).click();
+
+        firstRowY = firstRow.getLocation().getY();
+
+        assertThat(
+                "Toggling row 3000's details open should not have caused scrolling",
+                (double) firstRowY, closeTo(wrapperY + headerHeight, 1d));
+
+        $(ButtonElement.class).id(SCROLL_TO_55).click();
+
+        WebElement row = getRow(0);
+        assertNotEquals("First row should be out of visual range", firstRowY,
+                row);
+
+        $(ButtonElement.class).id(TOGGLE_15).click();
+
+        assertEquals(
+                "Toggling row 15's details open should not have caused scrolling "
+                        + "when row 15 is outside of visual range",
+                row, getRow(0));
+    }
+
 }
diff --git a/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridDetailsManagerTest.java b/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridDetailsManagerTest.java
index e5301a4731..9ca8b2a974 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridDetailsManagerTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/treegrid/TreeGridDetailsManagerTest.java
@@ -3,6 +3,7 @@ package com.vaadin.tests.components.treegrid;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.number.IsCloseTo.closeTo;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 
@@ -292,4 +293,25 @@ public class TreeGridDetailsManagerTest extends MultiBrowserTest {
         assertNoErrors();
     }
 
+    @Test
+    public void expandAllOpenAllInitialDetails_hideOne() {
+        openTestURL();
+        $(ButtonElement.class).id(EXPAND_ALL).click();
+        $(ButtonElement.class).id(SHOW_DETAILS).click();
+        $(ButtonElement.class).id(ADD_GRID).click();
+
+        waitForElementPresent(By.className(CLASSNAME_TREEGRID));
+
+        treeGrid = $(TreeGridElement.class).first();
+
+        // check the position of a row
+        int oldY = treeGrid.getCell(2, 0).getLocation().getY();
+
+        // hide the spacer from previous row
+        treeGrid.getCell(1, 0).click();
+
+        // ensure the investigated row moved
+        assertNotEquals(oldY, treeGrid.getCell(2, 0).getLocation().getY());
+    }
+
 }