Refactor error messages not to use UIDL (#8437).

This change removes support for error messages on tabs of a tabsheet or
an accordion. Those should be implemented differently if needed.

src/com/vaadin/Application.java

@@ -37,11 +37,10 @@ import com.vaadin.data.util.converter.Converter;
 import com.vaadin.data.util.converter.ConverterFactory;
 import com.vaadin.data.util.converter.DefaultConverterFactory;
 import com.vaadin.service.ApplicationContext;
+import com.vaadin.terminal.AbstractErrorMessage;
 import com.vaadin.terminal.ApplicationResource;
 import com.vaadin.terminal.CombinedRequest;
-import com.vaadin.terminal.ErrorMessage;
 import com.vaadin.terminal.RequestHandler;
-import com.vaadin.terminal.SystemError;
 import com.vaadin.terminal.Terminal;
 import com.vaadin.terminal.VariableOwner;
 import com.vaadin.terminal.WrappedRequest;
@@ -1006,12 +1005,8 @@ public class Application implements Terminal.ErrorListener, Serializable {
 
         // Shows the error in AbstractComponent
         if (owner instanceof AbstractComponent) {
-            if (t instanceof ErrorMessage) {
-                ((AbstractComponent) owner).setComponentError((ErrorMessage) t);
-            } else {
-                ((AbstractComponent) owner)
-                        .setComponentError(new SystemError(t));
-            }
+            ((AbstractComponent) owner).setComponentError(AbstractErrorMessage
+                    .getErrorMessageForException(t));
         }
 
         // also print the error on console

src/com/vaadin/terminal/AbstractErrorMessage.java

@@ -9,7 +9,6 @@ import java.util.List;
 
 import com.vaadin.data.Buffered;
 import com.vaadin.data.Validator;
-import com.vaadin.terminal.gwt.client.ui.AbstractComponentConnector;
 import com.vaadin.terminal.gwt.server.AbstractApplicationServlet;
 
 /**
@@ -58,7 +57,7 @@ public abstract class AbstractErrorMessage implements ErrorMessage {
         this.message = message;
     }
 
-    protected String getMessage() {
+    public String getMessage() {
         return message;
     }
 
@@ -91,43 +90,41 @@ public abstract class AbstractErrorMessage implements ErrorMessage {
         causes.add(cause);
     }
 
-    @Deprecated
-    public void paint(PaintTarget target) throws PaintException {
-
-        // TODO if no message and only one cause, paint cause only? error level?
-
-        target.startTag(AbstractComponentConnector.ATTRIBUTE_ERROR);
-        target.addAttribute("level", level.getText());
-
-        paintContent(target);
-
-        target.endTag(AbstractComponentConnector.ATTRIBUTE_ERROR);
-    }
-
-    // TODO temporary method - move logic to client side
-    @Deprecated
-    protected void paintContent(PaintTarget target) throws PaintException {
-        // Paint the message
+    public String getFormattedHtmlMessage() {
+        String result = null;
         switch (getMode()) {
         case TEXT:
-            target.addText(AbstractApplicationServlet
-                    .safeEscapeForHtml(getMessage()));
+            result = AbstractApplicationServlet.safeEscapeForHtml(getMessage());
             break;
         case PREFORMATTED:
-            target.addText("<pre>"
+            result = "<pre>"
                     + AbstractApplicationServlet
-                            .safeEscapeForHtml(getMessage()) + "</pre>");
+                            .safeEscapeForHtml(getMessage()) + "</pre>";
             break;
         case XHTML:
-            target.addText(getMessage());
+            result = getMessage();
             break;
         }
-
-        if (getCauses().size() > 0) {
+        // if no message, combine the messages of all children
+        if (null == result && null != getCauses() && getCauses().size() > 0) {
+            StringBuilder sb = new StringBuilder();
             for (ErrorMessage cause : getCauses()) {
-                cause.paint(target);
+                String childMessage = cause.getFormattedHtmlMessage();
+                if (null != childMessage) {
+                    sb.append("<div>");
+                    sb.append(childMessage);
+                    sb.append("</div>\n");
+                }
             }
+            if (sb.length() > 0) {
+                result = sb.toString();
+            }
+        }
+        // still no message? use an empty string for backwards compatibility
+        if (null == result) {
+            result = "";
         }
+        return result;
     }
 
     // TODO replace this with a helper method elsewhere?

src/com/vaadin/terminal/ErrorMessage.java

@@ -111,19 +111,16 @@ public interface ErrorMessage extends Serializable {
     public ErrorLevel getErrorLevel();
 
     /**
-     * <p>
-     * Paints the error message into a UIDL stream. This method creates the UIDL
-     * sequence describing it and outputs it to the given UIDL stream.
-     * </p>
+     * Returns the HTML formatted message to show in as the error message on the
+     * client.
      * 
-     * @param target
-     *            the target UIDL stream where the error should paint itself to.
-     * @throws PaintException
-     *             if the paint operation failed.
-     * @deprecated error messages should not be painted to UIDL but sent
-     *             differently
+     * This method should perform any necessary escaping to avoid XSS attacks.
+     * 
+     * TODO this API may still change to use a separate data transfer object
+     * 
+     * @return HTML formatted string for the error message
+     * @since 7.0
      */
-    @Deprecated
-    public void paint(PaintTarget target) throws PaintException;
+    public String getFormattedHtmlMessage();
 
 }

src/com/vaadin/terminal/gwt/client/ComponentState.java

@@ -38,6 +38,11 @@ public class ComponentState extends SharedState {
      */
     private Set<String> registeredEventListeners = null;
 
+    // HTML formatted error message for the component
+    // TODO this could be an object with more information, but currently the UI
+    // only uses the message
+    private String errorMessage = null;
+
     /**
      * Returns the component height as set by the server.
      * 
@@ -376,7 +381,29 @@ public class ComponentState extends SharedState {
         if (registeredEventListeners.size() == 0) {
             registeredEventListeners = null;
         }
+    }
 
+    /**
+     * Returns the current error message for the component.
+     * 
+     * @return HTML formatted error message to show for the component or null if
+     *         none
+     */
+    public String getErrorMessage() {
+        return errorMessage;
+    }
+
+    /**
+     * Sets the current error message for the component.
+     * 
+     * TODO this could use an object with more details about the error
+     * 
+     * @param errorMessage
+     *            HTML formatted error message to show for the component or null
+     *            for none
+     */
+    public void setErrorMessage(String errorMessage) {
+        this.errorMessage = errorMessage;
     }
 
 }

src/com/vaadin/terminal/gwt/client/TooltipInfo.java

@@ -7,7 +7,7 @@ public class TooltipInfo {
 
     private String title;
 
-    private UIDL errorUidl;
+    private String errorMessageHtml;
 
     public TooltipInfo() {
     }
@@ -24,12 +24,12 @@ public class TooltipInfo {
         this.title = title;
     }
 
-    public UIDL getErrorUidl() {
-        return errorUidl;
+    public String getErrorMessage() {
+        return errorMessageHtml;
     }
 
-    public void setErrorUidl(UIDL errorUidl) {
-        this.errorUidl = errorUidl;
+    public void setErrorMessage(String errorMessage) {
+        errorMessageHtml = errorMessage;
     }
 
 }

src/com/vaadin/terminal/gwt/client/UIDL.java

@@ -493,17 +493,6 @@ public final class UIDL extends JavaScriptObject {
         return this.length - 2;
     }-*/;
 
-    /**
-     * Shorthand that returns the component errors as UIDL. Only applicable for
-     * Paintables.
-     * 
-     * @return the error UIDL if available
-     */
-    public native UIDL getErrors()
-    /*-{
-        return this[1]['error']; 
-    }-*/;
-
     native boolean isMapAttribute(String name)
     /*-{
         return typeof this[1][name] == "object";

src/com/vaadin/terminal/gwt/client/VCaption.java

@@ -32,11 +32,9 @@ public class VCaption extends HTML {
 
     private int maxWidth = -1;
 
-    protected static final String ATTRIBUTE_ICON = AbstractComponentConnector.ATTRIBUTE_ICON;
     protected static final String ATTRIBUTE_CAPTION = "caption";
     protected static final String ATTRIBUTE_DESCRIPTION = "description";
     protected static final String ATTRIBUTE_REQUIRED = AbstractComponentConnector.ATTRIBUTE_REQUIRED;
-    protected static final String ATTRIBUTE_ERROR = AbstractComponentConnector.ATTRIBUTE_ERROR;
     protected static final String ATTRIBUTE_HIDEERRORS = AbstractComponentConnector.ATTRIBUTE_HIDEERRORS;
 
     private enum InsertPosition {
@@ -120,8 +118,7 @@ public class VCaption extends HTML {
         boolean hasIcon = owner.getState().getIcon() != null;
         boolean showRequired = uidl
                 .getBooleanAttribute(AbstractComponentConnector.ATTRIBUTE_REQUIRED);
-        boolean showError = uidl
-                .hasAttribute(AbstractComponentConnector.ATTRIBUTE_ERROR)
+        boolean showError = owner.getState().getErrorMessage() != null
                 && !uidl.getBooleanAttribute(AbstractComponentConnector.ATTRIBUTE_HIDEERRORS);
 
         if (hasIcon) {
@@ -279,9 +276,6 @@ public class VCaption extends HTML {
             }
         }
         boolean hasIcon = iconURL != null;
-        boolean showError = uidl
-                .hasAttribute(AbstractComponentConnector.ATTRIBUTE_ERROR)
-                && !uidl.getBooleanAttribute(AbstractComponentConnector.ATTRIBUTE_HIDEERRORS);
 
         if (hasIcon) {
             if (icon == null) {
@@ -324,7 +318,7 @@ public class VCaption extends HTML {
                 // browsers when it is set to the empty string. If there is an
                 // icon, error indicator or required indicator they will ensure
                 // that space is reserved.
-                if (!hasIcon && !showError) {
+                if (!hasIcon) {
                     captionText.setInnerHTML(" ");
                 }
             } else {
@@ -337,22 +331,6 @@ public class VCaption extends HTML {
             captionText = null;
         }
 
-        if (showError) {
-            if (errorIndicatorElement == null) {
-                errorIndicatorElement = DOM.createDiv();
-                DOM.setInnerHTML(errorIndicatorElement, " ");
-                DOM.setElementProperty(errorIndicatorElement, "className",
-                        "v-errorindicator");
-
-                DOM.insertChild(getElement(), errorIndicatorElement,
-                        getInsertPosition(InsertPosition.ERROR));
-            }
-        } else if (errorIndicatorElement != null) {
-            // Remove existing
-            getElement().removeChild(errorIndicatorElement);
-            errorIndicatorElement = null;
-        }
-
         return (wasPlacedAfterComponent != placedAfterComponent);
     }
 
@@ -400,6 +378,9 @@ public class VCaption extends HTML {
             if (state.getIcon() != null) {
                 return true;
             }
+            if (state.getErrorMessage() != null) {
+                return true;
+            }
         } else {
             // TODO fallback for cases where the caption has no owner (Tabsheet,
             // Accordion)
@@ -410,9 +391,6 @@ public class VCaption extends HTML {
                 return true;
             }
         }
-        if (uidl.hasAttribute(AbstractComponentConnector.ATTRIBUTE_ERROR)) {
-            return true;
-        }
         if (uidl.hasAttribute(AbstractComponentConnector.ATTRIBUTE_REQUIRED)) {
             return true;
         }

src/com/vaadin/terminal/gwt/client/VErrorMessage.java

@@ -4,8 +4,6 @@
 
 package com.vaadin.terminal.gwt.client;
 
-import java.util.Iterator;
-
 import com.google.gwt.user.client.DOM;
 import com.google.gwt.user.client.Element;
 import com.google.gwt.user.client.ui.FlowPanel;
@@ -20,30 +18,13 @@ public class VErrorMessage extends FlowPanel {
         setStyleName(CLASSNAME);
     }
 
-    public void updateFromUIDL(UIDL uidl) {
+    public void updateMessage(String htmlErrorMessage) {
         clear();
-        if (uidl.getChildCount() == 0) {
+        if (htmlErrorMessage == null || htmlErrorMessage.length() == 0) {
             add(new HTML(" "));
         } else {
-            for (final Iterator<?> it = uidl.getChildIterator(); it.hasNext();) {
-                final Object child = it.next();
-                if (child instanceof String) {
-                    final String errorMessage = (String) child;
-                    add(new HTML(errorMessage));
-                } else {
-                    try {
-                        final VErrorMessage childError = new VErrorMessage();
-                        childError.updateFromUIDL((UIDL) child);
-                        add(childError);
-                    } catch (Exception e) {
-                        // TODO XML type error, check if this can even happen
-                        // anymore??
-                        final UIDL.XML xml = (UIDL.XML) child;
-                        add(new HTML(xml.getXMLAsString()));
-
-                    }
-                }
-            }
+            // pre-formatted on the server as div per child
+            add(new HTML(htmlErrorMessage));
         }
     }
 

src/com/vaadin/terminal/gwt/client/VTooltip.java

@@ -56,9 +56,9 @@ public class VTooltip extends VOverlay {
      */
     private void show(TooltipInfo info) {
         boolean hasContent = false;
-        if (info.getErrorUidl() != null) {
+        if (info.getErrorMessage() != null) {
             em.setVisible(true);
-            em.updateFromUIDL(info.getErrorUidl());
+            em.updateMessage(info.getErrorMessage());
             hasContent = true;
         } else {
             em.setVisible(false);

src/com/vaadin/terminal/gwt/client/ui/AbstractComponentConnector.java

@@ -35,7 +35,6 @@ public abstract class AbstractComponentConnector extends AbstractConnector
     // Not all references to the string literals have been converted to use
     // these!
     public static final String ATTRIBUTE_REQUIRED = "required";
-    public static final String ATTRIBUTE_ERROR = "error";
     public static final String ATTRIBUTE_HIDEERRORS = "hideErrors";
 
     private Widget widget;
@@ -168,11 +167,7 @@ public abstract class AbstractComponentConnector extends AbstractConnector
             tooltipInfo.setTitle(null);
         }
         // add error info to tooltip if present
-        if (uidl.hasAttribute(ATTRIBUTE_ERROR)) {
-            tooltipInfo.setErrorUidl(uidl.getErrors());
-        } else {
-            tooltipInfo.setErrorUidl(null);
-        }
+        tooltipInfo.setErrorMessage(getState().getErrorMessage());
 
         // Set captions
         if (delegateCaptionHandling()) {
@@ -358,7 +353,7 @@ public abstract class AbstractComponentConnector extends AbstractConnector
         }
 
         // add error classname to components w/ error
-        if (uidl.hasAttribute(ATTRIBUTE_ERROR)) {
+        if (null != state.getErrorMessage()) {
             styleBuf.append(" ");
             styleBuf.append(primaryStyleName);
             styleBuf.append(ApplicationConnection.ERROR_CLASSNAME_EXT);

src/com/vaadin/terminal/gwt/client/ui/ButtonConnector.java

@@ -74,7 +74,7 @@ public class ButtonConnector extends AbstractComponentConnector {
         getWidget().disableOnClick = getState().isDisableOnClick();
 
         // handle error
-        if (uidl.hasAttribute(ATTRIBUTE_ERROR)) {
+        if (null != getState().getErrorMessage()) {
             if (getWidget().errorIndicatorElement == null) {
                 getWidget().errorIndicatorElement = DOM.createSpan();
                 getWidget().errorIndicatorElement

src/com/vaadin/terminal/gwt/client/ui/CheckBoxConnector.java

@@ -36,7 +36,7 @@ public class CheckBoxConnector extends AbstractFieldConnector {
         getWidget().blurHandlerRegistration = EventHelper.updateBlurHandler(
                 this, client, getWidget().blurHandlerRegistration);
 
-        if (uidl.hasAttribute(ATTRIBUTE_ERROR)) {
+        if (null != getState().getErrorMessage()) {
             if (getWidget().errorIndicatorElement == null) {
                 getWidget().errorIndicatorElement = DOM.createSpan();
                 getWidget().errorIndicatorElement.setInnerHTML(" ");

src/com/vaadin/terminal/gwt/client/ui/FormConnector.java

@@ -64,9 +64,9 @@ public class FormConnector extends AbstractComponentContainerConnector
             getWidget().removeStyleDependentName("nocaption");
         }
 
-        if (uidl.hasAttribute(ATTRIBUTE_ERROR)) {
-            final UIDL errorUidl = uidl.getErrors();
-            getWidget().errorMessage.updateFromUIDL(errorUidl);
+        if (null != getState().getErrorMessage()) {
+            getWidget().errorMessage.updateMessage(getState()
+                    .getErrorMessage());
             getWidget().errorMessage.setVisible(true);
         } else {
             getWidget().errorMessage.setVisible(false);

src/com/vaadin/terminal/gwt/client/ui/LinkConnector.java

@@ -60,7 +60,7 @@ public class LinkConnector extends AbstractComponentConnector {
         getWidget().captionElement.setInnerText(getState().getCaption());
 
         // handle error
-        if (uidl.hasAttribute(ATTRIBUTE_ERROR)) {
+        if (null != getState().getErrorMessage()) {
             if (getWidget().errorIndicatorElement == null) {
                 getWidget().errorIndicatorElement = DOM.createDiv();
                 DOM.setElementProperty(getWidget().errorIndicatorElement,

src/com/vaadin/terminal/gwt/client/ui/NativeButtonConnector.java

@@ -51,7 +51,7 @@ public class NativeButtonConnector extends AbstractComponentConnector {
         getWidget().setText(getState().getCaption());
 
         // handle error
-        if (uidl.hasAttribute(ATTRIBUTE_ERROR)) {
+        if (null != getState().getErrorMessage()) {
             if (getWidget().errorIndicatorElement == null) {
                 getWidget().errorIndicatorElement = DOM.createSpan();
                 getWidget().errorIndicatorElement

src/com/vaadin/terminal/gwt/client/ui/PanelConnector.java

@@ -143,7 +143,8 @@ public class PanelConnector extends AbstractComponentContainerConnector
             getWidget().setIconUri(null, client);
         }
 
-        getWidget().handleError(uidl);
+        getWidget().setErrorIndicatorVisible(
+                null != getState().getErrorMessage());
 
         // We may have actions attached to this panel
         if (uidl.getChildCount() > 0) {

src/com/vaadin/terminal/gwt/client/ui/VFormLayout.java

@@ -351,7 +351,7 @@ public class VFormLayout extends SimplePanel {
 
         public void updateFromUIDL(UIDL uidl, ComponentConnector component) {
             owner = component;
-            if (uidl.hasAttribute(AbstractComponentConnector.ATTRIBUTE_ERROR)
+            if (null != owner.getState().getErrorMessage()
                     && !uidl.getBooleanAttribute(AbstractComponentConnector.ATTRIBUTE_HIDEERRORS)) {
                 if (errorIndicatorElement == null) {
                     errorIndicatorElement = DOM.createDiv();

src/com/vaadin/terminal/gwt/client/ui/VPanel.java

@@ -114,8 +114,8 @@ public class VPanel extends SimplePanel implements ShortcutActionHandlerOwner,
         DOM.setInnerHTML(captionText, text);
     }
 
-    void handleError(UIDL uidl) {
-        if (uidl.hasAttribute(AbstractComponentConnector.ATTRIBUTE_ERROR)) {
+    void setErrorIndicatorVisible(boolean showError) {
+        if (showError) {
             if (errorIndicatorElement == null) {
                 errorIndicatorElement = DOM.createSpan();
                 DOM.setElementProperty(errorIndicatorElement, "className",

src/com/vaadin/terminal/gwt/client/ui/VTabsheet.java

@@ -248,15 +248,12 @@ public class VTabsheet extends VTabsheetBase implements Focusable,
 
         @Override
         public boolean updateCaption(UIDL uidl) {
-            if (uidl.hasAttribute(TabsheetBaseConnector.ATTRIBUTE_TAB_DESCRIPTION)
-                    || uidl.hasAttribute(AbstractComponentConnector.ATTRIBUTE_ERROR)) {
+            if (uidl.hasAttribute(TabsheetBaseConnector.ATTRIBUTE_TAB_DESCRIPTION)) {
                 TooltipInfo tooltipInfo = new TooltipInfo();
                 tooltipInfo
                         .setTitle(uidl
                                 .getStringAttribute(TabsheetBaseConnector.ATTRIBUTE_TAB_DESCRIPTION));
-                if (uidl.hasAttribute(AbstractComponentConnector.ATTRIBUTE_ERROR)) {
-                    tooltipInfo.setErrorUidl(uidl.getErrors());
-                }
+                // TODO currently, there is no error indicator and message for a tab
                 client.registerTooltip(getTabsheet(), getElement(), tooltipInfo);
             } else {
                 client.registerTooltip(getTabsheet(), getElement(), null);

src/com/vaadin/terminal/gwt/server/JsonPaintTarget.java

@@ -34,7 +34,6 @@ import com.vaadin.terminal.StreamVariable;
 import com.vaadin.terminal.ThemeResource;
 import com.vaadin.terminal.VariableOwner;
 import com.vaadin.terminal.gwt.client.Connector;
-import com.vaadin.terminal.gwt.client.ui.AbstractComponentConnector;
 import com.vaadin.ui.Alignment;
 import com.vaadin.ui.ClientWidget;
 import com.vaadin.ui.CustomLayout;
@@ -83,8 +82,6 @@ public class JsonPaintTarget implements PaintTarget {
 
     private JsonTag tag;
 
-    private int errorsOpen;
-
     private boolean cacheEnabled = false;
 
     private final Collection<Paintable> paintedComponents = new HashSet<Paintable>();
@@ -162,10 +159,6 @@ public class JsonPaintTarget implements PaintTarget {
 
         tag = new JsonTag(tagName);
 
-        if (AbstractComponentConnector.ATTRIBUTE_ERROR.equals(tagName)) {
-            errorsOpen++;
-        }
-
         customLayoutArgumentsOpen = false;
 
     }
@@ -204,22 +197,7 @@ public class JsonPaintTarget implements PaintTarget {
                         + tagName + "' expected: '" + lastTag + "'.");
             }
 
-            // simple hack which writes error uidl structure into attribute
-            if (AbstractComponentConnector.ATTRIBUTE_ERROR.equals(lastTag)) {
-                if (errorsOpen == 1) {
-                    parent.addAttribute("\""
-                            + AbstractComponentConnector.ATTRIBUTE_ERROR
-                            + "\":[\""
-                            + AbstractComponentConnector.ATTRIBUTE_ERROR
-                            + "\",{}" + tag.getData() + "]");
-                } else {
-                    // sub error
-                    parent.addData(tag.getJSON());
-                }
-                errorsOpen--;
-            } else {
-                parent.addData(tag.getJSON());
-            }
+            parent.addData(tag.getJSON());
 
             tag = parent;
         } else {

src/com/vaadin/ui/AbstractComponent.java

@@ -754,10 +754,6 @@ public abstract class AbstractComponent implements Component, MethodEventSource
             // Paint the contents of the component
             paintContent(target);
 
-            final ErrorMessage error = getErrorMessage();
-            if (error != null) {
-                error.paint(target);
-            }
         }
         target.endPaintable(this);
 
@@ -863,6 +859,11 @@ public abstract class AbstractComponent implements Component, MethodEventSource
             sharedState.setWidth("");
         }
 
+        ErrorMessage error = getErrorMessage();
+        if (null != error) {
+            sharedState.setErrorMessage(error.getFormattedHtmlMessage());
+        }
+
         return sharedState;
     }
 

src/com/vaadin/ui/TabSheet.java

@@ -411,11 +411,6 @@ public class TabSheet extends AbstractComponentContainer implements Focusable,
                         description);
             }
 
-            final ErrorMessage componentError = tab.getComponentError();
-            if (componentError != null) {
-                componentError.paint(target);
-            }
-
             final String styleName = tab.getStyleName();
             if (styleName != null && styleName.length() != 0) {
                 target.addAttribute(VTabsheet.TAB_STYLE_NAME, styleName);
@@ -1000,12 +995,11 @@ public class TabSheet extends AbstractComponentContainer implements Focusable,
         public void setComponentError(ErrorMessage componentError);
 
         /**
-         * Gets the curent error message shown for the tab.
+         * Gets the current error message shown for the tab.
          * 
-         * @see AbstractComponent#setComponentError(ErrorMessage)
+         * TODO currently not sent to the client
          * 
-         * @param error
-         *            message or null if none
+         * @see AbstractComponent#setComponentError(ErrorMessage)
          */
         public ErrorMessage getComponentError();