Fix VScrollTable to clear reported ranges (#13353)

Change-Id: Ieb0e2dce37ae1564151bf40d9d51cb013490b865

3 files changed, 170 insertions, 2 deletions

diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java
index e800e7fe79..7c22fa2199 100644
--- a/client/src/com/vaadin/client/ui/VScrollTable.java
+++ b/client/src/com/vaadin/client/ui/VScrollTable.java
@@ -324,13 +324,13 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
             ArrayList<SelectionRange> ranges = new ArrayList<SelectionRange>(2);
 
             int endOfFirstRange = row.getIndex() - 1;
-            if (!(endOfFirstRange - startRow.getIndex() < 0)) {
+            if (endOfFirstRange >= startRow.getIndex()) {
                 // create range of first part unless its length is < 1
                 ranges.add(new SelectionRange(startRow, endOfFirstRange
                         - startRow.getIndex() + 1));
             }
             int startOfSecondRange = row.getIndex() + 1;
-            if (!(getEndIndex() - startOfSecondRange < 0)) {
+            if (getEndIndex() >= startOfSecondRange) {
                 // create range of second part unless its length is < 1
                 VScrollTableRow startOfRange = scrollBody
                         .getRowByRowIndex(startOfSecondRange);
@@ -860,6 +860,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
             // Send the selected row ranges
             client.updateVariable(paintableId, "selectedRanges",
                     ranges.toArray(new String[selectedRowRanges.size()]), false);
+            selectedRowRanges.clear();
 
             // clean selectedRowKeys so that they don't contain excess values
             for (Iterator<String> iterator = selectedRowKeys.iterator(); iterator
diff --git a/uitest/src/com/vaadin/tests/components/table/AddSelectionToRemovedRange.java b/uitest/src/com/vaadin/tests/components/table/AddSelectionToRemovedRange.java
new file mode 100644
index 0000000000..d54a8b467a
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/table/AddSelectionToRemovedRange.java
@@ -0,0 +1,93 @@
+/*
+ * Copyright 2000-2013 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.table;
+
+import java.util.Set;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.tests.tb3.AbstractTB3Test.RunLocally;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Notification;
+import com.vaadin.ui.Notification.Type;
+import com.vaadin.ui.Table;
+import com.vaadin.ui.VerticalLayout;
+
+/**
+ * Test to see if selecting and deselecting a table row after select range has
+ * been removed.
+ * 
+ * @since 7.1.13
+ * @author Vaadin Ltd
+ */
+@SuppressWarnings("serial")
+@RunLocally()
+public class AddSelectionToRemovedRange extends AbstractTestUI {
+
+    @Override
+    @SuppressWarnings("unchecked")
+    protected void setup(VaadinRequest request) {
+        VerticalLayout layout = new VerticalLayout();
+        layout.setMargin(true);
+        layout.setSpacing(true);
+        setContent(layout);
+
+        final Table table = new Table();
+        table.setMultiSelect(true);
+        table.setSelectable(true);
+        table.addContainerProperty("value", String.class, "");
+
+        for (int i = 0; i < 100; i++) {
+            table.addItem(i);
+            table.getContainerProperty(i, "value").setValue("value " + i);
+        }
+
+        layout.addComponent(table);
+
+        Button button = new Button("Remove");
+        button.addClickListener(new Button.ClickListener() {
+
+            @Override
+            public void buttonClick(Button.ClickEvent event) {
+                Set<Integer> selected = (Set<Integer>) table.getValue();
+
+                for (Integer item : selected) {
+                    if (null == item) {
+                        new Notification(
+                                "ERROR",
+                                "Table value has null in Set of selected items!",
+                                Type.ERROR_MESSAGE).show(getPage());
+                    }
+                    table.removeItem(item);
+                }
+            }
+        });
+
+        layout.addComponent(button);
+
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Verify that VScrollTable does not return bad ranges when ";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 13353;
+    }
+
+}
diff --git a/uitest/src/com/vaadin/tests/components/table/AddSelectionToRemovedRangeTest.java b/uitest/src/com/vaadin/tests/components/table/AddSelectionToRemovedRangeTest.java
new file mode 100644
index 0000000000..f6a503db72
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/table/AddSelectionToRemovedRangeTest.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2000-2013 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.table;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.NoSuchElementException;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.interactions.Actions;
+import org.openqa.selenium.remote.DesiredCapabilities;
+
+import com.vaadin.testbench.By;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class AddSelectionToRemovedRangeTest extends MultiBrowserTest {
+
+    @Override
+    public List<DesiredCapabilities> getBrowsersToTest() {
+        return Collections.unmodifiableList(Arrays.asList(Browser.CHROME
+                .getDesiredCapabilities()));
+    }
+
+    @Override
+    protected DesiredCapabilities getDesiredCapabilities() {
+        DesiredCapabilities cap = new DesiredCapabilities(
+                super.getDesiredCapabilities());
+        cap.setCapability("requireWindowFocus", true);
+        return cap;
+    }
+
+    @Test
+    public void addAndRemoveItemToRemovedRange() throws IOException {
+        openTestURL();
+        List<WebElement> rows = driver.findElements(By
+                .className("v-table-cell-wrapper"));
+        WebElement rangeStart = rows.get(0);
+        WebElement rangeEnd = rows.get(1);
+        rangeStart.click();
+        new Actions(driver).keyDown(Keys.SHIFT).perform();
+        rangeEnd.click();
+        new Actions(driver).keyUp(Keys.SHIFT).perform();
+        driver.findElement(By.className("v-button")).click();
+        WebElement extraRow = driver.findElements(
+                By.className("v-table-cell-wrapper")).get(1);
+        new Actions(driver).keyDown(Keys.CONTROL).click(extraRow)
+                .click(extraRow).keyUp(Keys.CONTROL).perform();
+        driver.findElement(By.className("v-button")).click();
+        try {
+            driver.findElement(By.vaadin("Root/VNotification[0]"));
+            Assert.fail("Notification is shown");
+        } catch (NoSuchElementException e) {
+            // All is well.
+        }
+    }
+}