From a14b4e87604dde45d1203df5840f469ac11ad7ee Mon Sep 17 00:00:00 2001
From: Jouni Koivuviita <jouni@jounikoivuviita.com>
Date: Sun, 15 Apr 2012 00:12:27 +0300
Subject: [PATCH] BoxLayout now working with LayoutManager (except for spacing
 size)

---
 GWT development mode for vaadin.launch        |  24 ++
 .../VAADIN/themes/tests-components/styles.css |   9 +
 .../terminal/gwt/DefaultWidgetSet.gwt.xml     |   4 +-
 .../terminal/gwt/client/LayoutManager.java    |   6 +-
 .../client/ui/AbstractBoxLayoutConnector.java | 345 ++++++++++++++----
 .../ui/HorizontalBoxLayoutConnector.java      |  10 +-
 .../terminal/gwt/client/ui/VBoxLayout.java    | 219 ++++++-----
 .../client/ui/VerticalBoxLayoutConnector.java |   6 +
 .../orderedlayout/BoxLayoutTest.java          |  23 +-
 9 files changed, 461 insertions(+), 185 deletions(-)
 create mode 100644 GWT development mode for vaadin.launch

diff --git a/GWT development mode for vaadin.launch b/GWT development mode for vaadin.launch
new file mode 100644
index 0000000000..5195348bf5
--- /dev/null
+++ b/GWT development mode for vaadin.launch	
@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<launchConfiguration type="org.eclipse.jdt.launching.localJavaApplication">
+<listAttribute key="org.eclipse.debug.core.MAPPED_RESOURCE_PATHS">
+<listEntry value="/vaadin"/>
+</listAttribute>
+<listAttribute key="org.eclipse.debug.core.MAPPED_RESOURCE_TYPES">
+<listEntry value="4"/>
+</listAttribute>
+<listAttribute key="org.eclipse.jdt.launching.CLASSPATH">
+<listEntry value="&lt;?xml version=&quot;1.0&quot; encoding=&quot;UTF-8&quot; standalone=&quot;no&quot;?&gt;&#10;&lt;runtimeClasspathEntry externalArchive=&quot;/Users/jouni/.ivy2/cache/com.google.gwt/gwt-dev/jars/gwt-dev-2.4.0.jar&quot; path=&quot;3&quot; type=&quot;2&quot;/&gt;&#10;"/>
+<listEntry value="&lt;?xml version=&quot;1.0&quot; encoding=&quot;UTF-8&quot; standalone=&quot;no&quot;?&gt;&#10;&lt;runtimeClasspathEntry externalArchive=&quot;/Users/jouni/.ivy2/cache/com.google.gwt/gwt-user/jars/gwt-user-2.4.0.jar&quot; path=&quot;3&quot; type=&quot;2&quot;/&gt;&#10;"/>
+<listEntry value="&lt;?xml version=&quot;1.0&quot; encoding=&quot;UTF-8&quot; standalone=&quot;no&quot;?&gt;&#10;&lt;runtimeClasspathEntry id=&quot;org.eclipse.jdt.launching.classpathentry.defaultClasspath&quot;&gt;&#10;&lt;memento exportedEntriesOnly=&quot;false&quot; project=&quot;vaadin&quot;/&gt;&#10;&lt;/runtimeClasspathEntry&gt;&#10;"/>
+<listEntry value="&lt;?xml version=&quot;1.0&quot; encoding=&quot;UTF-8&quot; standalone=&quot;no&quot;?&gt;&#10;&lt;runtimeClasspathEntry internalArchive=&quot;/vaadin/src&quot; path=&quot;3&quot; type=&quot;2&quot;/&gt;&#10;"/>
+<listEntry value="&lt;?xml version=&quot;1.0&quot; encoding=&quot;UTF-8&quot; standalone=&quot;no&quot;?&gt;&#10;&lt;runtimeClasspathEntry internalArchive=&quot;/vaadin/tests/testbench&quot; path=&quot;3&quot; type=&quot;2&quot;/&gt;&#10;"/>
+<listEntry value="&lt;?xml version=&quot;1.0&quot; encoding=&quot;UTF-8&quot; standalone=&quot;no&quot;?&gt;&#10;&lt;runtimeClasspathEntry internalArchive=&quot;/vaadin/tests/client-side&quot; path=&quot;3&quot; type=&quot;2&quot;/&gt;&#10;"/>
+<listEntry value="&lt;?xml version=&quot;1.0&quot; encoding=&quot;UTF-8&quot; standalone=&quot;no&quot;?&gt;&#10;&lt;runtimeClasspathEntry internalArchive=&quot;/vaadin/tests/server-side&quot; path=&quot;3&quot; type=&quot;2&quot;/&gt;&#10;"/>
+</listAttribute>
+<booleanAttribute key="org.eclipse.jdt.launching.DEFAULT_CLASSPATH" value="false"/>
+<stringAttribute key="org.eclipse.jdt.launching.MAIN_TYPE" value="com.google.gwt.dev.DevMode"/>
+<stringAttribute key="org.eclipse.jdt.launching.PROGRAM_ARGUMENTS" value="-noserver -war WebContent/VAADIN/widgetsets com.vaadin.terminal.gwt.DefaultWidgetSet -startupUrl http://localhost:8080/vaadin -bindAddress 0.0.0.0"/>
+<stringAttribute key="org.eclipse.jdt.launching.PROJECT_ATTR" value="vaadin"/>
+<stringAttribute key="org.eclipse.jdt.launching.VM_ARGUMENTS" value="-Xmx512M -XX:MaxPermSize=256M"/>
+<stringAttribute key="org.eclipse.jdt.launching.WORKING_DIRECTORY" value="/Users/jouni/Documents/Work/Dev/Vaadin/vaadin"/>
+</launchConfiguration>
diff --git a/WebContent/VAADIN/themes/tests-components/styles.css b/WebContent/VAADIN/themes/tests-components/styles.css
index 4a62696e1b..45a96b6d16 100644
--- a/WebContent/VAADIN/themes/tests-components/styles.css
+++ b/WebContent/VAADIN/themes/tests-components/styles.css
@@ -44,4 +44,13 @@
 
 .v-boxlayout.test .target {
 	outline: 2px dashed blue;
+}
+
+.fieldset {
+	padding: .5em 1em;
+	-webkit-box-shadow: inset 0 1px 2px rgba(0,0,0,.2);
+	-moz-box-shadow: inset 0 1px 2px rgba(0,0,0,.2);
+	box-shadow: inset 0 1px 2px rgba(0,0,0,.2);
+	border-radius: .5em;
+	background: rgba(0,0,0,.02);
 }
