Fixes #12564 (HorizontalLayout breaks w/ alignment & error indicator)

Removed some obsolete (hopefully!?) code doing some odd things with caption height calculation  and some refactoring to make that part of code slightly more readable. 

Change-Id: I960e4a9eba0388281868f18a182c8788cedf08f9

2 files changed, 111 insertions, 17 deletions

diff --git a/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java b/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java
index e0dc0d51df..ec4307e50b 100644
--- a/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java
+++ b/client/src/com/vaadin/client/ui/orderedlayout/AbstractOrderedLayoutConnector.java
@@ -19,6 +19,7 @@ import java.util.List;
 
 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.client.ApplicationConnection;
 import com.vaadin.client.ComponentConnector;
 import com.vaadin.client.ConnectorHierarchyChangeEvent;
@@ -611,13 +612,14 @@ public abstract class AbstractOrderedLayoutConnector extends
         LayoutManager layoutManager = getLayoutManager();
 
         for (ComponentConnector child : getChildComponents()) {
-            Slot slot = getWidget().getSlot(child.getWidget());
+            Widget childWidget = child.getWidget();
+            Slot slot = getWidget().getSlot(childWidget);
             Element captionElement = slot.getCaptionElement();
-            CaptionPosition pos = slot.getCaptionPosition();
+            CaptionPosition captionPosition = slot.getCaptionPosition();
 
-            Element childElement = child.getWidget().getElement();
-            int h = layoutManager.getOuterHeight(childElement);
-            if (h == -1) {
+            int pixelHeight = layoutManager.getOuterHeight(childWidget
+                    .getElement());
+            if (pixelHeight == -1) {
                 // Height has not yet been measured -> postpone actions that
                 // depend on the max height
                 return -1;
@@ -625,14 +627,10 @@ public abstract class AbstractOrderedLayoutConnector extends
 
             boolean hasRelativeHeight = slot.hasRelativeHeight();
 
+            boolean captionSizeShouldBeAddedtoComponentHeight = captionPosition == CaptionPosition.TOP
+                    || captionPosition == CaptionPosition.BOTTOM;
             boolean includeCaptionHeight = captionElement != null
-                    && (pos == CaptionPosition.TOP || pos == CaptionPosition.BOTTOM);
-            if (!hasRelativeHeight && !includeCaptionHeight
-                    && captionElement != null) {
-                String sHeight = childElement.getStyle().getHeight();
-                includeCaptionHeight = (sHeight == null || !sHeight
-                        .endsWith("%"));
-            }
+                    && captionSizeShouldBeAddedtoComponentHeight;
 
             if (includeCaptionHeight) {
                 int captionHeight = layoutManager
@@ -643,16 +641,16 @@ public abstract class AbstractOrderedLayoutConnector extends
                     // depend on the max height
                     return -1;
                 }
-                h += captionHeight;
+                pixelHeight += captionHeight;
             }
 
             if (!hasRelativeHeight) {
-                if (h > highestNonRelative) {
-                    highestNonRelative = h;
+                if (pixelHeight > highestNonRelative) {
+                    highestNonRelative = pixelHeight;
                 }
             } else {
-                if (h > highestRelative) {
-                    highestRelative = h;
+                if (pixelHeight > highestRelative) {
+                    highestRelative = pixelHeight;
                 }
             }
         }
diff --git a/uitest/src/com/vaadin/tests/components/orderedlayout/HorizontalLayoutFullsizeContentWithErrorMsg.java b/uitest/src/com/vaadin/tests/components/orderedlayout/HorizontalLayoutFullsizeContentWithErrorMsg.java
new file mode 100644
index 0000000000..25675e07c5
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/orderedlayout/HorizontalLayoutFullsizeContentWithErrorMsg.java
@@ -0,0 +1,96 @@
+package com.vaadin.tests.components.orderedlayout;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.Point;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.server.UserError;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import com.vaadin.ui.Alignment;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Button.ClickListener;
+import com.vaadin.ui.HorizontalLayout;
+import com.vaadin.ui.TextField;
+
+public class HorizontalLayoutFullsizeContentWithErrorMsg extends AbstractTestUI {
+
+    private static final String FIELD_ID = "f";
+    private static final String BUTTON_ID = "b";
+    private TextField tf;
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 12564;
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#setup(com.vaadin.server.
+     * VaadinRequest)
+     */
+    @Override
+    protected void setup(VaadinRequest request) {
+        HorizontalLayout hl = new HorizontalLayout();
+        hl.setWidth("500px");
+
+        tf = new TextField();
+        tf.setId(FIELD_ID);
+        tf.setWidth("100%");
+        hl.addComponent(tf);
+        hl.setExpandRatio(tf, 1);
+        hl.setComponentAlignment(tf, Alignment.MIDDLE_CENTER);
+
+        Button toggleError = new Button("Toggle error");
+        toggleError.setId(BUTTON_ID);
+        toggleError.addClickListener(new ClickListener() {
+            @Override
+            public void buttonClick(ClickEvent event) {
+                tf.setComponentError(tf.getComponentError() == null ? new UserError(
+                        "foo") : null);
+            }
+        });
+        hl.addComponent(toggleError);
+
+        addComponent(hl);
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription()
+     */
+    @Override
+    protected String getTestDescription() {
+        return "TextField should remain at same level vertically, horizontally width should adjust to fit error indicator.";
+    }
+
+    public static class TbTest extends MultiBrowserTest {
+
+        @Test
+        public void test() {
+            openTestURL();
+            WebElement element = getDriver().findElement(By.id(FIELD_ID));
+            Point location = element.getLocation();
+
+            WebElement errorToggleButton = getDriver().findElement(
+                    By.id(BUTTON_ID));
+
+            errorToggleButton.click();
+
+            Assert.assertEquals(location, element.getLocation());
+
+            errorToggleButton.click();
+
+            Assert.assertEquals(location, element.getLocation());
+
+        }
+
+    }
+
+}