Merged fixes from 6.7

svn changeset:22132/svn branch:6.8

src/com/vaadin/data/util/PropertyFormatter.java

@@ -34,7 +34,8 @@ import com.vaadin.data.Property;
  */
 @SuppressWarnings("serial")
 public abstract class PropertyFormatter extends AbstractProperty implements
-        Property.ValueChangeListener, Property.ReadOnlyStatusChangeListener {
+        Property.Viewer, Property.ValueChangeListener,
+        Property.ReadOnlyStatusChangeListener {
 
     /** Datasource that stores the actual value. */
     Property dataSource;
@@ -146,7 +147,10 @@ public abstract class PropertyFormatter extends AbstractProperty implements
      */
     @Override
     public String toString() {
-        Object value = dataSource == null ? false : dataSource.getValue();
+        if (dataSource == null) {
+            return null;
+        }
+        Object value = dataSource.getValue();
         if (value == null) {
             return null;
         }
@@ -154,6 +158,7 @@ public abstract class PropertyFormatter extends AbstractProperty implements
     }
 
     /** Reflects the read-only status of the datasource. */
+    @Override
     public boolean isReadOnly() {
         return dataSource == null ? false : dataSource.isReadOnly();
     }
@@ -190,6 +195,7 @@ public abstract class PropertyFormatter extends AbstractProperty implements
      * @param newStatus
      *            the new read-only status of the Property.
      */
+    @Override
     public void setReadOnly(boolean newStatus) {
         if (dataSource != null) {
             dataSource.setReadOnly(newStatus);
@@ -208,16 +214,14 @@ public abstract class PropertyFormatter extends AbstractProperty implements
             }
         } else {
             try {
-                dataSource.setValue(parse((String) newValue));
+                dataSource.setValue(parse(newValue.toString()));
                 if (!newValue.equals(toString())) {
                     fireValueChange();
                 }
+            } catch (ConversionException e) {
+                throw e;
             } catch (Exception e) {
-                if (e instanceof ConversionException) {
-                    throw (ConversionException) e;
-                } else {
-                    throw new ConversionException(e);
-                }
+                throw new ConversionException(e);
             }
         }
     }

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

@@ -5704,6 +5704,14 @@ public class VScrollTable extends FlowPanel implements Table, ScrollHandler,
         if (this.width.equals(width)) {
             return;
         }
+        if (!isVisible()) {
+            /*
+             * Do not update size when the table is hidden as all column widths
+             * will be set to zero and they won't be recalculated when the table
+             * is set visible again (until the size changes again)
+             */
+            return;
+        }
 
         this.width = width;
         if (width != null && !"".equals(width)) {

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

@@ -1027,7 +1027,7 @@ public class VWindow extends VOverlay implements Container,
     }
 
     @Override
-    /*
+    /**
      * Width is set to the out-most element (v-window).
      * 
      * This function should never be called with percentage values (it will
@@ -1088,33 +1088,45 @@ public class VWindow extends VOverlay implements Container,
     }
 
     @Override
-    /*
+    /**
      * Height is set to the out-most element (v-window).
      * 
      * This function should never be called with percentage values (it will
      * throw an exception)
+     * 
+     * @param height A CSS string specifying the new height of the window.
+     *               An empty string or null clears the height and lets
+     *               the browser to compute it based on the window contents. 
      */
     public void setHeight(String height) {
-        this.height = height;
-        if (!isAttached()) {
+        if (!isAttached() || (height == null 
+                ? this.height == null 
+                : height.equals(this.height))) {
             return;
         }
-        if (height != null && !"".equals(height)) {
-            DOM.setStyleAttribute(getElement(), "height", height);
-            int pixels = getElement().getOffsetHeight() - getExtraHeight();
-            if (pixels < MIN_CONTENT_AREA_HEIGHT) {
-                pixels = MIN_CONTENT_AREA_HEIGHT;
-                int rootHeight = pixels + getExtraHeight();
-                DOM.setStyleAttribute(getElement(), "height", (rootHeight)
-                        + "px");
-
+        if (height == null || "".equals(height)) {
+            getElement().getStyle().clearHeight();
+            contentPanel.getElement().getStyle().clearHeight();
+            // Reset to default, the exact value does not actually
+            // matter as an undefined-height parent should not have
+            // a relative-height child anyway.
+            renderSpace.setHeight(MIN_CONTENT_AREA_HEIGHT);
+        } else {
+            getElement().getStyle().setProperty("height", height);
+            int contentHeight =
+                    getElement().getOffsetHeight() - getExtraHeight();
+            if (contentHeight < MIN_CONTENT_AREA_HEIGHT) {
+                contentHeight = MIN_CONTENT_AREA_HEIGHT;
+                int rootHeight = contentHeight + getExtraHeight();
+                getElement().getStyle().setProperty(
+                        "height", rootHeight + "px");
             }
-            renderSpace.setHeight(pixels);
-            height = pixels + "px";
-            contentPanel.getElement().getStyle().setProperty("height", height);
-            updateShadowSizeAndPosition();
-
+            renderSpace.setHeight(contentHeight);
+            contentPanel.getElement().getStyle().setProperty(
+                    "height", contentHeight + "px");
         }
+        this.height = height;
+        updateShadowSizeAndPosition();
     }
 
     private int extraH = 0;

src/com/vaadin/ui/Form.java

@@ -491,7 +491,7 @@ public class Form extends AbstractField implements Item.Editor, Buffered, Item,
         }
 
         // Configures the field
-        field.setPropertyDataSource(property);
+        bindPropertyToField(id, property, field);
 
         // Register and attach the created field
         addField(id, field);
@@ -755,13 +755,39 @@ public class Form extends AbstractField implements Item.Editor, Buffered, Item,
                 final Field f = fieldFactory.createField(itemDatasource, id,
                         this);
                 if (f != null) {
-                    f.setPropertyDataSource(property);
+                    bindPropertyToField(id, property, f);
                     addField(id, f);
                 }
             }
         }
     }
 
