Take margin/border/padding into account when measuring ComboBox in IE (#17002)

Change-Id: I8fcf33b78ff239529b794d41088fd9d8aba5c518

6 files changed, 233 insertions, 10 deletions

diff --git a/client/src/com/vaadin/client/ui/VFilterSelect.java b/client/src/com/vaadin/client/ui/VFilterSelect.java
index 8d9e30ac6e..22eef57901 100644
--- a/client/src/com/vaadin/client/ui/VFilterSelect.java
+++ b/client/src/com/vaadin/client/ui/VFilterSelect.java
@@ -599,7 +599,8 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
                     + "]");
 
             Element menuFirstChild = menu.getElement().getFirstChildElement();
-            final int naturalMenuWidth = menuFirstChild.getOffsetWidth();
+            final int naturalMenuWidth = WidgetUtil
+                    .getRequiredWidth(menuFirstChild);
 
             if (popupOuterPadding == -1) {
                 popupOuterPadding = WidgetUtil
@@ -612,12 +613,18 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
             }
 
             if (BrowserInfo.get().isIE()) {
+                // Must take margin,border,padding manually into account for
+                // menu element as we measure the element child and set width to
+                // the element parent
+                int naturalMenuOuterWidth = naturalMenuWidth
+                        + getMarginBorderPaddingWidth(menu.getElement());
+
                 /*
                  * IE requires us to specify the width for the container
                  * element. Otherwise it will be 100% wide
                  */
-                int rootWidth = Math.max(desiredWidth, naturalMenuWidth)
-                        - popupOuterPadding;
+                int rootWidth = Math.max(desiredWidth - popupOuterPadding,
+                        naturalMenuOuterWidth);
                 getContainerElement().getStyle().setWidth(rootWidth, Unit.PX);
             }
 
@@ -1283,6 +1290,16 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
         sinkEvents(Event.ONPASTE);
     }
 
+    private static int getMarginBorderPaddingWidth(Element element) {
+        final ComputedStyle s = new ComputedStyle(element);
+        int[] margin = s.getMargin();
+        int[] border = s.getBorder();
+        int[] padding = s.getPadding();
+        return margin[1] + margin[3] + border[1] + border[3] + padding[1]
+                + padding[3];
+
+    }
+
     /*
      * (non-Javadoc)
      * 
diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboboxPopupScrolling.java b/uitest/src/com/vaadin/tests/components/combobox/ComboboxPopupScrolling.java
new file mode 100644
index 0000000000..9f1c4b9e03
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/combobox/ComboboxPopupScrolling.java
@@ -0,0 +1,40 @@
+package com.vaadin.tests.components.combobox;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.ComboBox;
+import com.vaadin.ui.HorizontalLayout;
+
+@Theme("valo")
+public class ComboboxPopupScrolling extends AbstractTestUIWithLog {
+    @Override
+    protected void setup(VaadinRequest request) {
+        ComboBox combobox = new ComboBox("100px wide combobox");
+        combobox.setWidth("100px");
+        combobox.addItem("AMERICAN SAMOA");
+        combobox.addItem("ANTIGUA AND BARBUDA");
+
+        ComboBox combobox2 = new ComboBox("250px wide combobox");
+        combobox2.setWidth("250px");
+        combobox2.addItem("AMERICAN SAMOA");
+        combobox2.addItem("ANTIGUA AND BARBUDA");
+
+        ComboBox combobox3 = new ComboBox("Undefined wide combobox");
+        combobox3.setWidth(null);
+        combobox3.addItem("AMERICAN SAMOA");
+        combobox3.addItem("ANTIGUA AND BARBUDA");
+
+        ComboBox combobox4 = new ComboBox("Another 100px wide combobox");
+        combobox4.setWidth("100px");
+        for (int i = 0; i < 10; i++) {
+            combobox4.addItem("AMERICAN SAMOA " + i);
+            combobox4.addItem("ANTIGUA AND BARBUDA " + i);
+        }
+
+        HorizontalLayout hl = new HorizontalLayout(combobox, combobox2,
+                combobox3, combobox4);
+        addComponent(hl);
+    }
+
+}
\ No newline at end of file
diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java b/uitest/src/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java
new file mode 100644
index 0000000000..ec5bc088da
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/combobox/ComboboxPopupScrollingTest.java
@@ -0,0 +1,60 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.combobox;
+
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class ComboboxPopupScrollingTest extends MultiBrowserTest {
+
+    @Test
+    public void testNoScrollbarsValo() {
+        testNoScrollbars("valo");
+    }
+
+    @Test
+    public void testNoScrollbarsChameleon() {
+        testNoScrollbars("chameleon");
+    }
+
+    @Test
+    public void testNoScrollbarsRuno() {
+        testNoScrollbars("runo");
+    }
+
+    @Test
+    public void testNoScrollbarsReindeer() {
+        testNoScrollbars("reindeer");
+    }
+
+    private void testNoScrollbars(String theme) {
+        openTestURL("theme=" + theme);
+
+        for (CustomComboBoxElement cb : $(CustomComboBoxElement.class).all()) {
+            String caption = cb.getCaption();
+            cb.openPopup();
+            WebElement popup = cb.getSuggestionPopup();
+            WebElement scrollable = popup.findElement(By
+                    .className("v-filterselect-suggestmenu"));
+            assertNoHorizontalScrollbar(scrollable, caption);
+            assertNoVerticalScrollbar(scrollable, caption);
+        }
+    }
+
+}
diff --git a/uitest/src/com/vaadin/tests/components/combobox/CustomComboBoxElement.java b/uitest/src/com/vaadin/tests/components/combobox/CustomComboBoxElement.java
new file mode 100644
index 0000000000..697d5eb932
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/combobox/CustomComboBoxElement.java
@@ -0,0 +1,40 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.combobox;
+
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.ComboBoxElement;
+import com.vaadin.testbench.elementsbase.ServerClass;
+
+@ServerClass("com.vaadin.ui.ComboBox")
+public class CustomComboBoxElement extends ComboBoxElement {
+    private static org.openqa.selenium.By bySuggestionPopup = By
+            .vaadin("#popup");
+
+    public WebElement getSuggestionPopup() {
+        ensurePopupOpen();
+        return findElement(bySuggestionPopup);
+    }
+
+    private void ensurePopupOpen() {
+        if (!isElementPresent(bySuggestionPopup)) {
+            openPopup();
+        }
+    }
+
+}
diff --git a/uitest/src/com/vaadin/tests/components/grid/GridDetailsLocationTest.java b/uitest/src/com/vaadin/tests/components/grid/GridDetailsLocationTest.java
index f20cc0f6f4..33f66d35be 100644
--- a/uitest/src/com/vaadin/tests/components/grid/GridDetailsLocationTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/GridDetailsLocationTest.java
@@ -23,7 +23,6 @@ import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.openqa.selenium.By;
-import org.openqa.selenium.JavascriptExecutor;
 import org.openqa.selenium.Keys;
 import org.openqa.selenium.StaleElementReferenceException;
 import org.openqa.selenium.WebDriver;
@@ -313,10 +312,6 @@ public class GridDetailsLocationTest extends MultiBrowserTest {
         checkBoxElement.click(5, 5);
     }
 
-    private Object executeScript(String string, Object... param) {
-        return ((JavascriptExecutor) getDriver()).executeScript(string, param);
-    }
-
     private void scrollAndToggle(int row) {
         setRow(row);
         getScrollAndToggle().click();
diff --git a/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java b/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
index 842fcbb859..0e983ab959 100644
--- a/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
+++ b/uitest/src/com/vaadin/tests/tb3/AbstractTB3Test.java
@@ -368,8 +368,8 @@ public abstract class AbstractTB3Test extends ParallelTest {
      *         {@link org.openqa.selenium.JavascriptExecutor#executeScript(String, Object...)}
      *         returns
      */
