Ensure Firefox always updates the grid scrollbar (#19802)

When the scrollbar is scrolled a bit down and the number of items
in the grid is reduced so that no scrollbar is needed anymore, it seems
that Firefox refuses to send a scroll event even though the scroll position
is updated before the scrollbar is hidden.

Change-Id: I626536a9efd036bc826b1e6be3363332a56774f6

3 files changed, 144 insertions, 8 deletions

diff --git a/client/src/main/java/com/vaadin/client/widget/escalator/ScrollbarBundle.java b/client/src/main/java/com/vaadin/client/widget/escalator/ScrollbarBundle.java
index 151deb87d1..367537d20d 100644
--- a/client/src/main/java/com/vaadin/client/widget/escalator/ScrollbarBundle.java
+++ b/client/src/main/java/com/vaadin/client/widget/escalator/ScrollbarBundle.java
@@ -31,6 +31,7 @@ import com.google.gwt.user.client.DOM;
 import com.google.gwt.user.client.Event;
 import com.google.gwt.user.client.EventListener;
 import com.google.gwt.user.client.Timer;
+import com.vaadin.client.BrowserInfo;
 import com.vaadin.client.DeferredWorker;
 import com.vaadin.client.WidgetUtil;
 import com.vaadin.client.widget.grid.events.ScrollEvent;
@@ -447,13 +448,26 @@ public abstract class ScrollbarBundle implements DeferredWorker {
      * set either <code>overflow-x</code> or <code>overflow-y</code> to "
      * <code>scroll</code>" in the scrollbar's direction.
      * <p>
-     * This is an IE8 workaround, since it doesn't always show scrollbars with
-     * <code>overflow: auto</code> enabled.
+     * This method is an IE8 workaround, since it doesn't always show scrollbars
+     * with <code>overflow: auto</code> enabled.
+     * <p>
+     * Firefox on the other hand loses pending scroll events when the scrollbar
+     * is hidden, so the event must be fired manually.
+     * <p>
+     * When IE8 support is dropped, this should really be simplified.
      */
     protected void forceScrollbar(boolean enable) {
         if (enable) {
             root.getStyle().clearDisplay();
         } else {
+            if (BrowserInfo.get().isFirefox()) {
+                /*
+                 * This is related to the Firefox workaround in setScrollSize
+                 * for setScrollPos(0)
+                 */
+                scrollEventFirer.scheduleEvent();
+            }
+
             root.getStyle().setDisplay(Display.NONE);
         }
         internalForceScrollbar(enable);
@@ -601,20 +615,38 @@ public abstract class ScrollbarBundle implements DeferredWorker {
          * This needs to be made step-by-step because IE8 flat-out refuses to
          * fire a scroll event when the scroll size becomes smaller than the
          * offset size. All other browser need to suffer alongside.
+         *
+         * This really should be changed to not use any temporary scroll
+         * handlers at all once IE8 support is dropped, like now done only for
+         * Firefox.
          */
 
         boolean newScrollSizeIsSmallerThanOffsetSize = px <= getOffsetSize();
         boolean scrollSizeBecomesSmallerThanOffsetSize = showsScrollHandle()
                 && newScrollSizeIsSmallerThanOffsetSize;
         if (scrollSizeBecomesSmallerThanOffsetSize && getScrollPos() != 0) {
+            /*
+             * For whatever reason, Firefox loses the scroll event in this case
+             * and the onscroll handler is never called (happens when reducing
+             * size from 1000 items to 1 while being scrolled a bit down, see
+             * #19802). Based on the comment above, only IE8 should really use
+             * 'delayedSizeSet'
+             */
+            boolean delayedSizeSet = !BrowserInfo.get().isFirefox();
             // must be a field because Java insists.
-            scrollSizeTemporaryScrollHandler = addScrollHandler(new ScrollHandler() {
-                @Override
-                public void onScroll(ScrollEvent event) {
-                    setScrollSizeNow(px);
-                }
-            });
+            if (delayedSizeSet) {
+                scrollSizeTemporaryScrollHandler = addScrollHandler(
+                        new ScrollHandler() {
+                            @Override
+                            public void onScroll(ScrollEvent event) {
+                                setScrollSizeNow(px);
+                            }
+                        });
+            }
             setScrollPos(0);
+            if (!delayedSizeSet) {
+                setScrollSizeNow(px);
+            }
         } else {
             setScrollSizeNow(px);
         }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridApplyFilterWhenScrolledDown.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridApplyFilterWhenScrolledDown.java
new file mode 100644
index 0000000000..93413c102a
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridApplyFilterWhenScrolledDown.java
@@ -0,0 +1,64 @@
+package com.vaadin.tests.components.grid;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.data.Container.Filterable;
+import com.vaadin.data.Item;
+import com.vaadin.data.util.filter.SimpleStringFilter;
+import com.vaadin.event.FieldEvents.TextChangeEvent;
+import com.vaadin.event.FieldEvents.TextChangeListener;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.Grid.HeaderRow;
+import com.vaadin.ui.TextField;
+import com.vaadin.ui.UI;
+import com.vaadin.ui.themes.ValoTheme;
+
+@Theme("valo")
+public class GridApplyFilterWhenScrolledDown extends UI {
+
+    Grid grid = new Grid();
+
+    @Override
+    protected void init(VaadinRequest vaadinRequest) {
+
+        grid.addColumn("Name", String.class);
+
+        HeaderRow appendHeaderRow = grid.appendHeaderRow();
+        TextField filter = getColumnFilter("Name");
+        appendHeaderRow.getCell("Name").setComponent(filter);
+
+        for (int i = 0; i < 1000; i++) {
+            Item addItem = grid.getContainerDataSource().addItem(i);
+            addItem.getItemProperty("Name").setValue("Name " + i);
+
+        }
+
+        Item addItem = grid.getContainerDataSource().addItem(1000);
+        addItem.getItemProperty("Name").setValue("Test");
+
+        grid.scrollToStart();
+        setContent(grid);
+    }
+
+    private TextField getColumnFilter(final Object columnId) {
+        TextField filter = new TextField();
+        filter.setWidth("100%");
+        filter.addStyleName(ValoTheme.TEXTFIELD_TINY);
+        filter.addTextChangeListener(new TextChangeListener() {
+            SimpleStringFilter filter = null;
+
+            @Override
+            public void textChange(TextChangeEvent event) {
+                Filterable f = (Filterable) grid.getContainerDataSource();
+                if (filter != null) {
+                    f.removeContainerFilter(filter);
+                }
+                filter = new SimpleStringFilter(columnId, event.getText(), true,
+                        true);
+                f.addContainerFilter(filter);
+            }
+        });
+        return filter;
+    }
+
+}
\ No newline at end of file
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridApplyFilterWhenScrolledDownTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridApplyFilterWhenScrolledDownTest.java
new file mode 100644
index 0000000000..279cf8ef95
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridApplyFilterWhenScrolledDownTest.java
@@ -0,0 +1,40 @@
+package com.vaadin.tests.components.grid;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.support.ui.ExpectedCondition;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.TestBenchElement;
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.testbench.elements.TextFieldElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class GridApplyFilterWhenScrolledDownTest extends MultiBrowserTest {
+
+    @Test
+    public void scrolledCorrectly() throws InterruptedException {
+        openTestURL();
+        final GridElement grid = $(GridElement.class).first();
+        grid.scrollToRow(50);
+        $(TextFieldElement.class).first().setValue("Test");
+        final TestBenchElement gridBody = grid.getBody();
+        // Can't use element API because it scrolls
+        waitUntil(new ExpectedCondition<Boolean>() {
+
+            @Override
+            public Boolean apply(WebDriver input) {
+                return gridBody.findElements(By.className("v-grid-row"))
+                        .size() == 1;
+            }
+        });
+        WebElement cell = gridBody.findElements(By.className("v-grid-cell"))
+                .get(0);
+        Assert.assertEquals("Test", cell.getText());
+
+        Assert.assertTrue(
+                grid.getVerticalScroller().getSize().getHeight() < 100);
+    }
+}