\ No newline at end of file
diff --git a/src/com/vaadin/terminal/gwt/DefaultWidgetSet.gwt.xml b/src/com/vaadin/terminal/gwt/DefaultWidgetSet.gwt.xml
index ae66d9d32d..518f91df7d 100644
--- a/src/com/vaadin/terminal/gwt/DefaultWidgetSet.gwt.xml
+++ b/src/com/vaadin/terminal/gwt/DefaultWidgetSet.gwt.xml
@@ -19,10 +19,10 @@
 	
 	<!-- TODO only for development -->
 	<replace-with class="com.vaadin.terminal.gwt.client.ui.VerticalBoxLayoutConnector">
-		<when-type-is class="com.vaadin.terminal.gwt.client.ui.VerticalLayoutConnector" />
+		<when-type-is class="com.vaadin.terminal.gwt.client.ui.orderedlayout.VerticalLayoutConnector" />
 	</replace-with>
 	<replace-with class="com.vaadin.terminal.gwt.client.ui.HorizontalBoxLayoutConnector">
-		<when-type-is class="com.vaadin.terminal.gwt.client.ui.HorizontalLayoutConnector" />
+		<when-type-is class="com.vaadin.terminal.gwt.client.ui.orderedlayout.HorizontalLayoutConnector" />
 	</replace-with>
 
 	<!-- Use own Scheduler implementation to be able to track if commands are 
diff --git a/src/com/vaadin/terminal/gwt/client/LayoutManager.java b/src/com/vaadin/terminal/gwt/client/LayoutManager.java
index 6039452ab4..22d5286714 100644
--- a/src/com/vaadin/terminal/gwt/client/LayoutManager.java
+++ b/src/com/vaadin/terminal/gwt/client/LayoutManager.java
@@ -131,7 +131,7 @@ public class LayoutManager {
             return;
         }
         measuredSize.removeDependent(owner.getConnectorId());
-        stopMeasuringIfUnecessary(element);
+        stopMeasuringIfUnnecessary(element);
     }
 
     public boolean isLayoutRunning() {
@@ -713,12 +713,12 @@ public class LayoutManager {
             listeners.remove(listener);
             if (listeners.isEmpty()) {
                 elementResizeListeners.remove(element);
-                stopMeasuringIfUnecessary(element);
+                stopMeasuringIfUnnecessary(element);
             }
         }
     }
 