+    /**
+     * Binds an item property to a field. The default behavior is to bind
+     * property straight to Field. If Property.Viewer type property (e.g.
+     * PropertyFormatter) is already set for field, the property is bound to
+     * that Property.Viewer.
+     * 
+     * @param propertyId
+     * @param property
+     * @param field
+     * @since 6.7.3
+     */
+    protected void bindPropertyToField(final Object propertyId,
+            final Property property, final Field field) {
+        // check if field has a property that is Viewer set. In that case we
+        // expect developer has e.g. PropertyFormatter that he wishes to use and
+        // assign the property to the Viewer instead.
+        boolean hasFilterProperty = field.getPropertyDataSource() != null
+                && (field.getPropertyDataSource() instanceof Property.Viewer);
+        if (hasFilterProperty) {
+            ((Property.Viewer) field.getPropertyDataSource())
+                    .setPropertyDataSource(property);
+        } else {
+            field.setPropertyDataSource(property);
+        }
+    }
+
     /**
      * Gets the layout of the form.
      * 

src/com/vaadin/ui/Table.java

@@ -3405,7 +3405,7 @@ public class Table extends AbstractSelect implements Action.Container,
                 // Remember that we have made this association so we can remove
                 // it when the component is removed
                 associatedProperties.put(f, property);
-                f.setPropertyDataSource(property);
+                bindPropertyToField(rowId, colId, property, f);
                 return f;
             }
         }
@@ -3413,6 +3413,33 @@ public class Table extends AbstractSelect implements Action.Container,
         return formatPropertyValue(rowId, colId, property);
     }
 
+    /**
+     * Binds an item property to a field generated by TableFieldFactory. The
+     * default behavior is to bind property straight to Field. If
+     * Property.Viewer type property (e.g. PropertyFormatter) is already set for
+     * field, the property is bound to that Property.Viewer.
+     * 
+     * @param rowId
+     * @param colId
+     * @param property
+     * @param field
+     * @since 6.7.3
+     */
+    protected void bindPropertyToField(Object rowId, Object colId,
+            Property property, Field field) {
+        // check if field has a property that is Viewer set. In that case we
+        // expect developer has e.g. PropertyFormatter that he wishes to use and
+        // assign the property to the Viewer instead.
+        boolean hasFilterProperty = field.getPropertyDataSource() != null
+                && (field.getPropertyDataSource() instanceof Property.Viewer);
+        if (hasFilterProperty) {
+            ((Property.Viewer) field.getPropertyDataSource())
+                    .setPropertyDataSource(property);
+        } else {
+            field.setPropertyDataSource(property);
+        }
+    }
+
     /**
      * Formats table cell property values. By default the property.toString()
      * and return a empty string for null properties.
@@ -3775,8 +3802,8 @@ public class Table extends AbstractSelect implements Action.Container,
      * <p>
      * Note, that some due to historical reasons the name of the method is bit
      * misleading. Some items may be partly or totally out of the viewport of
-     * the table's scrollable area. Actully detecting rows which can be actually
-     * seen by the end user may be problematic due to the client server
+     * the table's scrollable area. Actually detecting rows which can be
+     * actually seen by the end user may be problematic due to the client server
      * architecture. Using {@link #getCurrentPageFirstItemId()} combined with
      * {@link #getPageLength()} may produce good enough estimates in some
      * situations.
@@ -4507,19 +4534,14 @@ public class Table extends AbstractSelect implements Action.Container,
          * com.vaadin.event.dd.acceptcriteria.AcceptCriterion#accepts(com.vaadin
          * .event.dd.DragAndDropEvent)
          */
