Fix Checkbox event handling in Grid Editor (#19096)

This patch limits event preventDefault calls to happen only with Tab
navigation. No other events are prevented by default anymore. Earlier 
implementation that prevented too many events was in patch for #16841

Change-Id: I78924d35c86b822295fdaf720b3e4540c43df5c0

2 files changed, 23 insertions, 6 deletions

diff --git a/client/src/com/vaadin/client/widget/grid/DefaultEditorEventHandler.java b/client/src/com/vaadin/client/widget/grid/DefaultEditorEventHandler.java
index 5089f2f15c..0fea7c37bd 100644
--- a/client/src/com/vaadin/client/widget/grid/DefaultEditorEventHandler.java
+++ b/client/src/com/vaadin/client/widget/grid/DefaultEditorEventHandler.java
@@ -154,6 +154,8 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> {
                 rowDelta = (e.getShiftKey() ? -1 : +1);
             } else if (e.getKeyCode() == KEYCODE_MOVE_HORIZONTAL) {
                 colDelta = (e.getShiftKey() ? -1 : +1);
+                // Prevent tab out of Grid Editor
+                event.getDomEvent().preventDefault();
             }
 
             final boolean changed = rowDelta != 0 || colDelta != 0;
@@ -201,10 +203,10 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> {
      */
     protected boolean handleBufferedMoveEvent(EditorDomEvent<T> event) {
         Event e = event.getDomEvent();
-        final EventCellReference<T> cell = event.getCell();
 
         if (e.getType().equals(BrowserEvents.CLICK)
                 && event.getRowIndex() == event.getCell().getRowIndex()) {
+
             editRow(event, event.getRowIndex(), event.getCell()
                     .getColumnIndexDOM());
 
@@ -213,6 +215,9 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> {
         } else if (e.getType().equals(BrowserEvents.KEYDOWN)
                 && e.getKeyCode() == KEYCODE_MOVE_HORIZONTAL) {
 
+            // Prevent tab out of Grid Editor
+            event.getDomEvent().preventDefault();
+
             editRow(event, event.getRowIndex(), event.getFocusedColumnIndex()
                     + (e.getShiftKey() ? -1 : +1));
 
@@ -306,11 +311,6 @@ public class DefaultEditorEventHandler<T> implements Editor.EventHandler<T> {
                     && handleOpenEvent(event);
         }
 
-        if (handled) {
-            // Prevent any defaults for handled events.
-            event.getDomEvent().preventDefault();
-        }
-
         // Buffered mode should swallow all events, if not already handled.
         boolean swallowEvent = event.getGrid().isEditorActive()
                 && editor.isBuffered();
diff --git a/uitest/src/com/vaadin/tests/fieldgroup/BasicCrudGridEditorRowTest.java b/uitest/src/com/vaadin/tests/fieldgroup/BasicCrudGridEditorRowTest.java
index 7b18053052..769fa52323 100644
--- a/uitest/src/com/vaadin/tests/fieldgroup/BasicCrudGridEditorRowTest.java
+++ b/uitest/src/com/vaadin/tests/fieldgroup/BasicCrudGridEditorRowTest.java
@@ -23,6 +23,7 @@ import org.openqa.selenium.Keys;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.interactions.Actions;
 
+import com.vaadin.testbench.elements.CheckBoxElement;
 import com.vaadin.testbench.elements.DateFieldElement;
 import com.vaadin.testbench.elements.GridElement;
 import com.vaadin.testbench.elements.GridElement.GridCellElement;
@@ -72,4 +73,20 @@ public class BasicCrudGridEditorRowTest extends MultiBrowserTest {
                 hasCssClass(dateField, "v-datefield-error"));
     }
 
+    @Test
+    public void testCheckboxInEditorWorks() {
+        GridCellElement ritaBirthdate = grid.getCell(2, 3);
+        // Open editor row
+        new Actions(getDriver()).doubleClick(ritaBirthdate).perform();
+
+        // Get CheckBox
+        GridEditorElement editor = grid.getEditor();
+        CheckBoxElement cb = editor.getField(5).wrap(CheckBoxElement.class);
+
+        // Check values
+        String value = cb.getValue();
+        cb.click(5, 5);
+        Assert.assertNotEquals("Checkbox value did not change", value,
+                cb.getValue());
+    }
 }