From 35f353274f2132086f8ee0bb6e6e73712a59b173 Mon Sep 17 00:00:00 2001
From: John Ahlroos <john@vaadin.com>
Date: Tue, 28 May 2013 12:39:20 +0300
Subject: [PATCH] Fixes current text being overwritten in server update on RTA
 #11741

If the server happens to update the state of the RTA while a user is
typing then the users text will be replaced by the value on the server. To fix
this the RTA should not update if the value is the same as the one
cached on the client side.

Also moves blur handling and server<->client syncronization to the
connector.

Change-Id: Ia807b1e2aa210eb881e4b9cea0870c0c5a9254b2
---
 .../src/com/vaadin/client/ui/VPopupView.java  |   8 +-
 .../com/vaadin/client/ui/VRichTextArea.java   | 165 +++++++++++-------
 .../richtextarea/RichTextAreaConnector.java   |  42 +++--
 .../RichTextAreaUpdateWhileTyping.html        |  42 +++++
 .../RichTextAreaUpdateWhileTyping.java        |  51 ++++++
 5 files changed, 229 insertions(+), 79 deletions(-)
 create mode 100644 uitest/src/com/vaadin/tests/components/richtextarea/RichTextAreaUpdateWhileTyping.html
 create mode 100644 uitest/src/com/vaadin/tests/components/richtextarea/RichTextAreaUpdateWhileTyping.java

diff --git a/client/src/com/vaadin/client/ui/VPopupView.java b/client/src/com/vaadin/client/ui/VPopupView.java
index 05fbd2c073..626780efee 100644
--- a/client/src/com/vaadin/client/ui/VPopupView.java
+++ b/client/src/com/vaadin/client/ui/VPopupView.java
@@ -40,6 +40,7 @@ import com.google.gwt.user.client.ui.RootPanel;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.client.ApplicationConnection;
 import com.vaadin.client.ComponentConnector;
+import com.vaadin.client.Util;
 import com.vaadin.client.VCaptionWrapper;
 import com.vaadin.client.VConsole;
 import com.vaadin.client.ui.ShortcutActionHandler.ShortcutActionHandlerOwner;