+        @SuppressWarnings("unchecked")
         public boolean accept(DragAndDropEvent dragEvent) {
             AbstractSelectTargetDetails dropTargetData = (AbstractSelectTargetDetails) dragEvent
                     .getTargetDetails();
             table = (Table) dragEvent.getTargetDetails().getTarget();
-            ArrayList<Object> visibleItemIds = new ArrayList<Object>(
-                    table.getPageLength());
-            visibleItemIds.size();
-            Object id = table.getCurrentPageFirstItemId();
-            for (int i = 0; i < table.getPageLength() && id != null; i++) {
-                visibleItemIds.add(id);
-                id = table.nextItemId(id);
-            }
-            allowedItemIds = getAllowedItemIds(dragEvent, table, visibleItemIds);
+            Collection<?> visibleItemIds = table.getVisibleItemIds();
+            allowedItemIds = getAllowedItemIds(dragEvent, table,
+                    (Collection<Object>) visibleItemIds);
 
             return allowedItemIds.contains(dropTargetData.getItemIdOver());
         }

tests/server-side/com/vaadin/tests/server/TestPropertyFormatter.java

@@ -0,0 +1,69 @@
+package com.vaadin.tests.server;
+
+import java.util.Date;
+
+import junit.framework.TestCase;
+
+import org.junit.Test;
+
+import com.vaadin.data.util.ObjectProperty;
+import com.vaadin.data.util.PropertyFormatter;
+
+@SuppressWarnings("unchecked")
+public class TestPropertyFormatter extends TestCase {
+
+    class TestFormatter extends PropertyFormatter {
+
+        @Override
+        public String format(Object value) {
+            boolean isCorrectType = getExpectedClass().isAssignableFrom(
+                    value.getClass());
+            assertTrue(isCorrectType);
+            return "FOO";
+        }
+
+        @Override
+        public Object parse(String formattedValue) throws Exception {
+            return getExpectedClass().newInstance();
+        }
+    };
+    @SuppressWarnings("rawtypes")
+    private Class expectedClass;
+
+    @SuppressWarnings("rawtypes")
+    private Class getExpectedClass() {
+        return expectedClass;
+    }
+    
+    /**
+     * The object passed to format should be same as property's type.
+     * @throws IllegalAccessException 
+     * @throws InstantiationException 
+     */
+    @Test
+    @SuppressWarnings({ "rawtypes" })
+    public void testCorrectTypeForFormat() throws InstantiationException, IllegalAccessException {
+        Class[] testedTypes = new Class[] {Integer.class, Boolean.class, Double.class, String.class, Date.class};
+        Object[] testValues = new Object[] {new Integer(3), Boolean.FALSE, new Double(3.3), "bar", new Date()};
+        
+        int i = 0;
+        for (Class class1 : testedTypes) {
+            expectedClass = class1;
+            
+            TestFormatter formatter = new TestFormatter();
+            
+            // Should just return null, without formatting
+            Object value = formatter.getValue();
+            
+            // test with property which value is null
+            formatter.setPropertyDataSource(new ObjectProperty(null, expectedClass));
+            formatter.getValue(); // calls format
+            
+            // test with a value
+            formatter.setPropertyDataSource(new ObjectProperty(testValues[i++], expectedClass));
+            formatter.getValue(); // calls format
+        }
+        
+
+    }
+}

