Fix declarative margin reading in AbstractOrderedLayout (#18229)

Change-Id: Ia212d83568e4f0c891ec1a248b6d8567c0cf0095

2 files changed, 93 insertions, 49 deletions

diff --git a/server/src/com/vaadin/ui/AbstractLayout.java b/server/src/com/vaadin/ui/AbstractLayout.java
index 9cdb0a326a..0770670680 100644
--- a/server/src/com/vaadin/ui/AbstractLayout.java
+++ b/server/src/com/vaadin/ui/AbstractLayout.java
@@ -16,7 +16,12 @@
 
 package com.vaadin.ui;
 
+import org.jsoup.nodes.Element;
+
 import com.vaadin.shared.ui.AbstractLayoutState;
+import com.vaadin.shared.ui.MarginInfo;
+import com.vaadin.ui.declarative.DesignAttributeHandler;
+import com.vaadin.ui.declarative.DesignContext;
 
 /**
  * An abstract class that defines default implementation for the {@link Layout}
@@ -33,4 +38,90 @@ public abstract class AbstractLayout extends AbstractComponentContainer
         return (AbstractLayoutState) super.getState();
     }
 
+    /**
+     * Reads margin attributes from a design into a MarginInfo object. This
+     * helper method should be called from the
+     * {@link #readDesign(Element, DesignContext) readDesign} method of layouts
+     * that implement {@link MarginHandler}.
+     * 
+     * @since 7.5
+     * 
+     * @param design
+     *            the design from which to read
+     * @param defMargin
+     *            the default margin state for edges that are not set in the
+     *            design
+     * @param context
+     *            the DesignContext instance used for parsing the design
+     * @return the margin info
+     */
+    protected MarginInfo readMargin(Element design, MarginInfo defMargin,
+            DesignContext context) {
+
+        if (design.hasAttr("margin")) {
+            boolean margin = DesignAttributeHandler.readAttribute("margin",
+                    design.attributes(), boolean.class);
+            return new MarginInfo(margin);
+        } else {
+            boolean left = DesignAttributeHandler.readAttribute("margin-left",
+                    design.attributes(), defMargin.hasLeft(), boolean.class);
+
+            boolean right = DesignAttributeHandler.readAttribute(
+                    "margin-right", design.attributes(), defMargin.hasRight(),
+                    boolean.class);
+
+            boolean top = DesignAttributeHandler.readAttribute("margin-top",
+                    design.attributes(), defMargin.hasTop(), boolean.class);
+
+            boolean bottom = DesignAttributeHandler.readAttribute(
+                    "margin-bottom", design.attributes(),
+                    defMargin.hasBottom(), boolean.class);
+
+            return new MarginInfo(top, right, bottom, left);
+        }
+    }
+
+    /**
+     * Writes margin attributes from a MarginInfo object to a design. This
+     * helper method should be called from the
+     * {@link #readDesign(Element, DesignContext) writeDesign} method of layouts
+     * that implement {@link MarginHandler}.
+     * 
+     * 
+     * @since 7.5
+     * 
+     * @param design
+     *            the design to write to
+     * @param margin
+     *            the margin state to write
+     * @param defMargin
+     *            the default margin state to compare against
+     * @param context
+     *            the DesignContext instance used for parsing the design
+     */
+    protected void writeMargin(Element design, MarginInfo margin,
+            MarginInfo defMargin, DesignContext context) {
+        if (margin.hasAll()) {
+            DesignAttributeHandler.writeAttribute("margin",
+                    design.attributes(), margin.hasAll(), defMargin.hasAll(),
+                    boolean.class);
+        } else {
+
+            DesignAttributeHandler.writeAttribute("margin-left",
+                    design.attributes(), margin.hasLeft(), defMargin.hasLeft(),
+                    boolean.class);
+
+            DesignAttributeHandler.writeAttribute("margin-right",
+                    design.attributes(), margin.hasRight(),
+                    defMargin.hasRight(), boolean.class);
+
+            DesignAttributeHandler.writeAttribute("margin-top",
+                    design.attributes(), margin.hasTop(), defMargin.hasTop(),
+                    boolean.class);
+
+            DesignAttributeHandler.writeAttribute("margin-bottom",
+                    design.attributes(), margin.hasBottom(),
+                    defMargin.hasBottom(), boolean.class);
+        }
+    }
 }
diff --git a/server/src/com/vaadin/ui/AbstractOrderedLayout.java b/server/src/com/vaadin/ui/AbstractOrderedLayout.java
index 0214ff4be1..afe4717212 100644
--- a/server/src/com/vaadin/ui/AbstractOrderedLayout.java
+++ b/server/src/com/vaadin/ui/AbstractOrderedLayout.java
@@ -478,30 +478,7 @@ public abstract class AbstractOrderedLayout extends AbstractLayout implements
         // process default attributes
         super.readDesign(design, designContext);
 
-        // handle margins
-        if (design.hasAttr("margin")) {
-            setMargin(DesignAttributeHandler.readAttribute("margin",
-                    design.attributes(), Boolean.class));
-        } else {
-            boolean marginLeft = DesignAttributeHandler.readAttribute(
-                    "margin-left", design.attributes(), getMargin().hasLeft(),
-                    Boolean.class);
-
-            boolean marginRight = DesignAttributeHandler.readAttribute(
-                    "margin-right", design.attributes(),
-                    getMargin().hasRight(), Boolean.class);
-
-            boolean marginTop = DesignAttributeHandler.readAttribute(
-                    "margin-top", design.attributes(), getMargin().hasTop(),
-                    Boolean.class);
-
-            boolean marginBottom = DesignAttributeHandler.readAttribute(
-                    "margin-bottom", design.attributes(), getMargin()
-                            .hasBottom(), Boolean.class);
-
-            setMargin(new MarginInfo(marginTop, marginBottom, marginLeft,
-                    marginRight));
-        }
+        setMargin(readMargin(design, getMargin(), designContext));
 
         // handle children
         for (Element childComponent : design.children()) {
@@ -557,31 +534,7 @@ public abstract class AbstractOrderedLayout extends AbstractLayout implements
         AbstractOrderedLayout def = (AbstractOrderedLayout) designContext
                 .getDefaultInstance(this);
 
-        // handle margin
-        MarginInfo marginInfo = getMargin();
-
-        if (marginInfo.hasAll()) {
-            DesignAttributeHandler.writeAttribute("margin",
-                    design.attributes(), marginInfo.hasAll(), def.getMargin()
-                            .hasAll(), Boolean.class);
-        } else {
-
-            DesignAttributeHandler.writeAttribute("margin-left", design
-                    .attributes(), marginInfo.hasLeft(), def.getMargin()
-                    .hasLeft(), Boolean.class);
-
-            DesignAttributeHandler.writeAttribute("margin-right", design
-                    .attributes(), marginInfo.hasRight(), def.getMargin()
-                    .hasRight(), Boolean.class);
-
-            DesignAttributeHandler.writeAttribute("margin-top", design
-                    .attributes(), marginInfo.hasTop(), def.getMargin()
-                    .hasTop(), Boolean.class);
-
-            DesignAttributeHandler.writeAttribute("margin-bottom", design
-                    .attributes(), marginInfo.hasBottom(), def.getMargin()
-                    .hasBottom(), Boolean.class);
-        }
+        writeMargin(design, getMargin(), def.getMargin(), designContext);
 
         // handle children
         if (!designContext.shouldWriteChildren(this, def)) {