-    private void stopMeasuringIfUnecessary(Element element) {
+    private void stopMeasuringIfUnnecessary(Element element) {
         if (!needsMeasure(element)) {
             measuredNonPaintableElements.remove(element);
             setMeasuredSize(element, null);
diff --git a/src/com/vaadin/terminal/gwt/client/ui/AbstractBoxLayoutConnector.java b/src/com/vaadin/terminal/gwt/client/ui/AbstractBoxLayoutConnector.java
index a879853fa5..7c4fe01cbb 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/AbstractBoxLayoutConnector.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/AbstractBoxLayoutConnector.java
@@ -1,8 +1,11 @@
 package com.vaadin.terminal.gwt.client.ui;
 
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 
 import com.google.gwt.core.client.GWT;
+import com.google.gwt.dom.client.Style.Unit;
 import com.google.gwt.user.client.Element;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.terminal.gwt.client.ApplicationConnection;
@@ -15,14 +18,15 @@ import com.vaadin.terminal.gwt.client.ValueMap;
 import com.vaadin.terminal.gwt.client.communication.RpcProxy;
 import com.vaadin.terminal.gwt.client.communication.StateChangeEvent;
 import com.vaadin.terminal.gwt.client.communication.StateChangeEvent.StateChangeHandler;
-import com.vaadin.terminal.gwt.client.ui.AbstractOrderedLayoutConnector.AbstractOrderedLayoutServerRPC;
-import com.vaadin.terminal.gwt.client.ui.AbstractOrderedLayoutConnector.AbstractOrderedLayoutState;
 import com.vaadin.terminal.gwt.client.ui.VBoxLayout.Slot;
 import com.vaadin.terminal.gwt.client.ui.layout.ElementResizeEvent;
 import com.vaadin.terminal.gwt.client.ui.layout.ElementResizeListener;
+import com.vaadin.terminal.gwt.client.ui.orderedlayout.AbstractOrderedLayoutServerRPC;
+import com.vaadin.terminal.gwt.client.ui.orderedlayout.AbstractOrderedLayoutState;
 
 public abstract class AbstractBoxLayoutConnector extends
-        AbstractLayoutConnector implements Paintable, PreLayoutListener {
+        AbstractLayoutConnector implements Paintable, PreLayoutListener,
+        PostLayoutListener {
 
     AbstractOrderedLayoutServerRPC rpc;
 
@@ -63,10 +67,44 @@ public abstract class AbstractBoxLayoutConnector extends
         return (VBoxLayout) super.getWidget();
     }
 
+    /**
+     * For bookkeeping. Used to determine if extra calculations are needed for
+     * horizontal layout.
+     */
+    private HashSet<ComponentConnector> hasVerticalAlignment = new HashSet<ComponentConnector>();
+
+    /**
+     * For bookkeeping. Used to determine if extra calculations are needed for
+     * horizontal layout.
+     */
+    private HashSet<ComponentConnector> hasRelativeHeight = new HashSet<ComponentConnector>();
+
+    /**
+     * For bookkeeping. Used to determine if extra calculations are needed for
+     * horizontal layout.
+     */
+    private HashSet<ComponentConnector> hasExpandRatio = new HashSet<ComponentConnector>();
+
+    /**
+     * For bookkeeping. Used in extra calculations for horizontal layout.
+     */
+    private HashSet<Element> needsMeasure = new HashSet<Element>();
+
+    /**
+     * For bookkeeping. Used in extra calculations for horizontal layout.
+     */
+    private HashMap<Element, Integer> childElementHeight = new HashMap<Element, Integer>();
+
+    /**
+     * For bookkeeping. Used in extra calculations for horizontal layout.
+     */
+    private HashMap<Element, Integer> childCaptionElementHeight = new HashMap<Element, Integer>();
+
     public void updateFromUIDL(UIDL uidl, ApplicationConnection client) {
         if (!isRealUpdate(uidl)) {
             return;
         }
+
         clickEventHandler.handleEventHandlerRegistration();
 
         VBoxLayout layout = getWidget();
@@ -81,8 +119,14 @@ public abstract class AbstractBoxLayoutConnector extends
             AlignmentInfo alignment;
             if (alignments.containsKey(pid)) {
                 alignment = new AlignmentInfo(alignments.getInt(pid));
+                if (alignment.isVerticalCenter() || alignment.isBottom()) {
+                    hasVerticalAlignment.add(child);
+                } else {
+                    hasVerticalAlignment.remove(child);
+                }
             } else {
                 alignment = AlignmentInfo.TOP_LEFT;
+                hasVerticalAlignment.remove(child);
             }
             slot.setAlignment(alignment);
 
@@ -90,8 +134,10 @@ public abstract class AbstractBoxLayoutConnector extends
             if (expandRatios.containsKey(pid)
                     && expandRatios.getRawNumber(pid) > 0) {
                 expandRatio = expandRatios.getRawNumber(pid);
+                hasExpandRatio.add(child);
             } else {
                 expandRatio = -1;
+                hasExpandRatio.remove(child);
             }
             slot.setExpandRatio(expandRatio);
 
@@ -101,35 +147,42 @@ public abstract class AbstractBoxLayoutConnector extends
         layout.setSpacing(getState().isSpacing());
         // TODO add/remove spacing size listener
 
-        // TODO
-        getWidget().recalculateExpands();
-        getWidget().recalculateUsedSpace();
+        // if (needsFixedHeight()) {
+        // setLayoutHeightListener(true);
+        // } else {
+        // setLayoutHeightListener(false);
+        // }
+
     }
 
-    public void updateCaption(ComponentConnector connector) {
-        Slot slot = getWidget().getSlot(connector.getWidget());
+    public void updateCaption(ComponentConnector child) {
+        Slot slot = getWidget().getSlot(child.getWidget());
 
-        String caption = connector.getState().getCaption();
-        String iconUrl = connector.getState().getIcon() != null ? connector
-                .getState().getIcon().getURL() : null;
-        List<String> styles = connector.getState().getStyles();
-        String error = connector.getState().getErrorMessage();
+        String caption = child.getState().getCaption();
+        String iconUrl = child.getState().getIcon() != null ? child.getState()
+                .getIcon().getURL() : null;
+        List<String> styles = child.getState().getStyles();
+        String error = child.getState().getErrorMessage();
         // TODO Description is handled from somewhere else?
 
         slot.setCaption(caption, iconUrl, styles, error);
 
-        slot.setRelativeWidth(connector.isRelativeWidth());
-        slot.setRelativeHeight(connector.isRelativeHeight());
+        slot.setRelativeWidth(child.isRelativeWidth());
+        slot.setRelativeHeight(child.isRelativeHeight());
 
-        // Should also check captionposition: && captionPosition==TOP ||
+        // TODO Should also check captionposition: && captionPosition==TOP ||
         // captionPosition==BOTTOM
-        if (connector.isRelativeHeight() && slot.hasCaption()) {
+        if (slot.hasCaption()) {
             getLayoutManager().addElementResizeListener(
                     slot.getCaptionElement(), slotCaptionResizeListener);
         } else {
             getLayoutManager().removeElementResizeListener(
                     slot.getCaptionElement(), slotCaptionResizeListener);
         }
+
+        if (!slot.hasCaption()) {
+            childCaptionElementHeight.remove(child.getWidget().getElement());
+        }
     }
 
     @Override
@@ -144,9 +197,6 @@ public abstract class AbstractBoxLayoutConnector extends
             Widget childWidget = child.getWidget();
             Slot slot = layout.getSlot(childWidget);
             if (slot.getParent() != layout) {
-                getLayoutManager().addElementResizeListener(
-                        slot.getWidget().getElement(),
-                        childComponentResizeListener);
                 child.addStateChangeHandler(childStateChangeHandler);
             }
             layout.addOrMoveSlot(slot, currentIndex++);
@@ -154,51 +204,62 @@ public abstract class AbstractBoxLayoutConnector extends
 
         for (ComponentConnector child : previousChildren) {
             if (child.getParent() != this) {
-                Slot removed = layout.removeSlot(child.getWidget());
+                Slot slot = layout.getSlot(child.getWidget());
+                hasVerticalAlignment.remove(child);
+                hasRelativeHeight.remove(child);
+                hasExpandRatio.remove(child);
+                needsMeasure.remove(child.getWidget().getElement());
+                childElementHeight.remove(child.getWidget().getElement());
+                childCaptionElementHeight
+                        .remove(child.getWidget().getElement());
                 getLayoutManager().removeElementResizeListener(
-                        removed.getWidget().getElement(),
+                        child.getWidget().getElement(),
                         childComponentResizeListener);
-                // child.removeStateChangeHandler(this);
+                if (slot.hasCaption()) {
+                    getLayoutManager()
+                            .removeElementResizeListener(
+                                    slot.getCaptionElement(),
+                                    slotCaptionResizeListener);
+                }
+                layout.removeSlot(child.getWidget());
             }
         }
 
-        updateLayoutHeight();
-
-        getWidget().recalculateUsedSpace();
     }
 
-    private boolean layoutHeightListenerAdded = false;
+    @Override
+    public void onStateChanged(StateChangeEvent stateChangeEvent) {
+        super.onStateChanged(stateChangeEvent);
 
-    private void updateLayoutHeight() {
-        if (!getWidget().vertical && isUndefinedHeight()) {
-            if (!layoutHeightListenerAdded) {
-                getLayoutManager().addElementResizeListener(
-                        getWidget().getElement(), layoutHeightResizeListener);
-                layoutHeightListenerAdded = true;
-            }
-        } else if (getWidget().vertical || !isUndefinedHeight()) {
-            getLayoutManager().removeElementResizeListener(
-                    getWidget().getElement(), layoutHeightResizeListener);
-            layoutHeightListenerAdded = false;
+        getWidget().setMargin(new VMarginInfo(getState().getMarginsBitmask()));
+        getWidget().setSpacing(getState().isSpacing());
+
+        if (needsFixedHeight()) {
+            setLayoutHeightListener(true);
+        } else {
+            setLayoutHeightListener(false);
         }
-        getWidget().recalculateLayoutHeight();
     }
 
     StateChangeHandler childStateChangeHandler = new StateChangeHandler() {
         public void onStateChanged(StateChangeEvent stateChangeEvent) {
             ComponentConnector child = (ComponentConnector) stateChangeEvent
                     .getConnector();
-            // TODO handle captions here as well, once 'updateCaption' is
-            // removed
 
             // We need to update the slot size if the component size is changed
             // to relative
             Slot slot = getWidget().getSlot(child.getWidget());
-
             slot.setRelativeWidth(child.isRelativeWidth());
             slot.setRelativeHeight(child.isRelativeHeight());
+            if (slot.hasCaption()) {
+                getWidget().updateCaptionOffset(slot.getCaptionElement());
+            }
 
-            if (child.isRelativeHeight() && slot.hasCaption()) {
+            // If the slot has caption, we need to listen for it's size changes
+            // in order to update the padding/margin offset for relative sized
+            // components
+            if ((child.isRelativeHeight() || needsFixedHeight())
+                    && slot.hasCaption()) {
                 getLayoutManager().addElementResizeListener(
                         slot.getCaptionElement(), slotCaptionResizeListener);
             } else {
@@ -206,70 +267,194 @@ public abstract class AbstractBoxLayoutConnector extends
                         slot.getCaptionElement(), slotCaptionResizeListener);
             }
 
-            // TODO should copy component styles to the slot element as well,
-            // with a prefix
+            if (child.isRelativeHeight()) {
+                hasRelativeHeight.add(child);
+                needsMeasure.remove(child.getWidget().getElement());
+                childElementHeight.remove(child.getWidget().getElement());
+            } else {
+                hasRelativeHeight.remove(child);
+                needsMeasure.add(child.getWidget().getElement());
+            }
 
-            if (!getWidget().vertical && isUndefinedHeight()) {
-                getWidget().getElement().getStyle().clearHeight();
-                getLayoutManager().setNeedsMeasure(
-                        AbstractBoxLayoutConnector.this);
-                getLayoutManager().layoutNow();
+            if (needsFixedHeight()) {
+                setLayoutHeightListener(true);
+                getLayoutManager().addElementResizeListener(
+                        child.getWidget().getElement(),
+                        childComponentResizeListener);
+            } else {
+                setLayoutHeightListener(false);
+                getLayoutManager().removeElementResizeListener(
+                        child.getWidget().getElement(),
+                        childComponentResizeListener);
             }
 
-            updateLayoutHeight();
         }
     };
 
-    @Override
-    public void onStateChanged(StateChangeEvent stateChangeEvent) {
-        super.onStateChanged(stateChangeEvent);
-        getWidget().setMargin(new VMarginInfo(getState().getMarginsBitmask()));
-        getWidget().setSpacing(getState().isSpacing());
-        updateLayoutHeight();
+    private boolean needsFixedHeight() {
+        if (!getWidget().vertical
+                && isUndefinedHeight()
+                && (hasRelativeHeight.size() > 0 || hasVerticalAlignment.size() > 0)) {
+            return true;
+        }
+        return false;
     }
 
-    @Override
-    public void onUnregister() {
-        getLayoutManager().removeElementResizeListener(
-                getWidget().getElement(), layoutHeightResizeListener);
-
-        for (int i = 0; i < getWidget().getWidgetCount(); i++) {
-            Slot slot = (Slot) getWidget().getWidget(i);
-
-            getLayoutManager().removeElementResizeListener(
-                    slot.getCaptionElement(), slotCaptionResizeListener);
+    private boolean needsExpand() {
+        for (ComponentConnector child : getChildren()) {
+            if (getWidget().getSlot(child.getWidget()).getExpandRatio() > -1) {
+                return true;
+            }
+        }
+        return false;
+    }
 
-            getLayoutManager()
-                    .removeElementResizeListener(slot.getWidget().getElement(),
-                            childComponentResizeListener);
+    public void preLayout() {
+        if (needsFixedHeight()) {
+            getWidget().clearHeight();
+            getLayoutManager().setNeedsMeasure(this);
         }
 
-        super.onUnregister();
+        // TODO currently not using LayoutManager properly
+        if (needsExpand()) {
+            getWidget().updateExpand();
+        } else {
+            getWidget().clearExpand();
+        }
     }
 
-    public void preLayout() {
-        if (!getWidget().vertical && isUndefinedHeight()) {
-            getWidget().getElement().getStyle().clearHeight();
+    public void postLayout() {
+        if (needsFixedHeight()) {
+            // Re-measure all elements that are available
+            for (Element el : needsMeasure) {
+                childElementHeight.put(el, getLayoutManager()
+                        .getOuterHeight(el));
+
+                // Element captionElement = el.getParentElement()
+                // .getFirstChildElement().cast();
+                // if (captionElement.getClassName().contains("v-caption")) {
+                // childCaptionElementHeight.put(el, getLayoutManager()
+                // .getOuterHeight(captionElement));
+                // }
+            }
+
+            String h = getWidget().getElement().getStyle().getHeight();
+            if (h == null || h.equals("")) {
+                int height = getLayoutManager().getOuterHeight(
+                        getWidget().getElement())
+                        - getLayoutManager().getMarginHeight(
+                                getWidget().getElement());
+                // int height = getMaxHeight();
+                getWidget().getElement().getStyle().setHeight(height, Unit.PX);
+            }
         }
     }
 
-    ElementResizeListener layoutHeightResizeListener = new ElementResizeListener() {
+    private ElementResizeListener layoutResizeListener = new ElementResizeListener() {
         public void onElementResize(ElementResizeEvent e) {
-            getWidget().recalculateLayoutHeight();
+            updateLayoutHeight();
         }
     };
 
-    ElementResizeListener slotCaptionResizeListener = new ElementResizeListener() {
+    private ElementResizeListener slotCaptionResizeListener = new ElementResizeListener() {
         public void onElementResize(ElementResizeEvent e) {
-            getWidget().updateSize((Element) e.getElement().cast());
+            Element captionElement = (Element) e.getElement().cast();
+
+            getWidget().updateCaptionOffset(captionElement);
+
+            // TODO take caption position into account
+            Element widgetElement = captionElement.getParentElement()
+                    .getLastChild().cast();
+
+            if (captionElement == widgetElement) {
+                // Caption element already detached
+                return;
+            }
+
+            String widgetHeight = widgetElement.getStyle().getHeight();
+            if (widgetHeight == null || !widgetHeight.endsWith("%")) {
+                int h = getLayoutManager().getOuterHeight(captionElement)
+                        - getLayoutManager().getMarginHeight(captionElement);
+                System.out.println("Adding caption height: " + h);
+                childCaptionElementHeight.put(widgetElement, h);
+                updateLayoutHeight();
+            }
         }
     };
 
     private ElementResizeListener childComponentResizeListener = new ElementResizeListener() {
         public void onElementResize(ElementResizeEvent e) {
+            int h = getLayoutManager().getOuterHeight(e.getElement());
+            childElementHeight.put((Element) e.getElement().cast(), h);
             updateLayoutHeight();
-            getWidget().recalculateUsedSpace();
-            getWidget().recalculateLayoutHeight();
         }
     };
+
+    private void updateLayoutHeight() {
+        if (needsFixedHeight() && childElementHeight.size() > 0) {
+            System.out.println("All sizes: "
+                    + childElementHeight.values().toString()
+                    + " - Caption sizes: "
+                    + childCaptionElementHeight.values().toString());
+            int h = getMaxHeight();
+            System.out.println("Max height: " + h);
+            h += getLayoutManager().getBorderHeight(getWidget().getElement())
+                    + getLayoutManager().getPaddingHeight(
+                            getWidget().getElement());
+            getWidget().getElement().getStyle().setHeight(h, Unit.PX);
+            getLayoutManager().setNeedsMeasure(this);
+        }
+    }
+
+    private int getMaxHeight() {
+        int h = 0;
+        for (Element el : childElementHeight.keySet()) {
+            int height = childElementHeight.get(el);
+            if (childCaptionElementHeight.containsKey(el)) {
+                height += childCaptionElementHeight.get(el);
+            }
+            if (height > h) {
+                h = height;
+            }
+        }
+        return h;
+    }
+
+    @Override
+    public void onUnregister() {
+        // Cleanup all ElementResizeListeners
+
+        getLayoutManager().removeElementResizeListener(
+                getWidget().getElement(), layoutResizeListener);
+
+        for (int i = 0; i < getWidget().getWidgetCount(); i++) {
+            // TODO unsafe
+            Slot slot = (Slot) getWidget().getWidget(i);
+
+            if (slot.hasCaption()) {
+                getLayoutManager().removeElementResizeListener(
+                        slot.getCaptionElement(), slotCaptionResizeListener);
+            }
+
+            getLayoutManager()
+                    .removeElementResizeListener(slot.getWidget().getElement(),
+                            childComponentResizeListener);
+
+        }
+
+        super.onUnregister();
+    }
+
+    private void setLayoutHeightListener(boolean add) {
+        if (add) {
+            getLayoutManager().addElementResizeListener(
+                    getWidget().getElement(), layoutResizeListener);
+        } else {
+            getLayoutManager().removeElementResizeListener(
+                    getWidget().getElement(), layoutResizeListener);
+            childElementHeight.clear();
+            childCaptionElementHeight.clear();
+        }
+    }
+
 }
diff --git a/src/com/vaadin/terminal/gwt/client/ui/HorizontalBoxLayoutConnector.java b/src/com/vaadin/terminal/gwt/client/ui/HorizontalBoxLayoutConnector.java
index 6a4d80a802..bdd856537b 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/HorizontalBoxLayoutConnector.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/HorizontalBoxLayoutConnector.java
@@ -8,10 +8,16 @@ import com.vaadin.ui.HorizontalLayout;
 @Component(value = HorizontalLayout.class, loadStyle = LoadStyle.EAGER)
 public class HorizontalBoxLayoutConnector extends AbstractBoxLayoutConnector {
 
+    @Override
+    public void init() {
+        super.init();
+        getWidget().setVertical(false);
+    }
+
     @Override
     public void updateFromUIDL(UIDL uidl, ApplicationConnection client) {
-        // TODO fix when Vaadin style name handling is improved so that it won't
-        // override extra client side style names
+        // TODO remove when Vaadin style name handling is improved so that it
+        // won't override extra client side style names
         getWidget().setVertical(false);
         super.updateFromUIDL(uidl, client);
         getWidget().setVertical(false);
diff --git a/src/com/vaadin/terminal/gwt/client/ui/VBoxLayout.java b/src/com/vaadin/terminal/gwt/client/ui/VBoxLayout.java
index 71b7e2faad..54f8cf2031 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/VBoxLayout.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VBoxLayout.java
@@ -4,10 +4,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import com.google.gwt.core.client.Scheduler;
-import com.google.gwt.core.client.Scheduler.ScheduledCommand;
-import com.google.gwt.dom.client.DivElement;
-import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.Node;
 import com.google.gwt.dom.client.Style;
 import com.google.gwt.dom.client.Style.Unit;
@@ -18,11 +14,14 @@ import com.google.gwt.user.client.Element;
 import com.google.gwt.user.client.Event;
 import com.google.gwt.user.client.ui.FlowPanel;
 import com.google.gwt.user.client.ui.SimplePanel;
+import com.google.gwt.user.client.ui.UIObject;
 import com.google.gwt.user.client.ui.Widget;
 import com.vaadin.terminal.gwt.client.LayoutManager;
 
 public class VBoxLayout extends FlowPanel {
 
+    private static final String ALIGN_CLASS_PREFIX = "v-align-";
+
     protected boolean spacing = false;
 
     protected boolean vertical = true;
@@ -107,14 +106,12 @@ public class VBoxLayout extends FlowPanel {
 
     protected class Slot extends SimplePanel {
 
-        private static final String ALIGN_CLASS_PREFIX = "v-align-";
-
-        private DivElement spacer;
+        private Element spacer;
 
         private Element captionWrap;
         private Element caption;
         private Element captionText;
-        private Element icon;
+        private Icon icon;
         private Element errorIcon;
 
         private CaptionPosition captionPosition = CaptionPosition.TOP;
@@ -166,7 +163,7 @@ public class VBoxLayout extends FlowPanel {
 
         public void setSpacing(boolean spacing) {
             if (spacing && spacer == null) {
-                spacer = Document.get().createDivElement();
+                spacer = DOM.createDiv();
                 spacer.addClassName("v-spacing");
                 getElement().getParentElement().insertBefore(spacer,
                         getElement());
@@ -176,18 +173,26 @@ public class VBoxLayout extends FlowPanel {
             }
         }
 
-        // TODO use ElementResizeListener for this
+        public Element getSpacingElement() {
+            return spacer;
+        }
+
         protected int getSpacingSize(boolean vertical) {
+            // No spacer attached
             if (spacer == null) {
                 return 0;
             }
-            // TODO place for optimization (in expense of theme flexibility):
-            // only measure one of the elements and cache the value
-            if (vertical) {
-                return spacer.getOffsetHeight();
-            } else {
-                return spacer.getOffsetWidth();
-            }
+
+            // if (layoutManager != null) {
+            // return vertical ? layoutManager.getOuterHeight(spacer)
+            // : layoutManager.getOuterWidth(spacer);
+            // } else {
+            // TODO place for optimization (in expense of theme
+            // flexibility): only measure one of the elements and cache the
+            // value
+            return vertical ? spacer.getOffsetHeight() : spacer
+                    .getOffsetWidth();
+            // }
         }
 
         public void setCaptionPosition(CaptionPosition captionPosition) {
@@ -245,13 +250,15 @@ public class VBoxLayout extends FlowPanel {
             // Icon
             if (iconUrl != null) {
                 if (icon == null) {
-                    icon = DOM.createImg();
-                    icon.setClassName("v-icon");
-                    caption.insertFirst(icon);
+                    icon = new Icon();
+                    // icon = DOM.createImg();
+                    // icon.setClassName("v-icon");
+                    caption.insertFirst(icon.getElement());
                 }
-                icon.setAttribute("src", iconUrl);
+                // icon.setAttribute("src", iconUrl);
+                icon.setUri(iconUrl);
             } else if (icon != null) {
-                icon.removeFromParent();
+                icon.getElement().removeFromParent();
                 icon = null;
             }
 
@@ -323,11 +330,11 @@ public class VBoxLayout extends FlowPanel {
         public void onBrowserEvent(Event event) {
             super.onBrowserEvent(event);
             if (DOM.eventGetType(event) == Event.ONLOAD
-                    && icon == DOM.eventGetTarget(event)) {
+                    && icon.getElement() == DOM.eventGetTarget(event)) {
                 if (layoutManager != null) {
                     layoutManager.layoutLater();
                 } else {
-                    updateSize(caption);
+                    updateCaptionOffset(caption);
                 }
             }
         }
@@ -342,6 +349,32 @@ public class VBoxLayout extends FlowPanel {
 
     }
 
+    protected class Icon extends UIObject {
+        public static final String CLASSNAME = "v-icon";
+        private String myUrl;
+
+        public Icon() {
+            setElement(DOM.createImg());
+            DOM.setElementProperty(getElement(), "alt", "");
+            setStyleName(CLASSNAME);
+        }
+
+        public void setUri(String url) {
+            if (!url.equals(myUrl)) {
+                /*
+                 * Start sinking onload events, widgets responsibility to react.
+                 * We must do this BEFORE we set src as IE fires the event
+                 * immediately if the image is found in cache (#2592).
+                 */
+                sinkEvents(Event.ONLOAD);
+
+                DOM.setElementProperty(getElement(), "src", url);
+                myUrl = url;
+            }
+        }
+
+    }
+
     void setLayoutManager(LayoutManager manager) {
         layoutManager = manager;
     }
@@ -349,11 +382,11 @@ public class VBoxLayout extends FlowPanel {
     private static final RegExp captionPositionRegexp = RegExp
             .compile("v-caption-on-(\\S+)");
 
-    public void updateSize(Element caption) {
+    void updateCaptionOffset(Element caption) {
 
         Element captionWrap = caption.getParentElement().cast();
-        Style captionWrapStyle = captionWrap.getStyle();
 
+        Style captionWrapStyle = captionWrap.getStyle();
         captionWrapStyle.clearPaddingTop();
         captionWrapStyle.clearPaddingRight();
         captionWrapStyle.clearPaddingBottom();
@@ -432,7 +465,7 @@ public class VBoxLayout extends FlowPanel {
         }
     }
 
-    public void recalculateExpands() {
+    private void recalculateExpands() {
         double total = 0;
         for (Slot slot : widgetToSlot.values()) {
             if (slot.getExpandRatio() > -1) {
@@ -459,85 +492,79 @@ public class VBoxLayout extends FlowPanel {
 
     private Element expandWrapper;
 
-    private boolean recalculateUsedSpaceScheduled = false;
-
-    public void recalculateUsedSpace() {
-        if (!recalculateUsedSpaceScheduled) {
-            Scheduler.get().scheduleDeferred(updateExpandSlotSize);
-            recalculateUsedSpaceScheduled = true;
+    void clearExpand() {
+        if (expandWrapper != null) {
+            for (; expandWrapper.getChildCount() > 0;) {
+                Element el = expandWrapper.getChild(0).cast();
+                getElement().appendChild(el);
+                if (vertical) {
+                    el.getStyle().clearHeight();
+                    el.getStyle().clearMarginTop();
+                } else {
+                    el.getStyle().clearWidth();
+                    el.getStyle().clearMarginLeft();
+                }
+            }
+            expandWrapper.removeFromParent();
+            expandWrapper = null;
         }
     }
 
-    private ScheduledCommand updateExpandSlotSize = new ScheduledCommand() {
-        public void execute() {
-            boolean isExpanding = false;
-            for (Widget w : getChildren()) {
-                if (((Slot) w).getExpandRatio() > -1) {
-                    isExpanding = true;
+    public void updateExpand() {
+        boolean isExpanding = false;
+        for (Widget slot : getChildren()) {
+            if (((Slot) slot).getExpandRatio() > -1) {
+                isExpanding = true;
+            } else {
+                if (vertical) {
+                    slot.getElement().getStyle().clearHeight();
                 } else {
-                    if (vertical) {
-                        w.getElement().getStyle().clearHeight();
-                    } else {
-                        w.getElement().getStyle().clearWidth();
-                    }
+                    slot.getElement().getStyle().clearWidth();
                 }
-                w.getElement().getStyle().clearMarginLeft();
-                w.getElement().getStyle().clearMarginTop();
             }
-            if (isExpanding) {
-                if (expandWrapper == null) {
-                    expandWrapper = DOM.createDiv();
-                    expandWrapper.setClassName("v-expand");
-                    for (; getElement().getChildCount() > 0;) {
-                        Node el = getElement().getChild(0);
-                        expandWrapper.appendChild(el);
-                    }
-                    getElement().appendChild(expandWrapper);
+            slot.getElement().getStyle().clearMarginLeft();
+            slot.getElement().getStyle().clearMarginTop();
+        }
+        if (isExpanding) {
+            if (expandWrapper == null) {
+                expandWrapper = DOM.createDiv();
+                expandWrapper.setClassName("v-expand");
+                for (; getElement().getChildCount() > 0;) {
+                    Node el = getElement().getChild(0);
+                    expandWrapper.appendChild(el);
                 }
+                getElement().appendChild(expandWrapper);
+            }
 
-                int totalSize = 0;
-                for (Widget w : getChildren()) {
-                    Slot slot = (Slot) w;
-                    if (slot.getExpandRatio() == -1) {
-                        totalSize += vertical ? slot.getOffsetHeight() : slot
-                                .getOffsetWidth();
-                    }
-                    // TODO fails in Opera, always returns 0
-                    totalSize += slot.getSpacingSize(vertical);
+            int totalSize = 0;
+            for (Widget w : getChildren()) {
+                Slot slot = (Slot) w;
+                if (slot.getExpandRatio() == -1) {
+                    // TODO use layoutManager?
+                    totalSize += vertical ? slot.getOffsetHeight() : slot
+                            .getOffsetWidth();
                 }
+                // TODO fails in Opera, always returns 0
+                totalSize += slot.getSpacingSize(vertical);
+            }
 
-                // When we set the margin to the first child, we don't need
-                // overflow:hidden in the layout root element, since the wrapper
-                // would otherwise be placed outside of the layout root element
-                // and block events on elements below it.
-                if (vertical) {
-                    expandWrapper.getStyle().setPaddingTop(totalSize, Unit.PX);
-                    expandWrapper.getFirstChildElement().getStyle()
-                            .setMarginTop(-totalSize, Unit.PX);
-                } else {
-                    expandWrapper.getStyle().setPaddingLeft(totalSize, Unit.PX);
-                    expandWrapper.getFirstChildElement().getStyle()
-                            .setMarginLeft(-totalSize, Unit.PX);
-                }
-                recalculateExpands();
-
-            } else if (expandWrapper != null) {
-                for (; expandWrapper.getChildCount() > 0;) {
-                    Node el = expandWrapper.getChild(0);
-                    getElement().appendChild(el);
-                    if (vertical) {
-                        ((Element) el.cast()).getStyle().clearHeight();
-                    } else {
-                        ((Element) el.cast()).getStyle().clearWidth();
-                    }
-                }
-                expandWrapper.removeFromParent();
-                expandWrapper = null;
+            // When we set the margin to the first child, we don't need
+            // overflow:hidden in the layout root element, since the wrapper
+            // would otherwise be placed outside of the layout root element
+            // and block events on elements below it.
+            if (vertical) {
+                expandWrapper.getStyle().setPaddingTop(totalSize, Unit.PX);
+                expandWrapper.getFirstChildElement().getStyle()
+                        .setMarginTop(-totalSize, Unit.PX);
+            } else {
+                expandWrapper.getStyle().setPaddingLeft(totalSize, Unit.PX);
+                expandWrapper.getFirstChildElement().getStyle()
+                        .setMarginLeft(-totalSize, Unit.PX);
             }
+            recalculateExpands();
 
-            recalculateUsedSpaceScheduled = false;
         }
-    };
+    }
 
     public void recalculateLayoutHeight() {
         // Only needed if a horizontal layout is undefined high, and contains
@@ -556,7 +583,7 @@ public class VBoxLayout extends FlowPanel {
                 hasRelativeHeightChildren = true;
             }
             AlignmentInfo a = ((Slot) slot).getAlignment();
-            if (a.isVerticalCenter() || a.isBottom()) {
+            if (a != null && (a.isVerticalCenter() || a.isBottom())) {
                 hasVAlign = true;
             }
         }
@@ -575,6 +602,10 @@ public class VBoxLayout extends FlowPanel {
 
     }
 
+    void clearHeight() {
+        getElement().getStyle().clearHeight();
+    }
+
     @Override
     public void setHeight(String height) {
         super.setHeight(height);
diff --git a/src/com/vaadin/terminal/gwt/client/ui/VerticalBoxLayoutConnector.java b/src/com/vaadin/terminal/gwt/client/ui/VerticalBoxLayoutConnector.java
index 0e9e04e381..80100fc0c7 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/VerticalBoxLayoutConnector.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VerticalBoxLayoutConnector.java
@@ -8,6 +8,12 @@ import com.vaadin.ui.VerticalLayout;
 @Component(value = VerticalLayout.class, loadStyle = LoadStyle.EAGER)
 public class VerticalBoxLayoutConnector extends AbstractBoxLayoutConnector {
 
+    @Override
+    public void init() {
+        super.init();
+        getWidget().setVertical(true);
+    }
+
     @Override
     public void updateFromUIDL(UIDL uidl, ApplicationConnection client) {
         // TODO fix when Vaadin style name handling is improved so that it won't
diff --git a/tests/testbench/com/vaadin/tests/components/orderedlayout/BoxLayoutTest.java b/tests/testbench/com/vaadin/tests/components/orderedlayout/BoxLayoutTest.java
index b11ef4f98d..ca67ed6435 100644
--- a/tests/testbench/com/vaadin/tests/components/orderedlayout/BoxLayoutTest.java
+++ b/tests/testbench/com/vaadin/tests/components/orderedlayout/BoxLayoutTest.java
@@ -18,8 +18,8 @@ import com.vaadin.ui.Alignment;
 import com.vaadin.ui.Button;
 import com.vaadin.ui.Button.ClickEvent;
 import com.vaadin.ui.CheckBox;
-import com.vaadin.ui.ComboBox;
 import com.vaadin.ui.Component;
+import com.vaadin.ui.GridLayout;
 import com.vaadin.ui.HorizontalLayout;
 import com.vaadin.ui.Label;
 import com.vaadin.ui.Label.ContentMode;
@@ -96,8 +96,24 @@ public class BoxLayoutTest extends AbstractTestRoot {
         Button addComponent = new Button("Add Component",
                 new Button.ClickListener() {
                     public void buttonClick(ClickEvent event) {
-                        l.addComponent(new ComboBox("ComboBox "
-                                + (l.getComponentCount() + 1)));
+                        GridLayout grid = new GridLayout(2, 2);
+                        Button grow = new Button("Grow Me",
+                                new Button.ClickListener() {
+                                    public void buttonClick(ClickEvent event) {
+                                        if (event.getButton().getWidth() == -1) {
+                                            event.getButton().setHeight("50px");
+                                            event.getButton().setWidth("200px");
+                                        } else {
+                                            event.getButton()
+                                                    .setSizeUndefined();
+                                        }
+                                    }
+                                });
+                        grid.addComponent(new Label("Grid cell 1"));
+                        grid.addComponent(new Label("Grid cell 2"));
+                        grid.addComponent(grow);
+                        grid.addComponent(new Label("Grid cell 4"));
+                        l.addComponent(grid);
                     }
                 });
         header.addComponent(addComponent);
@@ -170,7 +186,6 @@ public class BoxLayoutTest extends AbstractTestRoot {
         });
         spacing.setImmediate(true);
         layout.addComponent(spacing);
-        layout.setComponentAlignment(spacing, Alignment.MIDDLE_LEFT);
 
         // Cell controls
         HorizontalLayout cell = new HorizontalLayout();
-- 
2.39.5