Fix multiselection pressing shift before starting (#13483)

selectionRangeStart wasn't set when starting a selection pressing shift.
SelectAllRowsTest was rewritten to enable extension to test this issue.

Change-Id: I1b578b28ba89fc8215ec853d92de09f44c2d58e6

5 files changed, 118 insertions, 45 deletions

diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java
index 2e3c110d43..859d600daf 100644
--- a/client/src/com/vaadin/client/ui/VScrollTable.java
+++ b/client/src/com/vaadin/client/ui/VScrollTable.java
@@ -6439,6 +6439,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                 VScrollTableRow startRow = selectionRangeStart;
                 if (startRow == null) {
                     startRow = focusedRow;
+                    selectionRangeStart = focusedRow;
                     // If start row is null then we have a multipage selection
                     // from
                     // above
diff --git a/uitest/src/com/vaadin/tests/components/table/SelectAllRows.java b/uitest/src/com/vaadin/tests/components/table/SelectAllRows.java
index 6cc6a68a7d..4778bca3b5 100644
--- a/uitest/src/com/vaadin/tests/components/table/SelectAllRows.java
+++ b/uitest/src/com/vaadin/tests/components/table/SelectAllRows.java
@@ -22,7 +22,6 @@ import com.vaadin.tests.components.AbstractTestUI;
 import com.vaadin.ui.Button;
 import com.vaadin.ui.Label;
 import com.vaadin.ui.Table;
-import com.vaadin.ui.VerticalLayout;
 
 public class SelectAllRows extends AbstractTestUI {
 
@@ -33,10 +32,6 @@ public class SelectAllRows extends AbstractTestUI {
 
     @Override
     protected void setup(VaadinRequest request) {
-        VerticalLayout layout = new VerticalLayout();
-        layout.setMargin(true);
-        layout.setSpacing(true);
-        setContent(layout);
 
         final Table table = new Table();
         table.setId(TABLE);
@@ -44,16 +39,16 @@ public class SelectAllRows extends AbstractTestUI {
         table.setMultiSelect(true);
         table.setSelectable(true);
         table.addContainerProperty("row", String.class, null);
-        layout.addComponent(table);
+        addComponent(table);
 
         Button button = new Button("Count");
         button.setId(COUNT_SELECTED_BUTTON);
-        layout.addComponent(button);
+        addComponent(button);
 
         final Label label = new Label();
         label.setId(COUNT_OF_SELECTED_ROWS_LABEL);
         label.setCaption("Selected count:");
-        layout.addComponent(label);
+        addComponent(label);
 
         button.addClickListener(new Button.ClickListener() {
 
diff --git a/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirst.java b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirst.java
new file mode 100644
index 0000000000..8d3e922fa9
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirst.java
@@ -0,0 +1,30 @@
+/*
+ * Copyright 2000-2014 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;
+
+public class SelectAllRowsShiftFirst extends SelectAllRows {
+
+    @Override
+    protected String getTestDescription() {
+        return "Selecting all rows does not work by pressing shift and selecting the first row, and then press shift then select last row";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 13483;
+    }
+
+}
diff --git a/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirstTest.java b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirstTest.java
new file mode 100644
index 0000000000..28cf9b0718
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsShiftFirstTest.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2000-2014 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.List;
+
+import org.openqa.selenium.WebElement;
+
+/**
+ * Test to see if all items of the table can be selected by pressing shift and
+ * selecting the first row, and then press shift then select last row (#13483)
+ * 
+ * @author Vaadin Ltd
+ */
+public class SelectAllRowsShiftFirstTest extends SelectAllRowsTest {
+
+    @Override
+    protected void clickFirstRow() {
+        List<WebElement> rows = getVisibleTableRows();
+        shiftClickElement(rows.get(0));
+    }
+
+}
diff --git a/uitest/src/com/vaadin/tests/components/table/SelectAllRowsTest.java b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsTest.java
index d4e8441757..f6c6ca3ddc 100644
--- a/uitest/src/com/vaadin/tests/components/table/SelectAllRowsTest.java
+++ b/uitest/src/com/vaadin/tests/components/table/SelectAllRowsTest.java
@@ -15,9 +15,6 @@
  */
 package com.vaadin.tests.components.table;
 
-import static com.vaadin.tests.components.table.SelectAllRows.COUNT_OF_SELECTED_ROWS_LABEL;
-import static com.vaadin.tests.components.table.SelectAllRows.COUNT_SELECTED_BUTTON;
-import static com.vaadin.tests.components.table.SelectAllRows.TABLE;
 import static com.vaadin.tests.components.table.SelectAllRows.TOTAL_NUMBER_OF_ROWS;
 import static org.junit.Assert.assertEquals;
 
@@ -27,12 +24,24 @@ import java.util.List;
 import org.junit.Test;
 import org.openqa.selenium.By;
 import org.openqa.selenium.Keys;
+import org.openqa.selenium.NoSuchElementException;
+import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.interactions.Actions;
 import org.openqa.selenium.remote.DesiredCapabilities;
+import org.openqa.selenium.support.ui.ExpectedCondition;
 
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.LabelElement;
+import com.vaadin.testbench.elements.TableElement;
 import com.vaadin.tests.tb3.MultiBrowserTest;
 
+/**
+ * Test to see if all items of the table can be selected by selecting first row,
+ * press shift then select last (#13008)
+ * 
+ * @author Vaadin Ltd
+ */
 public class SelectAllRowsTest extends MultiBrowserTest {
 
     @Override
@@ -54,56 +63,58 @@ public class SelectAllRowsTest extends MultiBrowserTest {
     public void testAllRowsAreSelected() {
         openTestURL();
 
-        selectAllRowsInTable();
-        int selectedRows = countSelectedItems();
+        clickFirstRow();
+        scrollTableToBottom();
+        clickLastRow();
 
-        assertEquals(TOTAL_NUMBER_OF_ROWS, selectedRows);
+        assertEquals(TOTAL_NUMBER_OF_ROWS, countSelectedItems());
     }
 
-    private int countSelectedItems() {
-        WebElement countButton = vaadinElementById(COUNT_SELECTED_BUTTON);
-        countButton.click();
-        WebElement countOfSelectedRows = vaadinElementById(COUNT_OF_SELECTED_ROWS_LABEL);
-        String count = countOfSelectedRows.getText();
-        return Integer.parseInt(count);
+    protected void clickFirstRow() {
+        getVisibleTableRows().get(0).click();
     }
 
-    private void selectAllRowsInTable() {
-        clickFirstRow();
-        scrollTableToBottom();
-        new Actions(getDriver()).keyDown(Keys.SHIFT).click(getLastRow())
+    private void clickLastRow() {
+        List<WebElement> rows = getVisibleTableRows();
+        shiftClickElement(rows.get(rows.size() - 1));
+    }
+
+    protected void shiftClickElement(WebElement element) {
+        new Actions(getDriver()).keyDown(Keys.SHIFT).click(element)
                 .keyUp(Keys.SHIFT).perform();
     }
 
-    private WebElement getLastRow() {
-        List<WebElement> rows = allVisibleTableRows();
-        WebElement lastRow = rows.get(rows.size() - 1);
-        return lastRow;
+    private int countSelectedItems() {
+        $(ButtonElement.class).first().click();
+        String count = $(LabelElement.class).get(1).getText();
+        return Integer.parseInt(count);
     }
 
-    private void clickFirstRow() {
-        WebElement firstRow = allVisibleTableRows().get(0);
-        firstRow.click();
+    private TableElement getTable() {
+        return $(TableElement.class).first();
     }
 
     private void scrollTableToBottom() {
-        WebElement table = vaadinElementById(TABLE);
-        testBenchElement(table.findElement(By.className("v-scrollable")))
+        testBenchElement(getTable().findElement(By.className("v-scrollable")))
                 .scroll(TOTAL_NUMBER_OF_ROWS * 30);
+        waitUntilRowIsVisible(TOTAL_NUMBER_OF_ROWS - 1);
+    }
 
-        // Wait for scrolling to complete. Otherwise, clicking last row will
-        // fail with Chrome
-        try {
-            Thread.sleep(200);
-        } catch (InterruptedException e) {
-            e.printStackTrace();
-        }
+    private void waitUntilRowIsVisible(final int row) {
+        waitUntil(new ExpectedCondition<Object>() {
+            @Override
+            public Object apply(WebDriver input) {
+                try {
+                    return getTable().getCell(row, 0) != null;
+                } catch (NoSuchElementException e) {
+                    return false;
+                }
+            }
+        });
     }
 
-    private List<WebElement> allVisibleTableRows() {
-        WebElement table = vaadinElementById(TABLE);
-        List<WebElement> rows = table.findElements(By
-                .cssSelector(".v-table-table tr"));
-        return rows;
+    protected List<WebElement> getVisibleTableRows() {
+        return getTable().findElements(By.cssSelector(".v-table-table tr"));
     }
+
 }