DateField ValueChange is now fired after flags are set (#14487)

When the UI had an invalid string the ValueChange event was fired
before setting all the flags causing an invalid isValid result when
handling ValueChange. Now the event is fired after the flags are set.

Change-Id: Ie4e6ba21edc81bf41c2c661aa27e0ace71e1bef0

server/src/com/vaadin/ui/DateField.java

@@ -153,6 +153,13 @@ public class DateField extends AbstractField<Date> implements
 
     private DateRangeValidator currentRangeValidator;
 
+    /**
+     * Determines whether the ValueChangeEvent should be fired. Used to prevent
+     * firing the event when UI has invalid string until uiHasValidDateString
+     * flag is set
+     */
+    private boolean preventValueChangeEvent = false;
+
     static {
         variableNameForResolution.put(Resolution.SECOND, "sec");
         variableNameForResolution.put(Resolution.MINUTE, "min");
@@ -543,13 +550,21 @@ public class DateField extends AbstractField<Date> implements
 
                     /*
                      * Datefield now contains some text that could't be parsed
-                     * into date.
+                     * into date. ValueChangeEvent is fired after the value is
+                     * changed and the flags are set
                      */
                     if (oldDate != null) {
                         /*
-                         * Set the logic value to null.
+                         * Set the logic value to null without firing the
+                         * ValueChangeEvent
                          */
-                        setValue(null);
+                        preventValueChangeEvent = true;
+                        try {
+                            setValue(null);
+                        } finally {
+                            preventValueChangeEvent = false;
+                        }
+
                         /*
                          * Reset the dateString (overridden to null by setValue)
                          */
@@ -570,6 +585,13 @@ public class DateField extends AbstractField<Date> implements
                      */
                     uiHasValidDateString = false;
 
+                    /*
+                     * If value was changed fire the ValueChangeEvent
+                     */
+                    if (oldDate != null) {
+                        fireValueChange(false);
+                    }
+
                     /*
                      * Because of our custom implementation of isValid(), that
                      * also checks the parsingSucceeded flag, we must also
@@ -603,6 +625,16 @@ public class DateField extends AbstractField<Date> implements
         }
     }
 
+    /*
+     * only fires the event if preventValueChangeEvent flag is false
+     */
+    @Override
+    protected void fireValueChange(boolean repaintIsNotNeeded) {
+        if (!preventValueChangeEvent) {
+            super.fireValueChange(repaintIsNotNeeded);
+        }
+    }
+
     /**
      * This method is called to handle a non-empty date string from the client
      * if the client could not parse it as a Date.

uitest/src/com/vaadin/tests/components/datefield/DateFieldIsValid.java

@@ -0,0 +1,69 @@
+package com.vaadin.tests.components.datefield;
+
+import java.text.SimpleDateFormat;
+import java.util.Date;
+
+import com.vaadin.data.Property.ValueChangeEvent;
+import com.vaadin.data.Property.ValueChangeListener;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Button.ClickListener;
+import com.vaadin.ui.DateField;
+
+public class DateFieldIsValid extends AbstractTestUIWithLog {
+
+    @Override
+    protected String getTestDescription() {
+        return "A dateField with invalid text should return false in isValid both when "
+                + "handling ValueChange event and after value is changed.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 14487;
+    }
+
+    private String pattern = "dd/MM/yy";
+    private SimpleDateFormat format = new SimpleDateFormat(pattern);
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        final DateField dateField = new DateField("Insert Date: ");
+        dateField.setImmediate(true);
+        dateField.setDateFormat(pattern);
+
+        dateField.addValueChangeListener(new ValueChangeListener() {
+            @Override
+            public void valueChange(ValueChangeEvent event) {
+                log("valueChange: value: " + format(dateField.getValue())
+                        + ", is valid: " + dateField.isValid());
+            }
+        });
+        addComponent(dateField);
+        Button button = new Button("check dateField");
+        button.addClickListener(new ClickListener() {
+
+            @Override
+            public void buttonClick(ClickEvent event) {
+                log("buttonClick: value: " + format(dateField.getValue())
+                        + ", is valid: " + dateField.isValid());
+            }
+        });
+        addComponent(button);
+    }
+
+    /**
+     * @since
+     * @param value
+     * @return
+     */
+    protected String format(Date value) {
+        if (value != null) {
+            return format.format(value);
+        } else {
+            return null;
+        }
+    }
+}

uitest/src/com/vaadin/tests/components/datefield/DateFieldIsValidTest.java

@@ -0,0 +1,65 @@
+/*
+ * 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.datefield;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.DateFieldElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * @author Vaadin Ltd
+ */
+public class DateFieldIsValidTest extends MultiBrowserTest {
+
+    @Test
+    public void testInvalidText() throws Exception {
+        openTestURL();
+
+        waitForElementVisible(By.id("Log"));
+        waitForElementVisible(By.className("v-datefield"));
+        WebElement dateTextbox = $(DateFieldElement.class).first().findElement(
+                By.className("v-textfield"));
+        ButtonElement button = $(ButtonElement.class).first();
+
+        dateTextbox.sendKeys("01/01/01", Keys.TAB);
+        assertLogText("1. valueChange: value: 01/01/01, is valid: true");
+        button.click();
+        assertLogText("2. buttonClick: value: 01/01/01, is valid: true");
+
+        dateTextbox.sendKeys("lala", Keys.TAB);
+        assertLogText("3. valueChange: value: null, is valid: false");
+        button.click();
+        assertLogText("4. buttonClick: value: null, is valid: false");
+
+        dateTextbox.clear();
+        dateTextbox.sendKeys("02/02/02", Keys.TAB);
+        assertLogText("5. valueChange: value: 02/02/02, is valid: true");
+        button.click();
+        assertLogText("6. buttonClick: value: 02/02/02, is valid: true");
+    }
+
+    private void assertLogText(String expected) throws Exception {
+        String text = findElement(By.vaadin("PID_SLog_row_0")).getText();
+        Assert.assertTrue("Expected '" + expected + "' found '" + text + "'",
+                text.equals(expected));
+    }
+}