In CompositeCell, consumed events are determined as an union of all sub cells' consumed events. However, during event handling, an event may be fired to a sub-cell that doesn't actually consume this event. This is problematic if the cell doesn't check consumed events itself (which in most cases they don't).
Review at http://gwt-code-reviews.appspot.com/1632804
Review by: jlabanca@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10860 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/cell/client/CompositeCell.java b/user/src/com/google/gwt/cell/client/CompositeCell.java
index 8e83e0e..72e9e87 100644
--- a/user/src/com/google/gwt/cell/client/CompositeCell.java
+++ b/user/src/com/google/gwt/cell/client/CompositeCell.java
@@ -219,6 +219,13 @@
private <X> void onBrowserEventImpl(final Context context, Element parent,
final C object, NativeEvent event, final ValueUpdater<C> valueUpdater,
final HasCell<C, X> hasCell) {
+ Cell<X> cell = hasCell.getCell();
+ String eventType = event.getType();
+ Set<String> cellConsumedEvents = cell.getConsumedEvents();
+ if (cellConsumedEvents == null || !cellConsumedEvents.contains(eventType)) {
+ // If this sub-cell doesn't consume this event.
+ return;
+ }
ValueUpdater<X> tempUpdater = null;
final FieldUpdater<C, X> fieldUpdater = hasCell.getFieldUpdater();
if (fieldUpdater != null) {
@@ -232,7 +239,6 @@
}
};
}
- Cell<X> cell = hasCell.getCell();
cell.onBrowserEvent(context, parent, hasCell.getValue(object), event,
tempUpdater);
}
diff --git a/user/test/com/google/gwt/cell/client/CompositeCellTest.java b/user/test/com/google/gwt/cell/client/CompositeCellTest.java
index 2dd7d7f..c69b6df 100644
--- a/user/test/com/google/gwt/cell/client/CompositeCellTest.java
+++ b/user/test/com/google/gwt/cell/client/CompositeCellTest.java
@@ -39,19 +39,7 @@
// Add one cell that consumes events.
List<HasCell<String, ?>> cells = createHasCells(3);
final MockCell<String> mock = new MockCell<String>(true, null);
- cells.add(new HasCell<String, String>() {
- public Cell<String> getCell() {
- return mock;
- }
-
- public FieldUpdater<String, String> getFieldUpdater() {
- return null;
- }
-
- public String getValue(String object) {
- return object;
- }
- });
+ addCell(mock, cells);
CompositeCell<String> cell = new CompositeCell<String>(cells);
assertNull(cell.getConsumedEvents());
assertTrue(cell.dependsOnSelection());
@@ -64,19 +52,7 @@
// Add one cell that consumes events.
List<HasCell<String, ?>> cells = createHasCells(3);
final MockCell<String> mock = new MockCell<String>(false, null, "click");
- cells.add(new HasCell<String, String>() {
- public Cell<String> getCell() {
- return mock;
- }
-
- public FieldUpdater<String, String> getFieldUpdater() {
- return null;
- }
-
- public String getValue(String object) {
- return object;
- }
- });
+ addCell(mock, cells);
CompositeCell<String> cell = new CompositeCell<String>(cells);
assertEquals(1, cell.getConsumedEvents().size());
assertTrue(cell.getConsumedEvents().contains("click"));
@@ -100,19 +76,7 @@
return true;
}
};
- cells.add(new HasCell<String, String>() {
- public Cell<String> getCell() {
- return mock;
- }
-
- public FieldUpdater<String, String> getFieldUpdater() {
- return null;
- }
-
- public String getValue(String object) {
- return object;
- }
- });
+ addCell(mock, cells);
CompositeCell<String> cell = new CompositeCell<String>(cells);
Element parent = Document.get().createDivElement();
parent.setInnerHTML(getExpectedInnerHtml());
@@ -139,15 +103,16 @@
Document.get().getBody().appendChild(parent);
// Create the composite cell and updater.
- List<HasCell<String, ?>> cells = createHasCells(3);
+ List<HasCell<String, ?>> cells = createHasCells(2);
+ MockCell<String> innerCell = new MockCell<String>(false, "fromCell2", "click");
+ addCell(innerCell, cells);
final CompositeCell<String> cell = new CompositeCell<String>(cells);
- MockCell<String> innerCell = (MockCell<String>) cells.get(1).getCell();
// Add an event listener.
EventListener listener = new EventListener() {
public void onBrowserEvent(Event event) {
Context context = new Context(3, 4, "key");
- cell.onBrowserEvent(context, parent, "test", event, null);
+ cell.onBrowserEvent(context, parent, "test-x", event, null);
}
};
DOM.sinkEvents(parent, Event.ONCLICK);
@@ -156,14 +121,20 @@
// Fire the event on one of the inner cells.
NativeEvent event = Document.get().createClickEvent(0, 0, 0, 0, 0, false,
false, false, false);
- Element.as(parent.getChild(1)).dispatchEvent(event);
- innerCell.assertLastEventValue("test-1");
- innerCell.assertLastParentElement(Element.as(parent.getChild(1)));
+ Element.as(parent.getChild(2)).dispatchEvent(event);
+ innerCell.assertLastEventValue("test-x");
+ innerCell.assertLastParentElement(Element.as(parent.getChild(2)));
Context innerContext = innerCell.getLastContext();
assertEquals("key", innerContext.getKey());
assertEquals(3, innerContext.getIndex());
assertEquals(4, innerContext.getColumn());
+ // Fire the event to another cell that doesn't consume this event. Shouldn't respond
+ // to the event
+ MockCell<String> innerCell2 = (MockCell<String>) cells.get(1).getCell();
+ Element.as(parent.getChild(1)).dispatchEvent(event);
+ innerCell2.assertLastEventValue(null);
+
// Remove the element and event listener.
DOM.setEventListener(parent, null);
Document.get().getBody().removeChild(parent);
@@ -213,6 +184,25 @@
}
/**
+ * Add a cell to a {@link HasCell} list.
+ */
+ private void addCell(final Cell<String> cell, List<HasCell<String, ?>> cells) {
+ cells.add(new HasCell<String, String>() {
+ public Cell<String> getCell() {
+ return cell;
+ }
+
+ public FieldUpdater<String, String> getFieldUpdater() {
+ return null;
+ }
+
+ public String getValue(String object) {
+ return object;
+ }
+ });
+ }
+
+ /**
* Create an array of {@link HasCell}.
*
* @param count the number of cells to create
