#6588 Repainting in TextChangeListener will send wrong value to client

svn changeset:21332/svn branch:6.7

4 files changed, 178 insertions, 3 deletions

diff --git a/src/com/vaadin/terminal/gwt/client/ui/VTextField.java b/src/com/vaadin/terminal/gwt/client/ui/VTextField.java
index 8bc655f39f..f7edf5705f 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/VTextField.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VTextField.java
@@ -64,6 +64,7 @@ public class VTextField extends TextBoxBase implements Paintable, Field,
     private static final String CLASSNAME_PROMPT = "prompt";
     private static final String ATTR_INPUTPROMPT = "prompt";
     public static final String ATTR_TEXTCHANGE_TIMEOUT = "iet";
+    public static final String ATTR_TEXT_CHANGED = "textChanged";
     public static final String VAR_CURSOR = "c";
     public static final String ATTR_TEXTCHANGE_EVENTMODE = "iem";
     private static final String TEXTCHANGE_MODE_EAGER = "EAGER";
@@ -244,8 +245,16 @@ public class VTextField extends TextBoxBase implements Paintable, Field,
             setColumns(new Integer(uidl.getStringAttribute("cols")).intValue());
         }
 
-        final String text = uidl.hasVariable("text") ? uidl
-                .getStringVariable("text") : null;
+        final String text;
+        if (uidl.hasAttribute(ATTR_TEXT_CHANGED)
+                && uidl.getBooleanAttribute(ATTR_TEXT_CHANGED)
+                && uidl.hasVariable("text")) {
+            // Use value from UIDL only if something hans changed on the server
+            text = uidl.getStringVariable("text");
+        } else {
+            // Use what we already have if no change from the server
+            text = prompting ? null : getText();
+        }
         setPrompting(inputPrompt != null && focusedTextField != this
                 && (text == null || text.equals("")));
 
diff --git a/src/com/vaadin/ui/AbstractTextField.java b/src/com/vaadin/ui/AbstractTextField.java
index 4ed76d367b..c5f24ea4bb 100644
--- a/src/com/vaadin/ui/AbstractTextField.java
+++ b/src/com/vaadin/ui/AbstractTextField.java
@@ -92,6 +92,12 @@ public abstract class AbstractTextField extends AbstractField implements
      */
     private boolean changingVariables;
 
+    /**
+     * Track whether the value on the server has actually changed to avoid
+     * updating the text in the input element on every repaint
+     */
+    private boolean localValueChanged = true;
+
     protected AbstractTextField() {
         super();
     }
@@ -123,6 +129,11 @@ public abstract class AbstractTextField extends AbstractField implements
             throw new IllegalStateException(
                     "Null values are not allowed if the null-representation is null");
         }
