Fixes a recently introduced bug in Cell Widgets where clicking on the first row does not update the SelectionModel when bound to selection. The DefaultKeyboardSelectionHandler has an overly optimistic optimization that ignores the click event if it occurs on the current keyboard selected row. However, the keyboard selected row is initialized to 0, and updating the keyboard selected row triggers selection, so the optimization blocks selection of the 0th row. Removing the optimization check fixes the bug.
Review at http://gwt-code-reviews.appspot.com/1509806
Review by: devint@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10506 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/cellview/client/AbstractHasData.java b/user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
index 21dc3c9..25c3f04 100644
--- a/user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
+++ b/user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
@@ -138,16 +138,15 @@
* row, just updating it based on where the user clicked.
*/
int relRow = event.getIndex() - display.getPageStart();
- if (display.getKeyboardSelectedRow() != relRow) {
- // If a natively focusable element was just clicked, then do not steal
- // focus.
- boolean isFocusable = false;
- Element target = Element.as(event.getNativeEvent().getEventTarget());
- isFocusable = CellBasedWidgetImpl.get().isFocusable(target);
- display.setKeyboardSelectedRow(relRow, !isFocusable);
- // Do not cancel the event as the click may have occurred on a Cell.
- }
+ // If a natively focusable element was just clicked, then do not steal
+ // focus.
+ boolean isFocusable = false;
+ Element target = Element.as(event.getNativeEvent().getEventTarget());
+ isFocusable = CellBasedWidgetImpl.get().isFocusable(target);
+ display.setKeyboardSelectedRow(relRow, !isFocusable);
+
+ // Do not cancel the event as the click may have occurred on a Cell.
} else if ("focus".equals(eventType)) {
// Move keyboard focus to match the currently focused element.
int relRow = event.getIndex() - display.getPageStart();
diff --git a/user/test/com/google/gwt/user/cellview/client/CellListTest.java b/user/test/com/google/gwt/user/cellview/client/CellListTest.java
index 98b5b9e..74f388e 100644
--- a/user/test/com/google/gwt/user/cellview/client/CellListTest.java
+++ b/user/test/com/google/gwt/user/cellview/client/CellListTest.java
@@ -22,9 +22,11 @@
import com.google.gwt.safehtml.shared.SafeHtml;
import com.google.gwt.safehtml.shared.SafeHtmlBuilder;
import com.google.gwt.safehtml.shared.SafeHtmlUtils;
+import com.google.gwt.user.cellview.client.HasKeyboardSelectionPolicy.KeyboardSelectionPolicy;
import com.google.gwt.user.client.ui.Label;
import com.google.gwt.user.client.ui.RootPanel;
import com.google.gwt.view.client.ProvidesKey;
+import com.google.gwt.view.client.SingleSelectionModel;
import java.util.ArrayList;
import java.util.List;
@@ -81,6 +83,7 @@
// Create a model with only one level, and three values at that level.
ProvidesKey<String> keyProvider = new ProvidesKey<String>() {
+ @Override
public Object getKey(String item) {
return Integer.parseInt(item.substring(5));
}
@@ -91,6 +94,37 @@
assertEquals(10, rendered.size());
}
+ /**
+ * Test that clicking on the first item selects the item.
+ */
+ public void testSelectFirstItem() {
+ IndexCell<String> cell = new IndexCell<String>();
+ AbstractHasData<String> display = createAbstractHasData(cell);
+ populateData(display);
+
+ // Bind to selection.
+ SingleSelectionModel<String> selectionModel = new SingleSelectionModel<String>();
+ display.setSelectionModel(selectionModel);
+ display.setKeyboardSelectionPolicy(KeyboardSelectionPolicy.BOUND_TO_SELECTION);
+ display.getPresenter().flush();
+ assertNull(selectionModel.getSelectedObject());
+ assertEquals(0, display.getKeyboardSelectedRow());
+
+ // Fire a click event on the first item.
+ RootPanel.get().add(display);
+ NativeEvent clickEvent =
+ Document.get().createClickEvent(0, 0, 0, 0, 0, false, false, false, false);
+ display.getChildElement(0).dispatchEvent(clickEvent);
+ display.getPresenter().flush();
+
+ // Verify that the first item is now selected.
+ assertEquals("test 0", selectionModel.getSelectedObject());
+ assertEquals(0, display.getKeyboardSelectedRow());
+
+ // Cleanup.
+ RootPanel.get().remove(display);
+ }
+
@SuppressWarnings("deprecation")
public void testSetEmptyListWidget() {
CellList<String> cellList = createAbstractHasData(new TextCell());
