Clear selectionRangeStart if the key map has been cleared (#8584, #8736)

svn changeset:24045/svn branch:6.8
author: Leif Åstrand <leif@vaadin.com> 2012-08-02 12:11:08 +0000
committer: Leif Åstrand <leif@vaadin.com> 2012-08-02 12:11:08 +0000
commit: 6685816c2a548bad3d827d3064a70f35e7b69361 (patch)
tree: 585521325eacebef2fc930e2cbb7938befffbd1e
parent: 85c03155dc02e61dd10a9c861b064204bdeaca9f (diff)
download: vaadin-framework-6685816c2a548bad3d827d3064a70f35e7b69361.tar.gz
vaadin-framework-6685816c2a548bad3d827d3064a70f35e7b69361.zip
4 files changed, 276 insertions, 0 deletions
diff --git a/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java b/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java
index c1f90b0157..e54493ca61 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VScrollTable.java
@@ -109,6 +109,12 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
     public static final String ATTRIBUTE_PAGEBUFFER_FIRST = "pb-ft";
     public static final String ATTRIBUTE_PAGEBUFFER_LAST = "pb-l";
 
+    /**
+     * Tell the client that old keys are no longer valid because the server has
+     * cleared its key map.
+     */
+    public static final String ATTRIBUTE_KEY_MAPPER_RESET = "clearKeyMap";
+
     private static final String ROW_HEADER_COLUMN_KEY = "0";
 
     public static final String CLASSNAME = "v-table";
@@ -1051,6 +1057,20 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
             }
         }
 
+        /*
+         * If the server has (re)initialized the rows, our selectionRangeStart
+         * row will point to an index that the server knows nothing about,
+         * causing problems if doing multi selection with shift. The field will
+         * be cleared a little later when the row focus has been restored.
+         * (#8584)
+         */
+        if (uidl.hasAttribute(ATTRIBUTE_KEY_MAPPER_RESET)
+                && uidl.getBooleanAttribute(ATTRIBUTE_KEY_MAPPER_RESET)
+                && selectionRangeStart != null) {
+            assert !selectionRangeStart.isAttached();
+            selectionRangeStart = focusedRow;
+        }
+
         tabIndex = uidl.hasAttribute("tabindex") ? uidl
                 .getIntAttribute("tabindex") : 0;
         setProperTabIndex();
diff --git a/src/com/vaadin/ui/Table.java b/src/com/vaadin/ui/Table.java
index e96636dd70..e56f6003ab 100644
--- a/src/com/vaadin/ui/Table.java
+++ b/src/com/vaadin/ui/Table.java
@@ -413,6 +413,13 @@ public class Table extends AbstractSelect implements Action.Container,
 
     private boolean painted = false;
 