+
+        if (localValueChanged || target.isFullRepaint()) {
+            target.addAttribute(VTextField.ATTR_TEXT_CHANGED, true);
+            localValueChanged = false;
+        }
         target.addVariable(this, "text", value);
 
         if (selectionPosition != -1) {
@@ -213,7 +224,8 @@ public abstract class AbstractTextField extends AbstractField implements
                 if (newValue != oldValue
                         && (newValue == null || !newValue.equals(oldValue))) {
                     boolean wasModified = isModified();
-                    setValue(newValue, true);
+                    // Don't update the local change flag
+                    super.setValue(newValue, true);
 
                     // If the modified status changes, or if we have a
                     // formatter, repaint is needed after all.
@@ -238,6 +250,26 @@ public abstract class AbstractTextField extends AbstractField implements
     }
 
     @Override
+    protected void setValue(Object newValue, boolean repaintIsNotNeeded)
+            throws ReadOnlyException, ConversionException {
+        if (isChanged(newValue, getValue())
+                || isChanged(newValue, lastKnownTextContent)) {
+            // The client should use the new value
+            localValueChanged = true;
+            if (!repaintIsNotNeeded) {
+                // Repaint even if super.setValue doesn't detect any change
+                requestRepaint();
+            }
+        }
+        super.setValue(newValue, repaintIsNotNeeded);
+    }
+
+    private static boolean isChanged(Object newValue, Object oldValue) {
+        return oldValue != newValue
+                && (newValue == null || !newValue.equals(oldValue));
+    }
+
+    @Override
     public Class getType() {
         return String.class;
     }
diff --git a/tests/src/com/vaadin/tests/components/textfield/TextFieldEagerRepaint.html b/tests/src/com/vaadin/tests/components/textfield/TextFieldEagerRepaint.html
new file mode 100644
index 0000000000..ecd0467fb2
--- /dev/null
+++ b/tests/src/com/vaadin/tests/components/textfield/TextFieldEagerRepaint.html
@@ -0,0 +1,81 @@
+<?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.textfield.TextFieldEagerRepaint?restartApplication</td>
+	<td></td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstextfieldTextFieldEagerRepaint::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTextField[0]</td>
+	<td>78,8</td>
+</tr>
+<tr>
+	<td>enterCharacter</td>
+	<td>vaadin=runcomvaadintestscomponentstextfieldTextFieldEagerRepaint::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTextField[0]</td>
+	<td>abCDef</td>
+</tr>
+<tr>
+	<td>pause</td>
+	<td></td>
+	<td>100</td>
+</tr>
+<tr>
+	<td>assertValue</td>
+	<td>vaadin=runcomvaadintestscomponentstextfieldTextFieldEagerRepaint::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VTextField[0]</td>
+	<td>abef</td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentstextfieldTextFieldEagerRepaint::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VTextField[0]</td>
+	<td>73,15</td>
+</tr>
+<tr>
+	<td>enterCharacter</td>
+	<td>vaadin=runcomvaadintestscomponentstextfieldTextFieldEagerRepaint::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VTextField[0]</td>
+	<td>a</td>
+</tr>
+<tr>
+	<td>pause</td>
+	<td></td>
+	<td>100</td>
+</tr>
+<tr>
+	<td>assertElementWidth</td>
+	<td>vaadin=runcomvaadintestscomponentstextfieldTextFieldEagerRepaint::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VTextField[0]</td>
+	<td>150</td>
+</tr>
+<tr>
+	<td>enterCharacter</td>
+	<td>vaadin=runcomvaadintestscomponentstextfieldTextFieldEagerRepaint::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VTextField[0]</td>
+	<td>aB</td>
+</tr>
+<tr>
+	<td>pause</td>
+	<td></td>
+	<td>100</td>
+</tr>
+<tr>
+	<td>assertElementWidth</td>
+	<td>vaadin=runcomvaadintestscomponentstextfieldTextFieldEagerRepaint::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VTextField[0]</td>
+	<td>100</td>
+</tr>
+<tr>
+	<td>assertValue</td>
+	<td>vaadin=runcomvaadintestscomponentstextfieldTextFieldEagerRepaint::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VTextField[0]</td>
+	<td>aB</td>
+</tr>
+</tbody></table>
+</body>
+</html>
diff --git a/tests/src/com/vaadin/tests/components/textfield/TextFieldEagerRepaint.java b/tests/src/com/vaadin/tests/components/textfield/TextFieldEagerRepaint.java
new file mode 100644
index 0000000000..08751a59fd
--- /dev/null
+++ b/tests/src/com/vaadin/tests/components/textfield/TextFieldEagerRepaint.java
@@ -0,0 +1,53 @@
+package com.vaadin.tests.components.textfield;
+
+import com.vaadin.event.FieldEvents.TextChangeEvent;
+import com.vaadin.event.FieldEvents.TextChangeListener;
+import com.vaadin.tests.components.TestBase;
+import com.vaadin.ui.AbstractTextField.TextChangeEventMode;
+import com.vaadin.ui.TextField;
+
+public class TextFieldEagerRepaint extends TestBase {
+
+    @Override
+    protected void setup() {
+
+        final TextField tf1 = new TextField("Updates value");
+        tf1.setTextChangeEventMode(TextChangeEventMode.EAGER);
+        tf1.addListener(new TextChangeListener() {
+            public void textChange(TextChangeEvent event) {
+                String text = event.getText();
+                if (!text.matches("[a-z]*")) {
+                    String newValue = text.replaceAll("[^a-z]", "");
+                    tf1.setValue(newValue);
+                }
+            }
+        });
+
+        final TextField tf2 = new TextField("Updates width");
+        tf2.setTextChangeEventMode(TextChangeEventMode.EAGER);
+        tf2.addListener(new TextChangeListener() {
+            public void textChange(TextChangeEvent event) {
+                String text = event.getText();
+                if (!text.matches("[a-z]*")) {
+                    tf2.setWidth("100px");
+                } else {
+                    tf2.setWidth("150px");
+                }
+            }
+        });
+
+        addComponent(tf1);
+        addComponent(tf2);
+    }
+
+    @Override
+    protected String getDescription() {
+        return "Updating the value in an EAGER TextChangeListener should send the new value to the client while updating something else (e.g. the width) should preserve the text in the field. Both fields react when the field contains anything else than lower case letters a-z";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return Integer.valueOf(6588);
+    }
+
+}