-    protected Object executeScript(String script) {
-        return ((JavascriptExecutor) getDriver()).executeScript(script);
+    protected Object executeScript(String script, Object... args) {
+        return ((JavascriptExecutor) getDriver()).executeScript(script, args);
     }
 
     /**
@@ -1105,4 +1105,75 @@ public abstract class AbstractTB3Test extends ParallelTest {
         return isElementPresent(By.className("v-debugwindow"));
     }
 
+    protected void assertNoHorizontalScrollbar(WebElement element,
+            String errorMessage) {
+        // IE rounds clientWidth/clientHeight down and scrollHeight/scrollWidth
+        // up, so using clientWidth/clientHeight will fail if the element height
+        // is not an integer
+        int clientWidth = getClientWidth(element);
+        int scrollWidth = getScrollWidth(element);
+        boolean hasScrollbar = scrollWidth > clientWidth;
+
+        Assert.assertFalse(
+                "The element should not have a horizontal scrollbar (scrollWidth: "
+                        + scrollWidth + ", clientWidth: " + clientWidth + "): "
+                        + errorMessage, hasScrollbar);
+    }
+
+    protected void assertNoVerticalScrollbar(WebElement element,
+            String errorMessage) {
+        // IE rounds clientWidth/clientHeight down and scrollHeight/scrollWidth
+        // up, so using clientWidth/clientHeight will fail if the element height
+        // is not an integer
+        int clientHeight = getClientHeight(element);
+        int scrollHeight = getScrollHeight(element);
+        boolean hasScrollbar = scrollHeight > clientHeight;
+
+        Assert.assertFalse(
+                "The element should not have a vertical scrollbar (scrollHeight: "
+                        + scrollHeight + ", clientHeight: " + clientHeight
+                        + "): " + errorMessage, hasScrollbar);
+    }
+
+    protected int getScrollHeight(WebElement element) {
+        return ((Number) executeScript("return arguments[0].scrollHeight;",
+                element)).intValue();
+    }
+
+    protected int getScrollWidth(WebElement element) {
+        return ((Number) executeScript("return arguments[0].scrollWidth;",
+                element)).intValue();
+    }
+
+    /**
+     * Returns client height rounded up instead of as double because of IE9
+     * issues: https://dev.vaadin.com/ticket/18469
+     */
+    protected int getClientHeight(WebElement e) {
+        String script;
+        if (BrowserUtil.isIE8(getDesiredCapabilities())) {
+            script = "return arguments[0].clientHeight;"; //
+        } else {
+            script = "var cs = window.getComputedStyle(arguments[0]);"
+                    + "return Math.ceil(parseFloat(cs.height)+parseFloat(cs.paddingTop)+parseFloat(cs.paddingBottom));";
+        }
+        return ((Number) executeScript(script, e)).intValue();
+    }
+
+    /**
+     * Returns client width rounded up instead of as double because of IE9
+     * issues: https://dev.vaadin.com/ticket/18469
+     */
+    protected int getClientWidth(WebElement e) {
+        String script;
+        if (BrowserUtil.isIE8(getDesiredCapabilities())) {
+            script = "return arguments[0].clientWidth;";
+        } else {
+            script = "var cs = window.getComputedStyle(arguments[0]);"
+                    + "var h = parseFloat(cs.width)+parseFloat(cs.paddingLeft)+parseFloat(cs.paddingRight);"
+                    + "return Math.ceil(h);";
+        }
+
+        return ((Number) executeScript(script, e)).intValue();
+    }
 }