+    /**
+     * Set to true if the client-side should be informed that the key mapper has
+     * been reset so it can avoid sending back references to keys that are no
+     * longer present.
+     */
+    private boolean keyMapperReset;
+
     /* Table constructors */
 
     /**
@@ -2772,6 +2779,11 @@ public class Table extends AbstractSelect implements Action.Container,
 
         paintVisibleColumns(target);
 
+        if (keyMapperReset) {
+            keyMapperReset = false;
+            target.addAttribute(VScrollTable.ATTRIBUTE_KEY_MAPPER_RESET, true);
+        }
+
         if (dropHandler != null) {
             dropHandler.getAcceptCriterion().paint(target);
         }
@@ -3923,6 +3935,10 @@ public class Table extends AbstractSelect implements Action.Container,
     public void containerItemSetChange(Container.ItemSetChangeEvent event) {
         super.containerItemSetChange(event);
 
+        // super method clears the key map, must inform client about this to
+        // avoid getting invalid keys back (#8584)
+        keyMapperReset = true;
+
         // ensure that page still has first item in page, ignore buffer refresh
         // (forced in this method)
         setCurrentPageFirstItemIndex(getCurrentPageFirstItemIndex(), false);
diff --git a/tests/testbench/com/vaadin/tests/components/table/MultiSelectWithRemovedRow.html b/tests/testbench/com/vaadin/tests/components/table/MultiSelectWithRemovedRow.html
new file mode 100644
index 0000000000..c41f3fbfdd
--- /dev/null
+++ b/tests/testbench/com/vaadin/tests/components/table/MultiSelectWithRemovedRow.html
@@ -0,0 +1,150 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head profile="http://selenium-ide.openqa.org/profiles/test-case">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+<link rel="selenium.base" href="" />
+<title>New Test</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">New Test</td></tr>
+</thead><tbody>
+<tr>
+	<td>open</td>
+	<td>/run/com.vaadin.tests.components.table.MultiSelectWithRemovedRow?restartApplication</td>
+	<td></td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VScrollTable[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[0]/domChild[0]/domChild[0]</td>
+	<td>34,9</td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[2]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::PID_SLog_row_0</td>
+	<td>1. Selection: [Joe]</td>
+</tr>
+<!--Remove selection + shift select-->
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[3]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VScrollTable[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[2]/domChild[0]/domChild[0]</td>
+	<td>19,4:shift</td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[2]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::PID_SLog_row_0</td>
+	<td>2. Selection: [William, Jack, Averell]</td>
+</tr>
+<!--Remove first + shift select-->
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VScrollTable[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[0]</td>
+	<td>23,13</td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VScrollTable[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[3]/domChild[0]/domChild[0]</td>
+	<td>26,10:shift</td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[4]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[2]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::PID_SLog_row_0</td>
+	<td>3. Selection: [Jack, Averell, Bob]</td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VScrollTable[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[3]/domChild[0]/domChild[0]</td>
+	<td>34,14:shift</td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[2]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::PID_SLog_row_0</td>
+	<td>4. Selection: [William, Averell, Bob, Grat]</td>
+</tr>
+<!--Sort + shift select down-->
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VScrollTable[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[3]/domChild[0]/domChild[0]</td>
+	<td>31,15</td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VScrollTable[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[2]</td>
+	<td>36,11</td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VScrollTable[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[2]/domChild[0]/domChild[0]</td>
+	<td>30,6:shift</td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[2]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::PID_SLog_row_0</td>
+	<td>5. Selection: [Bob, Emmett, Grat]</td>
+</tr>
+<!--Sort + shift select up-->
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VScrollTable[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[2]/domChild[0]/domChild[0]</td>
+	<td>24,8</td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VScrollTable[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[1]</td>
+	<td>11,2</td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VScrollTable[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[1]/domChild[0]/domChild[0]</td>
+	<td>30,11:shift</td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[2]/VButton[0]/domChild[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestscomponentstableMultiSelectWithRemovedRow::PID_SLog_row_0</td>
+	<td>6. Selection: [Grat, Emmett, Bob]</td>
+</tr>
+</tbody></table>
+</body>
+</html>
diff --git a/tests/testbench/com/vaadin/tests/components/table/MultiSelectWithRemovedRow.java b/tests/testbench/com/vaadin/tests/components/table/MultiSelectWithRemovedRow.java
new file mode 100644
index 0000000000..64b9c503c3
--- /dev/null
+++ b/tests/testbench/com/vaadin/tests/components/table/MultiSelectWithRemovedRow.java
@@ -0,0 +1,90 @@
+package com.vaadin.tests.components.table;
+
+import java.util.Arrays;
+import java.util.Collection;
+
+import com.vaadin.data.util.BeanItemContainer;
+import com.vaadin.tests.components.TestBase;
+import com.vaadin.tests.util.Log;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Table;
+
+@SuppressWarnings("serial")
+public class MultiSelectWithRemovedRow extends TestBase {
+    public static class Person {
+        private final String name;
+
+        public Person(String name) {
+            this.name = name;
+        }
+
+        public String getName() {
+            return name;
+        }
+
+        @Override
+        public String toString() {
+            return name;
+        }
+    }
+
+    @Override
+    protected void setup() {
+        final Log log = new Log(5);
+        addComponent(log);
+
+        final BeanItemContainer<Person> container = new BeanItemContainer<Person>(
+                Person.class, Arrays.asList(new Person("Joe"), new Person(
+                        "William"), new Person("Jack"), new Person("Averell"),
+                        new Person("Bob"), new Person("Grat"), new Person(
+                                "Bill"), new Person("Emmett")));
+        final Table table = new Table("Table", container);
+        table.setSelectable(true);
+        table.setMultiSelect(true);
+        table.setImmediate(true);
+        addComponent(table);
+
+        Button showButton = new Button("Show selection");
+        showButton.addListener(new Button.ClickListener() {
+            public void buttonClick(Button.ClickEvent clickEvent) {
+                Collection<?> selection = (Collection<?>) table.getValue();
+                log.log("Selection: " + selection);
+            }
+        });
+        addComponent(showButton);
+
+        Button removeButton = new Button("Remove selection");
+        removeButton.addListener(new Button.ClickListener() {
+            public void buttonClick(Button.ClickEvent clickEvent) {
+                Collection<?> selection = (Collection<?>) table.getValue();
+                for (Object selected : selection) {
+                    container.removeItem(selected);
+                }
+            }
+        });
+        addComponent(removeButton);
+
+        addComponent(new Button("Remove first selected row",
+                new Button.ClickListener() {
+                    public void buttonClick(ClickEvent event) {
+                        Collection<?> selection = (Collection<?>) table
+                                .getValue();
+                        if (!selection.isEmpty()) {
+                            Object firstSelected = selection.iterator().next();
+                            container.removeItem(firstSelected);
+                        }
+                    }
+                }));
+    }
+
+    @Override
+    protected String getDescription() {
+        return "Multi select using shift should work after removing the currently selected row";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return Integer.valueOf(8584);
+    }
+}
+\ No newline at end of file
author	Leif Åstrand <leif@vaadin.com>	2012-08-02 12:11:08 +0000
committer	Leif Åstrand <leif@vaadin.com>	2012-08-02 12:11:08 +0000
commit	6685816c2a548bad3d827d3064a70f35e7b69361 (patch)
tree	585521325eacebef2fc930e2cbb7938befffbd1e
parent	85c03155dc02e61dd10a9c861b064204bdeaca9f (diff)
download	vaadin-framework-6685816c2a548bad3d827d3064a70f35e7b69361.tar.gz vaadin-framework-6685816c2a548bad3d827d3064a70f35e7b69361.zip