From: Denis Anisimov <denis@vaadin.com>
Date: Mon, 7 Nov 2016 07:41:01 +0000 (+0200)
Subject: Prevent adding several scrollbar handlers (#19189).
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=176e8d03897a2b806d407d81c56d689f81d197c1;p=vaadin-framework.git

Prevent adding several scrollbar handlers (#19189).

Change-Id: Ib0cc6c6835aab6d263f153362a328bcf2be7bc5c
---

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 2b615ebdb7..ed8d3cd57b 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
@@ -436,6 +436,9 @@ public abstract class ScrollbarBundle implements DeferredWorker {
         boolean offsetSizeBecomesGreaterThanScrollSize = showsScrollHandle()
                 && newOffsetSizeIsGreaterThanScrollSize;
         if (offsetSizeBecomesGreaterThanScrollSize && getScrollPos() != 0) {
+            if (offsetSizeTemporaryScrollHandler != null) {
+                offsetSizeTemporaryScrollHandler.removeHandler();
+            }
             // must be a field because Java insists.
             offsetSizeTemporaryScrollHandler = addScrollHandler(
                     new ScrollHandler() {
@@ -661,6 +664,9 @@ public abstract class ScrollbarBundle implements DeferredWorker {
              */
             boolean delayedSizeSet = !BrowserInfo.get().isFirefox();
             if (delayedSizeSet) {
+                if (scrollSizeTemporaryScrollHandler != null) {
+                    scrollSizeTemporaryScrollHandler.removeHandler();
+                }
                 scrollSizeTemporaryScrollHandler = addScrollHandler(
                         new ScrollHandler() {
                             @Override
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/HorizontalScrollAfterResize.java b/uitest/src/main/java/com/vaadin/tests/components/grid/HorizontalScrollAfterResize.java
new file mode 100644
index 0000000000..8e6afa5441
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/HorizontalScrollAfterResize.java
@@ -0,0 +1,60 @@
+/*
+ * Copyright 2000-2016 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.grid;
+
+import java.util.Arrays;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Grid;
+
+/**
+ * @author Vaadin Ltd
+ *
+ */
+public class HorizontalScrollAfterResize extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        final Grid grid = new Grid();
+        grid.setWidth("100%");
+        grid.setHeight("350px");
+        grid.setCaption("My Grid");
+
+        for (int i = 0; i < 10; i++) {
+            char ch = (char) ('a' + i);
+            grid.addColumn(ch, String.class);
+        }
+        for (int i = 0; i < 100; i++) {
+            String[] row = new String[10];
+            Arrays.fill(row, "test");
+            grid.addRow(row);
+        }
+
+        addComponents(grid);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Don't add more than one scroll handler";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 19189; // also 20254, 19622
+    }
+
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/HorizontalScrollAfterResizeTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/HorizontalScrollAfterResizeTest.java
new file mode 100644
index 0000000000..e1f85fa074
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/HorizontalScrollAfterResizeTest.java
@@ -0,0 +1,68 @@
+/*
+ * Copyright 2000-2016 Vaadin Ltd.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.grid;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.Dimension;
+import org.openqa.selenium.Point;
+
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
+
+/**
+ * @author Vaadin Ltd
+ *
+ */
+public class HorizontalScrollAfterResizeTest extends GridBasicFeaturesTest {
+
+    /**
+     * The behavior without the fix differs across different browsers but
+     * scenario should work everywhere.
+     */
+    @Test
+    public void scrollAfterResize() {
+        getDriver().manage().window().setSize(new Dimension(600, 400));
+        openTestURL();
+        getDriver().manage().window().setSize(new Dimension(200, 400));
+
+        // First scroll to the right
+        scrollGridHorizontallyTo(600);
+        Point locationAfterFirstScroll = $(GridElement.class).first()
+                .getCell(0, 9).getLocation();
+
+        // resize back
+        getDriver().manage().window().setSize(new Dimension(600, 400));
+        // shrink again
+        getDriver().manage().window().setSize(new Dimension(200, 400));
+
+        // second scroll to the right
+        scrollGridHorizontallyTo(600);
+
+        Point lolocationAfterSecondScrollcation = $(GridElement.class).first()
+                .getCell(0, 9).getLocation();
+
+        // With the bug scrolling doesn't happen. Location should be the same as
+        // first time
+        Assert.assertEquals(locationAfterFirstScroll,
+                lolocationAfterSecondScrollcation);
+    }
+
+    @Override
+    protected Class<?> getUIClass() {
+        return HorizontalScrollAfterResize.class;
+    }
+}