tests/testbench/com/vaadin/tests/components/form/FormWithPropertyFormatterConnected.java

@@ -0,0 +1,90 @@
+package com.vaadin.tests.components.form;
+
+import com.vaadin.data.Item;
+import com.vaadin.data.util.BeanItem;
+import com.vaadin.data.util.PropertyFormatter;
+import com.vaadin.tests.components.TestBase;
+import com.vaadin.ui.AbstractField;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Component;
+import com.vaadin.ui.DefaultFieldFactory;
+import com.vaadin.ui.Field;
+import com.vaadin.ui.Form;
+import com.vaadin.ui.FormFieldFactory;
+
+public class FormWithPropertyFormatterConnected extends TestBase {
+    @Override
+    protected void setup() {
+        Form form2 = new Form();
+        form2.setFormFieldFactory(new FormFieldFactory() {
+
+            public Field createField(Item item, Object propertyId,
+                    Component uiContext) {
+                AbstractField f = (AbstractField) DefaultFieldFactory.get()
+                        .createField(item, propertyId, uiContext);
+                if (propertyId.equals("age")) {
+                    f.setPropertyDataSource(new PropertyFormatter() {
+
+                        @Override
+                        public Object parse(String formattedValue)
+                                throws Exception {
+                            String str = formattedValue
+                                    .replaceAll("[^0-9.]", "");
+                            if (formattedValue.toLowerCase().contains("months")) {
+                                return Double.parseDouble(str) / 12;
+                            }
+                            return Double.parseDouble(str);
+                        }
+
+                        @Override
+                        public String format(Object value) {
+                            Double dValue = (Double) value;
+                            if (dValue < 1) {
+                                return ((int)(dValue * 12)) + " months";
+                            }
+                            return dValue + " years";
+                        }
+                    });
+                    f.setImmediate(true);
+                }
+                return f;
+            }
+        });
+        form2.setItemDataSource(createItem());
+        
+        addComponent(form2);
+        addComponent(new Button("B"));
+    }
+
+    private Item createItem() {
+        return new BeanItem<Person>(new Person(0.5));
+    }
+
+    public class Person {
+        public Person(double age) {
+            super();
+            this.age = age;
+        }
+
+        public double getAge() {
+            return age;
+        }
+
+        public void setAge(double age) {
+            this.age = age;
+        }
+
+        private double age;
+    }
+
+    @Override
+    protected String getDescription() {
+        return "It should be possible to inject PropertyFormatter and similar classses to fields in form. The test app hooks formatter that displays age in years or months and also accepts value in both (years by default, months if mentioned in the field)";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return null;
+    }
+
+}

tests/testbench/com/vaadin/tests/components/window/SubWindowWithUndefinedHeight.html