@@ -313,8 +314,11 @@ public class VPopupView extends HTML implements Iterable<Widget> {
 
         private void checkForRTE(Widget popupComponentWidget2) {
             if (popupComponentWidget2 instanceof VRichTextArea) {
-                ((VRichTextArea) popupComponentWidget2)
-                        .synchronizeContentToServer();
+                ComponentConnector rtaConnector = Util
+                        .findConnectorFor(popupComponentWidget2);
+                if (rtaConnector != null) {
+                    rtaConnector.flush();
+                }
             } else if (popupComponentWidget2 instanceof HasWidgets) {
                 HasWidgets hw = (HasWidgets) popupComponentWidget2;
                 Iterator<Widget> iterator = hw.iterator();
diff --git a/client/src/com/vaadin/client/ui/VRichTextArea.java b/client/src/com/vaadin/client/ui/VRichTextArea.java
index 1498c096ed..7ed6e7c78a 100644
--- a/client/src/com/vaadin/client/ui/VRichTextArea.java
+++ b/client/src/com/vaadin/client/ui/VRichTextArea.java
@@ -16,11 +16,12 @@
 
 package com.vaadin.client.ui;
 
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+
 import com.google.gwt.core.client.Scheduler;
-import com.google.gwt.event.dom.client.BlurEvent;
 import com.google.gwt.event.dom.client.BlurHandler;
-import com.google.gwt.event.dom.client.ChangeEvent;
-import com.google.gwt.event.dom.client.ChangeHandler;
 import com.google.gwt.event.dom.client.KeyDownEvent;
 import com.google.gwt.event.dom.client.KeyDownHandler;
 import com.google.gwt.event.dom.client.KeyPressEvent;
@@ -49,8 +50,8 @@ import com.vaadin.client.ui.richtextarea.VRichTextToolbar;
  * @author Vaadin Ltd.
  * 
  */
-public class VRichTextArea extends Composite implements Field, ChangeHandler,
-        BlurHandler, KeyPressHandler, KeyDownHandler, Focusable {
+public class VRichTextArea extends Composite implements Field, KeyPressHandler,
+        KeyDownHandler, Focusable {
 
     /**
      * The input node CSS classname.
@@ -91,11 +92,10 @@ public class VRichTextArea extends Composite implements Field, ChangeHandler,
 
     private ShortcutActionHandlerOwner hasShortcutActionHandler;
 
-    /** For internal use only. May be removed or replaced in the future. */
-    public String currentValue = "";
-
     private boolean readOnly = false;
 
+    private final Map<BlurHandler, HandlerRegistration> blurHandlers = new HashMap<BlurHandler, HandlerRegistration>();
+
     public VRichTextArea() {
         createRTAComponents();
         fp.add(formatter);
@@ -110,9 +110,19 @@ public class VRichTextArea extends Composite implements Field, ChangeHandler,
     private void createRTAComponents() {
         rta = new RichTextArea();
         rta.setWidth("100%");
-        rta.addBlurHandler(this);
         rta.addKeyDownHandler(this);
         formatter = new VRichTextToolbar(rta);
+
+        // Add blur handlers
+        for (Entry<BlurHandler, HandlerRegistration> handler : blurHandlers
+                .entrySet()) {
+
+            // Remove old registration
+            handler.getValue().removeHandler();
+
+            // Add blur handlers
+            addBlurHandler(handler.getKey());
+        }
     }
 
     public void setEnabled(boolean enabled) {
@@ -127,6 +137,7 @@ public class VRichTextArea extends Composite implements Field, ChangeHandler,
      * Swaps html to rta and visa versa.
      */
     private void swapEditableArea() {
+        String value = getValue();
         if (html.isAttached()) {
             fp.remove(html);
             if (BrowserInfo.get().isWebkit()) {
@@ -134,13 +145,12 @@ public class VRichTextArea extends Composite implements Field, ChangeHandler,
                 createRTAComponents(); // recreate new RTA to bypass #5379
                 fp.add(formatter);
             }
-            rta.setHTML(currentValue);
             fp.add(rta);
         } else {
-            html.setHTML(currentValue);
             fp.remove(rta);
             fp.add(html);
         }
+        setValue(value);
     }
 
     /** For internal use only. May be removed or replaced in the future. */
@@ -180,62 +190,6 @@ public class VRichTextArea extends Composite implements Field, ChangeHandler,
         return readOnly;
     }
 
-    // TODO is this really used, or does everything go via onBlur() only?
-    @Override
-    public void onChange(ChangeEvent event) {
-        synchronizeContentToServer();
-    }
-
-    /**
-     * Method is public to let popupview force synchronization on close.
-     */
-    public void synchronizeContentToServer() {
-        if (client != null && id != null) {
-            final String html = sanitizeRichTextAreaValue(rta.getHTML());
-            if (!html.equals(currentValue)) {
-                client.updateVariable(id, "text", html, immediate);
-                currentValue = html;
-            }
-        }
-    }
-
-    /**
-     * Browsers differ in what they return as the content of a visually empty
-     * rich text area. This method is used to normalize these to an empty
-     * string. See #8004.
-     * 
-     * @param html
-     * @return cleaned html string
-     */
-    private String sanitizeRichTextAreaValue(String html) {
-        BrowserInfo browser = BrowserInfo.get();
-        String result = html;
-        if (browser.isFirefox()) {
-            if ("<br>".equals(html)) {
-                result = "";
-            }
-        } else if (browser.isWebkit()) {
-            if ("<div><br></div>".equals(html)) {
-                result = "";
-            }
-        } else if (browser.isIE()) {
-            if ("<P>&nbsp;</P>".equals(html)) {
-                result = "";
-            }
-        } else if (browser.isOpera()) {
-            if ("<br>".equals(html) || "<p><br></p>".equals(html)) {
-                result = "";
-            }
-        }
-        return result;
-    }
-
-    @Override
-    public void onBlur(BlurEvent event) {
-        synchronizeContentToServer();
-        // TODO notify possible server side blur/focus listeners
-    }
-
     /**
      * @return space used by components paddings and borders
      */
@@ -409,4 +363,81 @@ public class VRichTextArea extends Composite implements Field, ChangeHandler,
         rta.setTabIndex(index);
     }
 
+    /**
+     * Set the value of the text area
+     * 
+     * @param value
+     *            The text value. Can be html.
+     */
+    public void setValue(String value) {
+        if (rta.isAttached()) {
+            rta.setHTML(value);
+        } else {
+            html.setHTML(value);
+        }
+    }
+
+    /**
+     * Get the value the text area
+     */
+    public String getValue() {
+        if (rta.isAttached()) {
+            return rta.getHTML();
+        } else {
+            return html.getHTML();
+        }
+    }
+
+    /**
+     * Browsers differ in what they return as the content of a visually empty
+     * rich text area. This method is used to normalize these to an empty
+     * string. See #8004.
+     * 
+     * @return cleaned html string
+     */
+    public String getSanitazedValue() {
+        BrowserInfo browser = BrowserInfo.get();
+        String result = getValue();
+        if (browser.isFirefox()) {
+            if ("<br>".equals(html)) {
+                result = "";
+            }
+        } else if (browser.isWebkit()) {
+            if ("<div><br></div>".equals(html)) {
+                result = "";
+            }
+        } else if (browser.isIE()) {
+            if ("<P>&nbsp;</P>".equals(html)) {
+                result = "";
+            }
+        } else if (browser.isOpera()) {
+            if ("<br>".equals(html) || "<p><br></p>".equals(html)) {
+                result = "";
+            }
+        }
+        return result;
+    }
+
+    /**
+     * Adds a blur handler to the component.
+     * 
+     * @param blurHandler
+     *            the blur handler to add
+     */
+    public void addBlurHandler(BlurHandler blurHandler) {
+        blurHandlers.put(blurHandler, rta.addBlurHandler(blurHandler));
+    }
+
+    /**
+     * Removes a blur handler.
+     * 
+     * @param blurHandler
+     *            the handler to remove
+     */
+    public void removeBlurHandler(BlurHandler blurHandler) {
+        HandlerRegistration registration = blurHandlers.remove(blurHandler);
+        if (registration != null) {
+            registration.removeHandler();
+        }
+    }
 }
diff --git a/client/src/com/vaadin/client/ui/richtextarea/RichTextAreaConnector.java b/client/src/com/vaadin/client/ui/richtextarea/RichTextAreaConnector.java
index 36182464a3..8135777c0a 100644
--- a/client/src/com/vaadin/client/ui/richtextarea/RichTextAreaConnector.java
+++ b/client/src/com/vaadin/client/ui/richtextarea/RichTextAreaConnector.java
@@ -15,6 +15,8 @@
  */
 package com.vaadin.client.ui.richtextarea;
 
+import com.google.gwt.event.dom.client.BlurEvent;
+import com.google.gwt.event.dom.client.BlurHandler;
 import com.google.gwt.user.client.Event;
 import com.vaadin.client.ApplicationConnection;
 import com.vaadin.client.Paintable;
@@ -24,33 +26,47 @@ import com.vaadin.client.ui.ShortcutActionHandler.BeforeShortcutActionListener;
 import com.vaadin.client.ui.VRichTextArea;
 import com.vaadin.shared.ui.Connect;
 import com.vaadin.shared.ui.Connect.LoadStyle;
+import com.vaadin.shared.util.SharedUtil;
 import com.vaadin.ui.RichTextArea;
 
 @Connect(value = RichTextArea.class, loadStyle = LoadStyle.LAZY)
 public class RichTextAreaConnector extends AbstractFieldConnector implements
         Paintable, BeforeShortcutActionListener {
 
+    /*
+     * Last value received from the server
+     */
+    private String cachedValue = "";
+
+    @Override
+    protected void init() {
+        getWidget().addBlurHandler(new BlurHandler() {
+
+            @Override
+            public void onBlur(BlurEvent event) {
+                flush();
+            }
+        });
+    }
+
     @Override
     public void updateFromUIDL(final UIDL uidl, ApplicationConnection client) {
         getWidget().client = client;
         getWidget().id = uidl.getId();
 
         if (uidl.hasVariable("text")) {
-            getWidget().currentValue = uidl.getStringVariable("text");
-            if (getWidget().rta.isAttached()) {
-                getWidget().rta.setHTML(getWidget().currentValue);
-            } else {
-                getWidget().html.setHTML(getWidget().currentValue);
+            String newValue = uidl.getStringVariable("text");
+            if (!SharedUtil.equals(newValue, cachedValue)) {
+                getWidget().setValue(newValue);
+                cachedValue = newValue;
             }
         }
-        if (isRealUpdate(uidl)) {
-            getWidget().setEnabled(isEnabled());
-        }
 
         if (!isRealUpdate(uidl)) {
             return;
         }
 
+        getWidget().setEnabled(isEnabled());
         getWidget().setReadOnly(isReadOnly());
         getWidget().immediate = getState().immediate;
         int newMaxLength = uidl.hasAttribute("maxLength") ? uidl
@@ -85,7 +101,13 @@ public class RichTextAreaConnector extends AbstractFieldConnector implements
 
     @Override
     public void flush() {
-        getWidget().synchronizeContentToServer();
+        if (getConnection() != null && getConnectorId() != null) {
+            final String html = getWidget().getSanitazedValue();
+            if (!html.equals(cachedValue)) {
+                getConnection().updateVariable(getConnectorId(), "text", html,
+                        getState().immediate);
+                getWidget().setValue(html);
+            }
+        }
     };
-
 }
diff --git a/uitest/src/com/vaadin/tests/components/richtextarea/RichTextAreaUpdateWhileTyping.html b/uitest/src/com/vaadin/tests/components/richtextarea/RichTextAreaUpdateWhileTyping.html
new file mode 100644
index 0000000000..6ddd1b4097
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/richtextarea/RichTextAreaUpdateWhileTyping.html
@@ -0,0 +1,42 @@
+<?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="http://localhost:7070/" />
+<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.richtextarea.RichTextAreaUpdateWhileTyping?restartApplication</td>
+    <td></td>
+</tr>
+<tr>
+    <td>selectFrame</td>
+    <td>xpath=//div[@id='rta']/iframe</td>
+    <td></td>
+</tr>
+<tr>
+    <td>type</td>
+    <td>//body</td>
+    <td>This text should be visible, even after an update while I type.</td>
+</tr>
+<tr>
+    <td>pause</td>
+    <td>2000</td>
+    <td></td>
+</tr>
+<tr>
+    <td>assertText</td>
+    <td>//body</td>
+    <td>This text should be visible, even after an update while I type.</td>
+</tr>
+
+</tbody></table>
+</body>
+</html>
diff --git a/uitest/src/com/vaadin/tests/components/richtextarea/RichTextAreaUpdateWhileTyping.java b/uitest/src/com/vaadin/tests/components/richtextarea/RichTextAreaUpdateWhileTyping.java
new file mode 100644
index 0000000000..df1d6df463
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/richtextarea/RichTextAreaUpdateWhileTyping.java
@@ -0,0 +1,51 @@
+package com.vaadin.tests.components.richtextarea;
+
+import com.vaadin.data.Property;
+import com.vaadin.data.Property.ValueChangeEvent;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.shared.ui.progressindicator.ProgressIndicatorServerRpc;
+import com.vaadin.tests.components.AbstractComponentTest;
+import com.vaadin.tests.components.AbstractTestCase;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.ProgressIndicator;
+import com.vaadin.ui.RichTextArea;
+
+public class RichTextAreaUpdateWhileTyping extends AbstractTestUI {
+
+    private RichTextArea rta;
+
+    @Override
+    protected void setup(VaadinRequest request) {
+
+        // Progress indicator for changing the value of the RTA
+        ProgressIndicator pi = new ProgressIndicator() {
+            {
+                registerRpc(new ProgressIndicatorServerRpc() {
+
+                    @Override
+                    public void poll() {
+                        rta.markAsDirty();
+                    }
+                });
+            }
+        };
+        pi.setHeight("0px");
+        addComponent(pi);
+
+        rta = new RichTextArea();
+        rta.setId("rta");
+        rta.setImmediate(true);
+        addComponent(rta);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        // TODO Auto-generated method stub
+        return null;
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 11741;
+    }
+}
-- 
2.39.5