Merge branch 'master' into layoutperformance

10 files changed, 44 insertions, 117 deletions

diff --git a/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java b/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java
index f438e27f78..4fbb4b26b9 100644
--- a/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java
+++ b/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java
@@ -1668,7 +1668,8 @@ public class ApplicationConnection {
         final String pid = uidl.getId();
         if (!paintableMap.hasPaintable(pid)) {
             // Create and register a new paintable if no old was found
-            VPaintableWidget p = widgetSet.createWidget(uidl, configuration);
+            VPaintableWidget p = widgetSet.createWidget(uidl.getTag(),
+                    configuration);
             p.setConnection(this);
             p.setId(pid);
             p.init();
diff --git a/src/com/vaadin/terminal/gwt/client/VPaintableMap.java b/src/com/vaadin/terminal/gwt/client/VPaintableMap.java
index 48d32f4a5b..03fb599fe9 100644
--- a/src/com/vaadin/terminal/gwt/client/VPaintableMap.java
+++ b/src/com/vaadin/terminal/gwt/client/VPaintableMap.java
@@ -22,7 +22,6 @@ import com.vaadin.terminal.gwt.client.RenderInformation.Size;
 public class VPaintableMap {
 
     private Map<String, VPaintable> idToPaintable = new HashMap<String, VPaintable>();
-    private Map<VPaintable, String> paintableToId = new HashMap<VPaintable, String>();
 
     public static VPaintableMap get(ApplicationConnection applicationConnection) {
         return applicationConnection.getPaintableMap();
@@ -81,7 +80,6 @@ public class VPaintableMap {
      */
     public void clear() {
         idToPaintable.clear();
-        paintableToId.clear();
         idToComponentDetail.clear();
     }
 
@@ -99,7 +97,6 @@ public class VPaintableMap {
         ComponentDetail componentDetail = GWT.create(ComponentDetail.class);
         idToComponentDetail.put(pid, componentDetail);
         idToPaintable.put(pid, paintable);
-        paintableToId.put(paintable, pid);
         if (paintable instanceof VPaintableWidget) {
             VPaintableWidget pw = (VPaintableWidget) paintable;
             setPid(pw.getWidgetForPaintable().getElement(), pid);
@@ -122,12 +119,14 @@ public class VPaintableMap {
      *            the paintable who's id is needed
      * @return the id for the given paintable or null if the paintable could not
      *         be found
+     * @deprecated use {@link VPaintable#getId()} instead
      */
+    @Deprecated
     public String getPid(VPaintable paintable) {
         if (paintable == null) {
             return null;
         }
-        return paintableToId.get(paintable);
+        return paintable.getId();
     }
 
     @Deprecated
@@ -250,7 +249,6 @@ public class VPaintableMap {
                     // clean reference to paintable
                     idToComponentDetail.remove(pid);
                     idToPaintable.remove(pid);
-                    paintableToId.remove(paintable);
                 }
                 /*
                  * else NOP : same component has been reattached to another
@@ -351,7 +349,7 @@ public class VPaintableMap {
     }
 
     public Collection<? extends VPaintable> getPaintables() {
-        return Collections.unmodifiableCollection(paintableToId.keySet());
+        return Collections.unmodifiableCollection(idToPaintable.values());
     }
 
     /**
@@ -391,4 +389,4 @@ public class VPaintableMap {
         return getPid(w) != null;
     }
 
-}
\ No newline at end of file
+}
diff --git a/src/com/vaadin/terminal/gwt/client/WidgetSet.java b/src/com/vaadin/terminal/gwt/client/WidgetSet.java
index 390d79ce17..7874c9d068 100644
--- a/src/com/vaadin/terminal/gwt/client/WidgetSet.java
+++ b/src/com/vaadin/terminal/gwt/client/WidgetSet.java
@@ -6,8 +6,6 @@ package com.vaadin.terminal.gwt.client;
 
 import com.google.gwt.core.client.GWT;
 import com.google.gwt.user.client.ui.Widget;
-import com.vaadin.terminal.gwt.client.ui.VFilterSelectPaintable;
-import com.vaadin.terminal.gwt.client.ui.VListSelectPaintable;
 import com.vaadin.terminal.gwt.client.ui.VUnknownComponentPaintable;
 
 public class WidgetSet {
@@ -24,15 +22,15 @@ public class WidgetSet {
      * component must be a {@link Widget} that implements
      * {@link VPaintableWidget}.
      * 
-     * @param uidl
-     *            UIDL to be painted with returned component.
+     * @param tag
+     *            component type tag for the component to create
      * @param client
      *            the application connection that whishes to instantiate widget
      * 
      * @return New uninitialized and unregistered component that can paint given
      *         UIDL.
      */
-    public VPaintableWidget createWidget(UIDL uidl,
+    public VPaintableWidget createWidget(String tag,
             ApplicationConfiguration conf) {
         /*
          * Yes, this (including the generated code in WidgetMap) may look very
@@ -40,15 +38,14 @@ public class WidgetSet {
          * Luckily this is mostly written by WidgetSetGenerator, here are just
          * some hacks. Extra instantiation code is needed if client side widget
          * has no "native" counterpart on client side.
-         * 
-         * TODO should try to get rid of these exceptions here
          */
 
-        final Class<? extends VPaintableWidget> classType = resolveWidgetType(
-                uidl, conf);
+        Class<? extends VPaintableWidget> classType = resolveWidgetType(tag,
+                conf);
+
         if (classType == null || classType == VUnknownComponentPaintable.class) {
             String serverSideName = conf
-                    .getUnknownServerClassNameByEncodedTagName(uidl.getTag());
+                    .getUnknownServerClassNameByEncodedTagName(tag);
             VUnknownComponentPaintable c = GWT
                     .create(VUnknownComponentPaintable.class);
             c.setServerSideClassName(serverSideName);
@@ -62,43 +59,16 @@ public class WidgetSet {
 
     }
 
-    protected Class<? extends VPaintableWidget> resolveWidgetType(UIDL uidl,
+    protected Class<? extends VPaintableWidget> resolveWidgetType(String tag,
             ApplicationConfiguration conf) {
-        final String tag = uidl.getTag();
-
         Class<? extends VPaintableWidget> widgetClass = conf
                 .getWidgetClassByEncodedTag(tag);
 
-        // add our historical quirks
-
-        if (widgetClass == VFilterSelectPaintable.class) {
-            if (uidl.hasAttribute("type")) {
-                final String type = uidl.getStringAttribute("type").intern();
-                if ("legacy-multi" == type) {
-                    return VListSelectPaintable.class;
-                }
-            }
-        }
         return widgetClass;
 
     }
 
     /**
-     * Test if the given component implementation conforms to UIDL.
-     * 
-     * @param currentWidget
-     *            Current implementation of the component
-     * @param uidl
-     *            UIDL to test against
-     * @return true iff createWidget would return a new component of the same
-     *         class than currentWidget
-     */
-    public boolean isCorrectImplementation(Widget currentWidget, UIDL uidl,
-            ApplicationConfiguration conf) {
-        return currentWidget.getClass() == resolveWidgetType(uidl, conf);
-    }
-
-    /**
      * Due its nature, GWT does not support dynamic classloading. To bypass this
      * limitation, widgetset must have function that returns Class by its fully
      * qualified name.
@@ -115,16 +85,6 @@ public class WidgetSet {
         Class<? extends VPaintableWidget> implementationByServerSideClassName = widgetMap
                 .getImplementationByServerSideClassName(fullyqualifiedName);
 
-        /*
-         * Also ensure that our historical quirks have their instantiators
-         * loaded. Without these, legacy code will throw NPEs when e.g. a Select
-         * is in multiselect mode, causing the clientside implementation to
-         * *actually* be VListSelect, when the annotation says VFilterSelect
-         */
-        if (fullyqualifiedName.equals("com.vaadin.ui.Select")) {
-            loadImplementation(VListSelectPaintable.class);
-        }
-
         return implementationByServerSideClassName;
 
     }
diff --git a/src/com/vaadin/ui/AbstractSelect.java b/src/com/vaadin/ui/AbstractSelect.java
index 5e086f0b8d..47f132aa77 100644
--- a/src/com/vaadin/ui/AbstractSelect.java
+++ b/src/com/vaadin/ui/AbstractSelect.java
@@ -976,10 +976,13 @@ public abstract class AbstractSelect extends AbstractField<Object> implements
     }
 
     /**
-     * Sets the multiselect mode. Setting multiselect mode false may loose
+     * Sets the multiselect mode. Setting multiselect mode false may lose
      * selection information: if selected items set contains one or more
      * selected items, only one of the selected items is kept as selected.
      * 
+     * Subclasses of AbstractSelect can choose not to support changing the
+     * multiselect mode, and may throw {@link UnsupportedOperationException}.
+     * 
      * @param multiSelect
      *            the New value of property multiSelect.
      */
diff --git a/src/com/vaadin/ui/ComboBox.java b/src/com/vaadin/ui/ComboBox.java
index 013fe6ab85..12db8c18af 100644
--- a/src/com/vaadin/ui/ComboBox.java
+++ b/src/com/vaadin/ui/ComboBox.java
@@ -34,36 +34,24 @@ public class ComboBox extends Select {
     private boolean textInputAllowed = true;
 
     public ComboBox() {
-        setMultiSelect(false);
         setNewItemsAllowed(false);
     }
 
     public ComboBox(String caption, Collection<?> options) {
         super(caption, options);
-        setMultiSelect(false);
         setNewItemsAllowed(false);
     }
 
     public ComboBox(String caption, Container dataSource) {
         super(caption, dataSource);
-        setMultiSelect(false);
         setNewItemsAllowed(false);
     }
 
     public ComboBox(String caption) {
         super(caption);
-        setMultiSelect(false);
         setNewItemsAllowed(false);
     }
 
-    @Override
-    public void setMultiSelect(boolean multiSelect) {
-        if (multiSelect && !isMultiSelect()) {
-            throw new UnsupportedOperationException("Multiselect not supported");
-        }
-        super.setMultiSelect(multiSelect);
-    }
-
     /**
      * Gets the current input prompt.
      * 
diff --git a/src/com/vaadin/ui/Form.java b/src/com/vaadin/ui/Form.java
index c79804c7e7..dd3710cd2c 100644
--- a/src/com/vaadin/ui/Form.java
+++ b/src/com/vaadin/ui/Form.java
@@ -861,13 +861,16 @@ public class Form extends AbstractField<Object> implements Item.Editor,
      * match. Null values are not supported.
      * </p>
      * 
+     * Note: since Vaadin 7.0, returns an {@link AbstractSelect} instead of a
+     * {@link Select}.
+     * 
      * @param propertyId
      *            the id of the property.
      * @param values
      * @param descriptions
      * @return the select property generated
      */
-    public Select replaceWithSelect(Object propertyId, Object[] values,
+    public AbstractSelect replaceWithSelect(Object propertyId, Object[] values,
             Object[] descriptions) {
 
         // Checks the parameters
@@ -927,10 +930,8 @@ public class Form extends AbstractField<Object> implements Item.Editor,
         }
 
         // Creates the new field matching to old field parameters
-        final Select newField = new Select();
-        if (isMultiselect) {
-            newField.setMultiSelect(true);
-        }
+        final AbstractSelect newField = isMultiselect ? new ListSelect()
+                : new Select();
         newField.setCaption(oldField.getCaption());
         newField.setReadOnly(oldField.isReadOnly());
         newField.setReadThrough(oldField.isReadThrough());
diff --git a/src/com/vaadin/ui/Select.java b/src/com/vaadin/ui/Select.java
index 38785f3ab9..c1da88acea 100644
--- a/src/com/vaadin/ui/Select.java
+++ b/src/com/vaadin/ui/Select.java
@@ -754,11 +754,15 @@ public class Select extends AbstractSelect implements AbstractSelect.Filtering,
      * @deprecated use {@link ListSelect}, {@link OptionGroup} or
      *             {@link TwinColSelect} instead
      * @see com.vaadin.ui.AbstractSelect#setMultiSelect(boolean)
+     * @throws UnsupportedOperationException
+     *             if trying to activate multiselect mode
      */
     @Deprecated
     @Override
     public void setMultiSelect(boolean multiSelect) {
-        super.setMultiSelect(multiSelect);
+        if (multiSelect) {
+            throw new UnsupportedOperationException("Multiselect not supported");
+        }
     }
 
     /**
diff --git a/tests/testbench/com/vaadin/tests/components/select/MultiSelect.java b/tests/testbench/com/vaadin/tests/components/select/MultiSelect.java
deleted file mode 100644
index d823d1fd10..0000000000
--- a/tests/testbench/com/vaadin/tests/components/select/MultiSelect.java
+++ /dev/null
@@ -1,32 +0,0 @@
-package com.vaadin.tests.components.select;
-
-import com.vaadin.tests.components.TestBase;
-import com.vaadin.ui.Select;
-
-public class MultiSelect extends TestBase {
-
-    @SuppressWarnings("deprecation")
-    @Override
-    protected void setup() {
-        Select selectComponent = new Select();
-        selectComponent.setMultiSelect(true);
-
-        String[] selection = { "One", "Hund", "Three" };
-        for (String word : selection) {
-            selectComponent.addItem(word);
-        }
-
-        addComponent(selectComponent);
-    }
-
-    @Override
-    protected String getDescription() {
-        return "The select is in multi select mode and should be rendered as such";
-    }
-
-    @Override
-    protected Integer getTicketNumber() {
-        return 4553;
-    }
-
-}
diff --git a/tests/testbench/com/vaadin/tests/layouts/OrderedLayoutBasics.java b/tests/testbench/com/vaadin/tests/layouts/OrderedLayoutBasics.java
index 1fe11f1a92..9f18ef7887 100644
--- a/tests/testbench/com/vaadin/tests/layouts/OrderedLayoutBasics.java
+++ b/tests/testbench/com/vaadin/tests/layouts/OrderedLayoutBasics.java
@@ -16,8 +16,8 @@ import com.vaadin.ui.Component;
 import com.vaadin.ui.HorizontalLayout;
 import com.vaadin.ui.Label;
 import com.vaadin.ui.Layout;
+import com.vaadin.ui.ListSelect;
 import com.vaadin.ui.Panel;
-import com.vaadin.ui.Select;
 import com.vaadin.ui.TextArea;
 import com.vaadin.ui.TextField;
 import com.vaadin.ui.VerticalLayout;
@@ -116,13 +116,13 @@ public class OrderedLayoutBasics extends TestBase {
         ol.addComponent(tf);
         ol.setComponentAlignment(tf, Alignment.BOTTOM_LEFT);
 
-        Select s = new Select("100% high select");
+        ListSelect s = new ListSelect("100% high select");
         s.setMultiSelect(true);
         s.setHeight("100%");
         s.setWidth("");
         ol.addComponent(s);
 
-        s = new Select("200 px high select");
+        s = new ListSelect("200 px high select");
         s.setMultiSelect(true);
         s.setHeight("200px");
         s.setWidth("");
@@ -172,7 +172,8 @@ public class OrderedLayoutBasics extends TestBase {
         ol.addComponent(l);
         ol.setComponentAlignment(l, Alignment.BOTTOM_LEFT);
 
-        Select s = new Select("100% high select, should fit into layout");
+        ListSelect s = new ListSelect(
+                "100% high select, should fit into layout");
         s.setMultiSelect(true);
         s.setHeight("100%");
         s.setWidth("");
@@ -182,7 +183,7 @@ public class OrderedLayoutBasics extends TestBase {
 
         ol.addComponent(s);
 
-        s = new Select("200 px high select, should be partly outside");
+        s = new ListSelect("200 px high select, should be partly outside");
         s.setMultiSelect(true);
         s.setHeight("200px");
         s.setWidth("");
@@ -224,7 +225,8 @@ public class OrderedLayoutBasics extends TestBase {
         ol.addComponent(l);
         ol.setComponentAlignment(l, Alignment.BOTTOM_LEFT);
 
-        Select s = new Select("100% high select, should fit into layout");
+        ListSelect s = new ListSelect(
+                "100% high select, should fit into layout");
         s.setMultiSelect(true);
         s.setHeight("100%");
         s.setWidth("100%");
@@ -234,7 +236,8 @@ public class OrderedLayoutBasics extends TestBase {
 
         ol.addComponent(s);
 
-        s = new Select("200 px high select, should make the layout 200px high");
+        s = new ListSelect(
+                "200 px high select, should make the layout 200px high");
         s.setMultiSelect(true);
         s.setHeight("200px");
         s.setWidth("100%");
@@ -277,7 +280,8 @@ public class OrderedLayoutBasics extends TestBase {
         ol.addComponent(l);
         ol.setComponentAlignment(l, Alignment.BOTTOM_LEFT);
 
-        Select s = new Select("100% high select, should fit into layout");
+        ListSelect s = new ListSelect(
+                "100% high select, should fit into layout");
         s.setMultiSelect(true);
         s.setHeight("100%");
         s.setWidth("100%");
@@ -287,7 +291,8 @@ public class OrderedLayoutBasics extends TestBase {
 
         ol.addComponent(s);
 
-        s = new Select("200 px high select, should make the layout 200px high");
+        s = new ListSelect(
+                "200 px high select, should make the layout 200px high");
         s.setMultiSelect(true);
         s.setHeight("200px");
         s.setWidth("100%");
diff --git a/tests/testbench/com/vaadin/tests/tickets/Ticket1230.java b/tests/testbench/com/vaadin/tests/tickets/Ticket1230.java
index 7cdbb1300e..872084d436 100644
--- a/tests/testbench/com/vaadin/tests/tickets/Ticket1230.java
+++ b/tests/testbench/com/vaadin/tests/tickets/Ticket1230.java
@@ -127,7 +127,6 @@ public class Ticket1230 extends Application.LegacyApplication {
     @SuppressWarnings("deprecation")
     private Select createSelect() {
         Select select = new Select();
-        select.setMultiSelect(false);
         select.addContainerProperty(PROPERTY_ID, String.class, "");
         select.setItemCaptionPropertyId(PROPERTY_ID);