@@ -0,0 +1,52 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head profile="http://selenium-ide.openqa.org/profiles/test-case">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+<link rel="selenium.base" href="" />
+<title>SubWindowWithUndefinedHeight</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">SubWindowWithUndefinedHeight</td></tr>
+</thead><tbody>
+<tr>
+	<td>open</td>
+	<td>/run/com.vaadin.tests.components.window.SubWindowWithUndefinedHeight?restartApplication</td>
+	<td></td>
+</tr>
+<tr>
+	<td>click</td>
+	<td>vaadin=runcomvaadintestscomponentswindowSubWindowWithUndefinedHeight::/VVerticalLayout[0]/ChildComponentContainer[2]/VButton[0]/domChild[0]</td>
+	<td></td>
+</tr>
+<tr>
+	<td>screenCapture</td>
+	<td></td>
+	<td>initial-tab1</td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentswindowSubWindowWithUndefinedHeight::/VWindow[0]/FocusableScrollPanel[0]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[1]/domChild[0]/domChild[0]/domChild[0]</td>
+	<td>17,13</td>
+</tr>
+<tr>
+	<td>screenCapture</td>
+	<td></td>
+	<td>select-tab2</td>
+</tr>
+<tr>
+	<td>mouseClick</td>
+	<td>vaadin=runcomvaadintestscomponentswindowSubWindowWithUndefinedHeight::/VWindow[0]/FocusableScrollPanel[0]/VVerticalLayout[0]/ChildComponentContainer[0]/VTabsheet[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]</td>
+	<td>8,9</td>
+</tr>
+<tr>
+	<td>screenCapture</td>
+	<td></td>
+	<td>select-tab1</td>
+</tr>
+
+</tbody></table>
+</body>
+</html>

tests/testbench/com/vaadin/tests/components/window/SubWindowWithUndefinedHeight.java

@@ -0,0 +1,64 @@
+package com.vaadin.tests.components.window;
+
+import com.vaadin.tests.components.TestBase;
+import com.vaadin.ui.Window;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Table;
+import com.vaadin.ui.TabSheet;
+
+public class SubWindowWithUndefinedHeight extends TestBase {
+
+    @Override
+    protected String getDescription() {
+        return "Setting subwindow height to undefined after initial rendering does not update visual height";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 7916;
+    }
+
+    @Override
+    protected void setup() {
+        final Window subwindow = new Window("subwindow");
+        subwindow.center();
+        subwindow.setSizeUndefined();
+        subwindow.getContent().setSizeUndefined();
+
+        final Button tabButton = new Button("A button");
+        tabButton.setCaption("Tab 1");
+        tabButton.setWidth("200px");
+
+        final Table table = new Table();
+        table.setCaption("tab 2");
+        table.setWidth("100%");
+        table.setHeight("100%");
+        
+        final TabSheet tabsheet = new TabSheet();
+        tabsheet.addComponent(tabButton);
+        tabsheet.addComponent(table);
+        tabsheet.addListener(new TabSheet.SelectedTabChangeListener() {
+            public void selectedTabChange(
+                    TabSheet.SelectedTabChangeEvent event) {
+                if (tabsheet.getSelectedTab() == tabButton) {
+                    tabsheet.setSizeUndefined();
+                    subwindow.getContent().setSizeUndefined();
+                    subwindow.setSizeUndefined();
+                } else if (tabsheet.getSelectedTab() == table) {
+                    subwindow.setWidth("500px");
+                    subwindow.setHeight("500px");
+                    subwindow.getContent().setSizeFull();
+                    tabsheet.setSizeFull();
+                }
+            }
+        });
+        subwindow.addComponent(tabsheet);
+
+        Button button = new Button("click me", new Button.ClickListener() {
+            public void buttonClick(Button.ClickEvent event) {
+                getMainWindow().addWindow(subwindow);
+            }
+        });
+        getMainWindow().addComponent(button);
+    }
+}