Merge branch 'master-18493' into grid-unbuffered-editor

Conflicts:
	client/src/com/vaadin/client/connectors/GridConnector.java
	server/src/com/vaadin/data/RpcDataProviderExtension.java
	server/src/com/vaadin/ui/Grid.java

Change-Id: Ie8931fbae322c93aeb409e8a6d393623ba9d9dc6

client/src/com/vaadin/client/connectors/GridConnector.java

@@ -19,13 +19,13 @@ package com.vaadin.client.connectors;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.logging.Logger;
 
@@ -40,6 +40,7 @@ import com.vaadin.client.ConnectorHierarchyChangeEvent;
 import com.vaadin.client.DeferredWorker;
 import com.vaadin.client.MouseEventDetailsBuilder;
 import com.vaadin.client.TooltipInfo;
+import com.vaadin.client.ServerConnector;
 import com.vaadin.client.communication.StateChangeEvent;
 import com.vaadin.client.connectors.RpcDataSourceConnector.DetailsListener;
 import com.vaadin.client.connectors.RpcDataSourceConnector.RpcDataSource;
@@ -85,10 +86,8 @@ import com.vaadin.client.widgets.Grid.FooterCell;
 import com.vaadin.client.widgets.Grid.FooterRow;
 import com.vaadin.client.widgets.Grid.HeaderCell;
 import com.vaadin.client.widgets.Grid.HeaderRow;
-import com.vaadin.shared.Connector;
 import com.vaadin.shared.data.sort.SortDirection;
 import com.vaadin.shared.ui.Connect;
-import com.vaadin.shared.ui.grid.DetailsConnectorChange;
 import com.vaadin.shared.ui.grid.EditorClientRpc;
 import com.vaadin.shared.ui.grid.EditorServerRpc;
 import com.vaadin.shared.ui.grid.GridClientRpc;
@@ -421,255 +420,107 @@ public class GridConnector extends AbstractHasComponentsConnector implements
         }
     };
 
-    private static class CustomDetailsGenerator implements DetailsGenerator {
+    private class CustomDetailsGenerator implements DetailsGenerator {
 
-        private final Map<Integer, ComponentConnector> indexToDetailsMap = new HashMap<Integer, ComponentConnector>();
+        private final Map<String, ComponentConnector> idToDetailsMap = new HashMap<String, ComponentConnector>();
+        private final Map<String, Integer> idToRowIndex = new HashMap<String, Integer>();
 
         @Override
-        @SuppressWarnings("boxing")
         public Widget getDetails(int rowIndex) {
-            ComponentConnector componentConnector = indexToDetailsMap
-                    .get(rowIndex);
-            if (componentConnector != null) {
-                return componentConnector.getWidget();
-            } else {
-                return null;
-            }
-        }
-
-        public void setDetailsConnectorChanges(
-                Set<DetailsConnectorChange> changes) {
-            /*
-             * To avoid overwriting connectors while moving them about, we'll
-             * take all the affected connectors, first all remove those that are
-             * removed or moved, then we add back those that are moved or added.
-             */
+            JsonObject row = getWidget().getDataSource().getRow(rowIndex);
 
-            /* Remove moved/removed connectors from bookkeeping */
-            for (DetailsConnectorChange change : changes) {
-                Integer oldIndex = change.getOldIndex();
-                Connector removedConnector = indexToDetailsMap.remove(oldIndex);
-
-                Connector connector = change.getConnector();
-                assert removedConnector == null || connector == null
-                        || removedConnector.equals(connector) : "Index "
-                        + oldIndex + " points to " + removedConnector
-                        + " while " + connector + " was expected";
-            }
-
-            /* Add moved/added connectors to bookkeeping */
-            for (DetailsConnectorChange change : changes) {
-                Integer newIndex = change.getNewIndex();
-                ComponentConnector connector = (ComponentConnector) change
-                        .getConnector();
-
-                if (connector != null) {
-                    assert newIndex != null : "An existing connector has a missing new index.";
-
-                    ComponentConnector prevConnector = indexToDetailsMap.put(
-                            newIndex, connector);
-
-                    assert prevConnector == null : "Connector collision at index "
-                            + newIndex
-                            + " between old "
-                            + prevConnector
-                            + " and new " + connector;
-                }
+            if (!row.hasKey(GridState.JSONKEY_DETAILS_VISIBLE)
+                    || row.getString(GridState.JSONKEY_DETAILS_VISIBLE)
+                            .isEmpty()) {
+                return null;
             }
-        }
-    }
-
-    @SuppressWarnings("boxing")
-    private static class DetailsConnectorFetcher implements DeferredWorker {
 
-        private static final int FETCH_TIMEOUT_MS = 5000;
+            String id = row.getString(GridState.JSONKEY_DETAILS_VISIBLE);
+            ComponentConnector componentConnector = idToDetailsMap.get(id);
+            idToRowIndex.put(id, rowIndex);
 
-        public interface Listener {
-            void fetchHasBeenScheduled(int id);
-
-            void fetchHasReturned(int id);
+            return componentConnector.getWidget();
         }
 
-        /** A flag making sure that we don't call scheduleFinally many times. */
-        private boolean fetcherHasBeenCalled = false;
-
-        /** A rolling counter for unique values. */
-        private int detailsFetchCounter = 0;
-
-        /** A collection that tracks the amount of requests currently underway. */
-        private Set<Integer> pendingFetches = new HashSet<Integer>(5);
-
-        private final ScheduledCommand lazyDetailsFetcher = new ScheduledCommand() {
-            @Override
-            public void execute() {
-                int currentFetchId = detailsFetchCounter++;
-                pendingFetches.add(currentFetchId);
-                rpc.sendDetailsComponents(currentFetchId);
-                fetcherHasBeenCalled = false;
-
-                if (listener != null) {
-                    listener.fetchHasBeenScheduled(currentFetchId);
+        public void updateConnectorHierarchy(List<ServerConnector> children) {
+            Set<String> connectorIds = new HashSet<String>();
+            for (ServerConnector child : children) {
+                if (child instanceof ComponentConnector) {
+                    connectorIds.add(child.getConnectorId());
+                    idToDetailsMap.put(child.getConnectorId(),
+                            (ComponentConnector) child);
                 }
-
-                assert assertRequestDoesNotTimeout(currentFetchId);
-            }
-        };
-
-        private DetailsConnectorFetcher.Listener listener = null;
-
-        private final GridServerRpc rpc;
-
-        public DetailsConnectorFetcher(GridServerRpc rpc) {
-            assert rpc != null : "RPC was null";
-            this.rpc = rpc;
-        }
-
-        public void schedule() {
-            if (!fetcherHasBeenCalled) {
-                Scheduler.get().scheduleFinally(lazyDetailsFetcher);
-                fetcherHasBeenCalled = true;
             }
-        }
 
-        public void responseReceived(int fetchId) {
-
-            if (fetchId < 0) {
-                /* Ignore negative fetchIds (they're pushed, not fetched) */
-                return;
+            Set<String> removedDetails = new HashSet<String>();
+            for (Entry<String, ComponentConnector> entry : idToDetailsMap
+                    .entrySet()) {
+                ComponentConnector connector = entry.getValue();
+                String id = connector.getConnectorId();
+                if (!connectorIds.contains(id)) {
+                    removedDetails.add(entry.getKey());
+                    if (idToRowIndex.containsKey(id)) {
+                        getWidget().setDetailsVisible(idToRowIndex.get(id),
+                                false);
+                    }
+                }
             }
 
-            boolean success = pendingFetches.remove(fetchId);
-            assert success : "Received a response with an unidentified fetch id";
-
-            if (listener != null) {
-                listener.fetchHasReturned(fetchId);
+            for (String id : removedDetails) {
+                idToDetailsMap.remove(id);
+                idToRowIndex.remove(id);
             }
         }
-
-        @Override
-        public boolean isWorkPending() {
-            return fetcherHasBeenCalled || !pendingFetches.isEmpty();
-        }
-
-        private boolean assertRequestDoesNotTimeout(final int fetchId) {
-            /*
-             * This method will not be compiled without asserts enabled. This
-             * only makes sure that any request does not time out.
-             * 
-             * TODO Should this be an explicit check? Is it worth the overhead?
-             */
-            new Timer() {
-                @Override
-                public void run() {
-                    assert !pendingFetches.contains(fetchId) : "Fetch id "
-                            + fetchId + " timed out.";
-                }
-            }.schedule(FETCH_TIMEOUT_MS);
-            return true;
-        }
-
-        public void setListener(DetailsConnectorFetcher.Listener listener) {
-            // if more are needed, feel free to convert this into a collection.
-            this.listener = listener;
-        }
     }
 
     /**
-     * The functionality that makes sure that the scroll position is still kept
-     * up-to-date even if more details are being fetched lazily.
+     * Class for handling scrolling issues with open details.
+     * 
+     * @since
+     * @author Vaadin Ltd
      */
-    private class LazyDetailsScrollAdjuster implements DeferredWorker {
+    private class LazyDetailsScroller implements DeferredWorker {
 
-        private static final int SCROLL_TO_END_ID = -2;
-        private static final int NO_SCROLL_SCHEDULED = -1;
-
-        private class ScrollStopChecker implements DeferredWorker {
-            private final ScheduledCommand checkCommand = new ScheduledCommand() {
-                @Override
-                public void execute() {
-                    isScheduled = false;
-                    if (queuedFetches.isEmpty()) {
-                        currentRow = NO_SCROLL_SCHEDULED;
-                        destination = null;
-                    }
-                }
-            };
-
-            private boolean isScheduled = false;
-
-            public void schedule() {
-                if (isScheduled) {
-                    return;
-                }
-                Scheduler.get().scheduleDeferred(checkCommand);
-                isScheduled = true;
-            }
-
-            @Override
-            public boolean isWorkPending() {
-                return isScheduled;
-            }
-        }
-
-        private DetailsConnectorFetcher.Listener fetcherListener = new DetailsConnectorFetcher.Listener() {
-            @Override
-            @SuppressWarnings("boxing")
-            public void fetchHasBeenScheduled(int id) {
-                if (currentRow != NO_SCROLL_SCHEDULED) {
-                    queuedFetches.add(id);
-                }
-            }
+        /* Timer value tested to work in our test cluster with slow IE8s. */
+        private static final int DISABLE_LAZY_SCROLL_TIMEOUT = 1500;
 
+        /*
+         * Cancels details opening scroll after timeout. Avoids any unexpected
+         * scrolls via details opening.
+         */
+        private Timer disableScroller = new Timer() {
             @Override
-            @SuppressWarnings("boxing")
-            public void fetchHasReturned(int id) {
-                if (currentRow == NO_SCROLL_SCHEDULED
-                        || queuedFetches.isEmpty()) {
-                    return;
-                }
-
-                queuedFetches.remove(id);
-                if (currentRow == SCROLL_TO_END_ID) {
-                    getWidget().scrollToEnd();
-                } else {
-                    getWidget().scrollToRow(currentRow, destination);
-                }
-
-                /*
-                 * Schedule a deferred call whether we should stop adjusting for
-                 * scrolling.
-                 * 
-                 * This is done deferredly just because we can't be absolutely
-                 * certain whether this most recent scrolling won't cascade into
-                 * further lazy details loading (perhaps deferredly).
-                 */
-                scrollStopChecker.schedule();
+            public void run() {
+                targetRow = -1;
             }
         };
 
-        private int currentRow = NO_SCROLL_SCHEDULED;
-        private final Set<Integer> queuedFetches = new HashSet<Integer>();
-        private final ScrollStopChecker scrollStopChecker = new ScrollStopChecker();
-        private ScrollDestination destination;
-
-        public LazyDetailsScrollAdjuster() {
-            detailsConnectorFetcher.setListener(fetcherListener);
-        }
+        private Integer targetRow = -1;
+        private ScrollDestination destination = null;
 
-        public void adjustForEnd() {
-            currentRow = SCROLL_TO_END_ID;
+        public void scrollToRow(Integer row, ScrollDestination dest) {
+            targetRow = row;
+            destination = dest;
+            disableScroller.schedule(DISABLE_LAZY_SCROLL_TIMEOUT);
         }
 
-        public void adjustFor(int row, ScrollDestination destination) {
-            currentRow = row;
-            this.destination = destination;
+        /**
+         * Inform LazyDetailsScroller that a details row has opened on a row.
+         * 
+         * @since
+         * @param rowIndex
+         *            index of row with details now open
+         */
+        public void detailsOpened(int rowIndex) {
+            if (targetRow == rowIndex) {
+                getWidget().scrollToRow(targetRow, destination);
+                disableScroller.run();
+            }
         }
 
         @Override
         public boolean isWorkPending() {
-            return currentRow != NO_SCROLL_SCHEDULED
-                    || !queuedFetches.isEmpty()
-                    || scrollStopChecker.isWorkPending();
+            return disableScroller.isRunning();
         }
     }
 
@@ -732,39 +583,46 @@ public class GridConnector extends AbstractHasComponentsConnector implements
     private final CustomDetailsGenerator customDetailsGenerator = new CustomDetailsGenerator();
     private final CustomStyleGenerator styleGenerator = new CustomStyleGenerator();
 
-    private final DetailsConnectorFetcher detailsConnectorFetcher = new DetailsConnectorFetcher(
-            getRpcProxy(GridServerRpc.class));
-
     private final DetailsListener detailsListener = new DetailsListener() {
         @Override
         public void reapplyDetailsVisibility(final int rowIndex,
                 final JsonObject row) {
-            Scheduler.get().scheduleDeferred(new ScheduledCommand() {
 
-                @Override
-                public void execute() {
-                    if (hasDetailsOpen(row)) {
-                        getWidget().setDetailsVisible(rowIndex, true);
-                        detailsConnectorFetcher.schedule();
-                    } else {
+            if (hasDetailsOpen(row)) {
+                // Command for opening details row.
+                ScheduledCommand openDetails = new ScheduledCommand() {
+                    @Override
+                    public void execute() {
+                        // Re-apply to force redraw.
                         getWidget().setDetailsVisible(rowIndex, false);
+                        getWidget().setDetailsVisible(rowIndex, true);
+                        lazyDetailsScroller.detailsOpened(rowIndex);
                     }
+                };
+
+                if (initialChange) {
+                    Scheduler.get().scheduleDeferred(openDetails);
+                } else {
+                    Scheduler.get().scheduleFinally(openDetails);
                 }
-            });
+            } else {
+                getWidget().setDetailsVisible(rowIndex, false);
+            }
         }
 
         private boolean hasDetailsOpen(JsonObject row) {
             return row.hasKey(GridState.JSONKEY_DETAILS_VISIBLE)
-                    && row.getBoolean(GridState.JSONKEY_DETAILS_VISIBLE);
-        }
-
-        @Override
-        public void closeDetails(int rowIndex) {
-            getWidget().setDetailsVisible(rowIndex, false);
+                    && row.getString(GridState.JSONKEY_DETAILS_VISIBLE) != null;
         }
     };
 
-    private final LazyDetailsScrollAdjuster lazyDetailsScrollAdjuster = new LazyDetailsScrollAdjuster();
+    private final LazyDetailsScroller lazyDetailsScroller = new LazyDetailsScroller();
+
+    /*
+     * Initially details need to behave a bit differently to allow some
+     * escalator magic.
+     */
+    private boolean initialChange;
 
     @Override
     @SuppressWarnings("unchecked")
@@ -799,11 +657,13 @@ public class GridConnector extends AbstractHasComponentsConnector implements
 
             @Override
             public void scrollToEnd() {
-                lazyDetailsScrollAdjuster.adjustForEnd();
                 Scheduler.get().scheduleFinally(new ScheduledCommand() {
                     @Override
                     public void execute() {
                         getWidget().scrollToEnd();
+                        // Scrolls further if details opens.
+                        lazyDetailsScroller.scrollToRow(dataSource.size() - 1,
+                                ScrollDestination.END);
                     }
                 });
             }
@@ -811,11 +671,12 @@ public class GridConnector extends AbstractHasComponentsConnector implements
             @Override
             public void scrollToRow(final int row,
                     final ScrollDestination destination) {
-                lazyDetailsScrollAdjuster.adjustFor(row, destination);
                 Scheduler.get().scheduleFinally(new ScheduledCommand() {
                     @Override
                     public void execute() {
                         getWidget().scrollToRow(row, destination);
+                        // Scrolls a bit further if details opens.
+                        lazyDetailsScroller.scrollToRow(row, destination);
                     }
                 });
             }
@@ -824,51 +685,6 @@ public class GridConnector extends AbstractHasComponentsConnector implements
             public void recalculateColumnWidths() {
                 getWidget().recalculateColumnWidths();
             }
-
-            @Override
-            @SuppressWarnings("boxing")
-            public void setDetailsConnectorChanges(
-                    Set<DetailsConnectorChange> connectorChanges, int fetchId) {
-                customDetailsGenerator
-                        .setDetailsConnectorChanges(connectorChanges);
-
-                List<DetailsConnectorChange> removedFirst = new ArrayList<DetailsConnectorChange>(
-                        connectorChanges);
-                Collections.sort(removedFirst,
-                        DetailsConnectorChange.REMOVED_FIRST_COMPARATOR);
-
-                // refresh moved/added details rows
-                for (DetailsConnectorChange change : removedFirst) {
-                    Integer oldIndex = change.getOldIndex();
-                    Integer newIndex = change.getNewIndex();
-
-                    assert oldIndex == null || oldIndex >= 0 : "Got an "
-                            + "invalid old index: " + oldIndex
-                            + " (connector: " + change.getConnector() + ")";
-                    assert newIndex == null || newIndex >= 0 : "Got an "
-                            + "invalid new index: " + newIndex
-                            + " (connector: " + change.getConnector() + ")";
-
-                    if (oldIndex != null) {
-                        /* Close the old/removed index */
-                        getWidget().setDetailsVisible(oldIndex, false);
-
-                        if (change.isShouldStillBeVisible()) {
-                            getWidget().setDetailsVisible(oldIndex, true);
-                        }
-                    }
-
-                    if (newIndex != null) {
-                        /*
-                         * Since the component was lazy loaded, we need to
-                         * refresh the details by toggling it.
-                         */
-                        getWidget().setDetailsVisible(newIndex, false);
-                        getWidget().setDetailsVisible(newIndex, true);
-                    }
-                }
-                detailsConnectorFetcher.responseReceived(fetchId);
-            }
         });
 
         getWidget().addSelectionHandler(internalSelectionChangeHandler);
@@ -953,17 +769,12 @@ public class GridConnector extends AbstractHasComponentsConnector implements
         layout();
     }
 
-    @Override
-    public void onUnregister() {
-        customDetailsGenerator.indexToDetailsMap.clear();
-
-        super.onUnregister();
-    }
-
     @Override
     public void onStateChanged(final StateChangeEvent stateChangeEvent) {
         super.onStateChanged(stateChangeEvent);
 
+        initialChange = stateChangeEvent.isInitialStateChange();
+
         // Column updates
         if (stateChangeEvent.hasPropertyChanged("columns")) {
 
@@ -1428,6 +1239,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements
     @Override
     public void onConnectorHierarchyChange(
             ConnectorHierarchyChangeEvent connectorHierarchyChangeEvent) {
+        customDetailsGenerator.updateConnectorHierarchy(getChildren());
     }
 
     public String getColumnId(Grid.Column<?, ?> column) {
@@ -1444,8 +1256,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements
 
     @Override
     public boolean isWorkPending() {
-        return detailsConnectorFetcher.isWorkPending()
-                || lazyDetailsScrollAdjuster.isWorkPending();
+        return lazyDetailsScroller.isWorkPending();
     }
 
     /**

client/src/com/vaadin/client/connectors/RpcDataSourceConnector.java

@@ -64,14 +64,6 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector {
          * @see GridState#JSONKEY_DETAILS_VISIBLE
          */
         void reapplyDetailsVisibility(int rowIndex, JsonObject row);
-
-        /**
-         * Closes details for a row.
-         * 
-         * @param rowIndex
-         *            the index of the row for which to close details
-         */
-        void closeDetails(int rowIndex);
     }
 
     public class RpcDataSource extends AbstractRemoteDataSource<JsonObject> {
@@ -221,11 +213,6 @@ public class RpcDataSourceConnector extends AbstractExtensionConnector {
                         rowData.get(i));
             }
         }
-
-        @Override
-        protected void onDropFromCache(int rowIndex) {
-            detailsListener.closeDetails(rowIndex);
-        }
     }
 
     private final RpcDataSource dataSource = new RpcDataSource();

client/src/com/vaadin/client/ui/VFilterSelect.java

@@ -65,6 +65,7 @@ import com.vaadin.client.BrowserInfo;
 import com.vaadin.client.ComponentConnector;
 import com.vaadin.client.ComputedStyle;
 import com.vaadin.client.ConnectorMap;
+import com.vaadin.client.DeferredWorker;
 import com.vaadin.client.Focusable;
 import com.vaadin.client.UIDL;
 import com.vaadin.client.VConsole;
@@ -90,7 +91,7 @@ import com.vaadin.shared.util.SharedUtil;
 public class VFilterSelect extends Composite implements Field, KeyDownHandler,
         KeyUpHandler, ClickHandler, FocusHandler, BlurHandler, Focusable,
         SubPartAware, HandlesAriaCaption, HandlesAriaInvalid,
-        HandlesAriaRequired {
+        HandlesAriaRequired, DeferredWorker {
 
     /**
      * Represents a suggestion in the suggestion popup box
@@ -417,7 +418,9 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
                 selectPrevPage();
 
             } else {
-                selectItem(menu.getItems().get(menu.getItems().size() - 1));
+                if (!menu.getItems().isEmpty()) {
+                    selectLastItem();
+                }
             }
         }
 
@@ -2185,11 +2188,15 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
 
     @Override
     public com.google.gwt.user.client.Element getSubPartElement(String subPart) {
-        if ("textbox".equals(subPart)) {
+        String[] parts = subPart.split("/");
+        if ("textbox".equals(parts[0])) {
             return tb.getElement();
-        } else if ("button".equals(subPart)) {
+        } else if ("button".equals(parts[0])) {
             return popupOpener.getElement();
-        } else if ("popup".equals(subPart) && suggestionPopup.isAttached()) {
+        } else if ("popup".equals(parts[0]) && suggestionPopup.isAttached()) {
+            if (parts.length == 2) {
+                return suggestionPopup.menu.getSubPartElement(parts[1]);
+            }
             return suggestionPopup.getElement();
         }
         return null;
@@ -2233,4 +2240,10 @@ public class VFilterSelect extends Composite implements Field, KeyDownHandler,
         selectPopupItemWhenResponseIsReceived = Select.NONE;
     }
 
+    @Override
+    public boolean isWorkPending() {
+        return waitingForFilteringResponse
+                || suggestionPopup.lazyPageScroller.isRunning();
+    }
+
 }

client/src/com/vaadin/client/ui/VOverlay.java

@@ -22,6 +22,8 @@ import java.util.logging.Logger;
 import com.google.gwt.animation.client.Animation;
 import com.google.gwt.aria.client.Roles;
 import com.google.gwt.core.client.JavaScriptObject;
+import com.google.gwt.core.client.Scheduler;
+import com.google.gwt.core.client.Scheduler.ScheduledCommand;
 import com.google.gwt.dom.client.Document;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.IFrameElement;
@@ -471,7 +473,17 @@ public class VOverlay extends PopupPanel implements CloseHandler<PopupPanel> {
         if (isAnimationEnabled()) {
             new ResizeAnimation().run(POPUP_PANEL_ANIMATION_DURATION);
         } else {
-            positionOrSizeUpdated(1.0);
+            if (BrowserInfo.get().isIE8()) {
+                Scheduler.get().scheduleFinally(new ScheduledCommand() {
+
+                    @Override
+                    public void execute() {
+                        positionOrSizeUpdated(1.0);
+                    }
+                });
+            } else {
+                positionOrSizeUpdated(1.0);
+            }
         }
         current = null;
     }

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

@@ -3628,6 +3628,12 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                     }
                 } else {
                     c.setText(caption);
+                    if (BrowserInfo.get().isIE10()) {
+                        // IE10 can some times define min-height to include
+                        // padding when setting the text...
+                        // See https://dev.vaadin.com/ticket/15169
+                        WidgetUtil.forceIERedraw(c.getElement());
+                    }
                 }
 
                 c.setSorted(false);

client/src/com/vaadin/client/ui/VTree.java

@@ -179,7 +179,7 @@ public class VTree extends FocusElementPanel implements VHasDropHandler,
 
                 @Override
                 public void execute() {
-                    Util.notifyParentOfSizeChange(VTree.this, true);
+                    doLayout();
                 }
 
             });
@@ -969,7 +969,7 @@ public class VTree extends FocusElementPanel implements VHasDropHandler,
             open = state;
 
             if (!rendering) {
-                Util.notifyParentOfSizeChange(VTree.this, false);
+                doLayout();
             }
         }
 
@@ -2239,4 +2239,15 @@ public class VTree extends FocusElementPanel implements VHasDropHandler,
             com.google.gwt.user.client.Element captionElement) {
         AriaHelper.bindCaption(body, captionElement);
     }
+
+    /**
+     * Tell LayoutManager that a layout is needed later for this VTree
+     */
+    private void doLayout() {
+        // IE8 needs a hack to measure the tree again after update
+        WidgetUtil.forceIE8Redraw(getElement());
+
+        // This calls LayoutManager setNeedsMeasure and layoutNow
+        Util.notifyParentOfSizeChange(this, false);
+    }
 }

client/src/com/vaadin/client/ui/tree/TreeConnector.java

@@ -27,8 +27,8 @@ import com.vaadin.client.BrowserInfo;
 import com.vaadin.client.Paintable;
 import com.vaadin.client.TooltipInfo;
 import com.vaadin.client.UIDL;
-import com.vaadin.client.WidgetUtil;
 import com.vaadin.client.VConsole;
+import com.vaadin.client.WidgetUtil;
 import com.vaadin.client.communication.StateChangeEvent;
 import com.vaadin.client.ui.AbstractComponentConnector;
 import com.vaadin.client.ui.VTree;
@@ -62,6 +62,10 @@ public class TreeConnector extends AbstractComponentConnector implements
 
         if (uidl.hasAttribute("partialUpdate")) {
             handleUpdate(uidl);
+
+            // IE8 needs a hack to measure the tree again after update
+            WidgetUtil.forceIE8Redraw(getWidget().getElement());
+
             getWidget().rendering = false;
             return;
         }

scripts/GenerateStagingReport.py

@@ -1,6 +1,7 @@
 #coding=UTF-8
 
 from BuildArchetypes import archetypes, getDeploymentContext
+from BuildDemos import demos
 import argparse, cgi
 
 parser = argparse.ArgumentParser(description="Build report generator")

server/src/com/vaadin/data/RpcDataProviderExtension.java

@@ -24,7 +24,6 @@ import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Set;
 
 import com.google.gwt.thirdparty.guava.common.collect.BiMap;
@@ -45,11 +44,9 @@ import com.vaadin.server.AbstractExtension;
 import com.vaadin.server.ClientConnector;
 import com.vaadin.shared.data.DataProviderRpc;
 import com.vaadin.shared.data.DataRequestRpc;
-import com.vaadin.shared.ui.grid.DetailsConnectorChange;
 import com.vaadin.shared.ui.grid.GridClientRpc;
 import com.vaadin.shared.ui.grid.GridState;
 import com.vaadin.shared.ui.grid.Range;
-import com.vaadin.shared.util.SharedUtil;
 import com.vaadin.ui.Component;
 import com.vaadin.ui.Grid;
 import com.vaadin.ui.Grid.Column;
@@ -111,15 +108,13 @@ public class RpcDataProviderExtension extends AbstractExtension {
             }
 
             for (Object itemId : itemsRemoved) {
-                detailComponentManager.destroyDetails(itemId);
                 itemIdToKey.remove(itemId);
             }
 
             for (Object itemId : itemSet) {
                 itemIdToKey.put(itemId, getKey(itemId));
-                if (detailComponentManager.visibleDetails.contains(itemId)) {
-                    detailComponentManager.createDetails(itemId,
-                            indexOf(itemId));
+                if (visibleDetails.contains(itemId)) {
+                    detailComponentManager.createDetails(itemId);
                 }
             }
         }
@@ -603,7 +598,8 @@ public class RpcDataProviderExtension extends AbstractExtension {
      * @since 7.5.0
      * @author Vaadin Ltd
      */
-    public static final class DetailComponentManager implements DataGenerator {
+    // TODO this should probably be a static nested class
+    public final class DetailComponentManager implements DataGenerator {
         /**
          * This map represents all the components that have been requested for
          * each item id.
@@ -611,9 +607,8 @@ public class RpcDataProviderExtension extends AbstractExtension {
          * Normally this map is consistent with what is displayed in the
          * component hierarchy (and thus the DOM). The only time this map is out
          * of sync with the DOM is between the any calls to
-         * {@link #createDetails(Object, int)} or
-         * {@link #destroyDetails(Object)}, and
-         * {@link GridClientRpc#setDetailsConnectorChanges(Set)}.
+         * {@link #createDetails(Object)} or {@link #destroyDetails(Object)},
+         * and {@link GridClientRpc#setDetailsConnectorChanges(Set)}.
          * <p>
          * This is easily checked: if {@link #unattachedComponents} is
          * {@link Collection#isEmpty() empty}, then this field is consistent
@@ -622,40 +617,11 @@ public class RpcDataProviderExtension extends AbstractExtension {
         private final Map<Object, Component> visibleDetailsComponents = Maps
                 .newHashMap();
 
-        /** A lookup map for which row contains which details component. */
-        private BiMap<Integer, Component> rowIndexToDetails = HashBiMap
-                .create();
-
-        /**
-         * A copy of {@link #rowIndexToDetails} from its last stable state. Used
-         * for creating a diff against {@link #rowIndexToDetails}.
-         * 
-         * @see #getAndResetConnectorChanges()
-         */
-        private BiMap<Integer, Component> prevRowIndexToDetails = HashBiMap
-                .create();
-
-        /**
-         * A set keeping track on components that have been created, but not
-         * attached. They should be attached at some later point in time.
-         * <p>
-         * This isn't strictly requried, but it's a handy explicit log. You
-         * could find out the same thing by taking out all the other components
-         * and checking whether Grid is their parent or not.
-         */
-        private final Set<Component> unattachedComponents = Sets.newHashSet();
-
         /**
          * Keeps tabs on all the details that did not get a component during
-         * {@link #createDetails(Object, int)}.
-         */
-        private final Map<Object, Integer> emptyDetails = Maps.newHashMap();
-
-        /**
-         * This map represents all the details that are user-defined as visible.
-         * This does not reflect the status in the DOM.
+         * {@link #createDetails(Object)}.
          */
-        private Set<Object> visibleDetails = new HashSet<Object>();
+        private final Set<Object> emptyDetails = Sets.newHashSet();
 
         private Grid grid;
 
@@ -669,19 +635,16 @@ public class RpcDataProviderExtension extends AbstractExtension {
          *            the item id for which to create the details component.
          *            Assumed not <code>null</code> and that a component is not
          *            currently present for this item previously
-         * @param rowIndex
-         *            the row index for {@code itemId}
          * @throws IllegalStateException
          *             if the current details generator provides a component
          *             that was manually attached, or if the same instance has
          *             already been provided
          */
-        public void createDetails(Object itemId, int rowIndex)
-                throws IllegalStateException {
+        public void createDetails(Object itemId) throws IllegalStateException {
             assert itemId != null : "itemId was null";
-            Integer newRowIndex = Integer.valueOf(rowIndex);
 
-            if (visibleDetailsComponents.containsKey(itemId)) {
+            if (visibleDetailsComponents.containsKey(itemId)
+                    || emptyDetails.contains(itemId)) {
                 // Don't overwrite existing components
                 return;
             }
@@ -692,58 +655,26 @@ public class RpcDataProviderExtension extends AbstractExtension {
             DetailsGenerator detailsGenerator = grid.getDetailsGenerator();
             Component details = detailsGenerator.getDetails(rowReference);
             if (details != null) {
-                String generatorName = detailsGenerator.getClass().getName();
                 if (details.getParent() != null) {
-                    throw new IllegalStateException(generatorName
+                    String name = detailsGenerator.getClass().getName();
+                    throw new IllegalStateException(name
                             + " generated a details component that already "
-                            + "was attached. (itemId: " + itemId + ", row: "
-                            + rowIndex + ", component: " + details);
-                }
-
-                if (rowIndexToDetails.containsValue(details)) {
-                    throw new IllegalStateException(generatorName
-                            + " provided a details component that already "
-                            + "exists in Grid. (itemId: " + itemId + ", row: "
-                            + rowIndex + ", component: " + details);
+                            + "was attached. (itemId: " + itemId
+                            + ", component: " + details + ")");
                 }
 
                 visibleDetailsComponents.put(itemId, details);
-                rowIndexToDetails.put(newRowIndex, details);
-                unattachedComponents.add(details);
 
-                assert !emptyDetails.containsKey(itemId) : "Bookeeping thinks "
+                details.setParent(grid);
+                grid.markAsDirty();
+
+                assert !emptyDetails.contains(itemId) : "Bookeeping thinks "
                         + "itemId is empty even though we just created a "
                         + "component for it (" + itemId + ")";
             } else {
-                assert assertItemIdHasNotMovedAndNothingIsOverwritten(itemId,
-                        newRowIndex);
-                emptyDetails.put(itemId, newRowIndex);
-            }
-
-            /*
-             * Don't attach the components here. It's done by
-             * GridServerRpc.sendDetailsComponents in a separate roundtrip.
-             */
-        }
-
-        private boolean assertItemIdHasNotMovedAndNothingIsOverwritten(
-                Object itemId, Integer newRowIndex) {
-
-            Integer oldRowIndex = emptyDetails.get(itemId);
-            if (!SharedUtil.equals(oldRowIndex, newRowIndex)) {
-
-                assert !emptyDetails.containsKey(itemId) : "Unexpected "
-                        + "change of empty details row index for itemId "
-                        + itemId + " from " + oldRowIndex + " to "
-                        + newRowIndex;
-
-                assert !emptyDetails.containsValue(newRowIndex) : "Bookkeeping"
-                        + " already had another itemId for this empty index "
-                        + "(index: " + newRowIndex + ", new itemId: " + itemId
-                        + ")";
+                emptyDetails.add(itemId);
             }
 
-            return true;
         }
 
         /**
@@ -764,8 +695,6 @@ public class RpcDataProviderExtension extends AbstractExtension {
                 return;
             }
 
-            rowIndexToDetails.inverse().remove(removedComponent);
-
             removedComponent.setParent(null);
             grid.markAsDirty();
         }
@@ -781,81 +710,12 @@ public class RpcDataProviderExtension extends AbstractExtension {
         public Collection<Component> getComponents() {
             Set<Component> components = new HashSet<Component>(
                     visibleDetailsComponents.values());
-            components.removeAll(unattachedComponents);
             return components;
         }
 
-        /**
-         * Gets information on how the connectors have changed.
-         * <p>
-         * This method only returns the changes that have been made between two
-         * calls of this method. I.e. Calling this method once will reset the
-         * state for the next state.
-         * <p>
-         * Used internally by the Grid object.
-         * 
-         * @return information on how the connectors have changed
-         */
-        public Set<DetailsConnectorChange> getAndResetConnectorChanges() {
-            Set<DetailsConnectorChange> changes = new HashSet<DetailsConnectorChange>();
-
-            // populate diff with added/changed
-            for (Entry<Integer, Component> entry : rowIndexToDetails.entrySet()) {
-                Component component = entry.getValue();
-                assert component != null : "rowIndexToDetails contains a null component";
-
-                Integer newIndex = entry.getKey();
-                Integer oldIndex = prevRowIndexToDetails.inverse().get(
-                        component);
-
-                /*
-                 * only attach components. Detaching already happened in
-                 * destroyDetails.
-                 */
-                if (newIndex != null && oldIndex == null) {
-                    assert unattachedComponents.contains(component) : "unattachedComponents does not contain component for index "
-                            + newIndex + " (" + component + ")";
-                    component.setParent(grid);
-                    unattachedComponents.remove(component);
-                }
-
-                if (!SharedUtil.equals(oldIndex, newIndex)) {
-                    changes.add(new DetailsConnectorChange(component, oldIndex,
-                            newIndex, emptyDetails.containsKey(component)));
-                }
-            }
-
-            // populate diff with removed
-            for (Entry<Integer, Component> entry : prevRowIndexToDetails
-                    .entrySet()) {
-                Integer oldIndex = entry.getKey();
-                Component component = entry.getValue();
-                Integer newIndex = rowIndexToDetails.inverse().get(component);
-                if (newIndex == null) {
-                    changes.add(new DetailsConnectorChange(null, oldIndex,
-                            null, emptyDetails.containsValue(oldIndex)));
-                }
-            }
-
-            // reset diff map
-            prevRowIndexToDetails = HashBiMap.create(rowIndexToDetails);
-
-            return changes;
-        }
-
         public void refresh(Object itemId) {
-            Component component = visibleDetailsComponents.get(itemId);
-            Integer rowIndex = null;
-            if (component != null) {
-                rowIndex = rowIndexToDetails.inverse().get(component);
-                destroyDetails(itemId);
-            } else {
-                rowIndex = emptyDetails.remove(itemId);
-            }
-
-            assert rowIndex != null : "Given itemId does not map to an "
-                    + "existing detail row (" + itemId + ")";
-            createDetails(itemId, rowIndex.intValue());
+            destroyDetails(itemId);
+            createDetails(itemId);
         }
 
         void setGrid(Grid grid) {
@@ -947,17 +807,13 @@ public class RpcDataProviderExtension extends AbstractExtension {
                     listener.removeListener();
                 }
 
-                // Wipe clean all details.
-                HashSet<Object> detailItemIds = new HashSet<Object>(
-                        detailComponentManager.visibleDetailsComponents
-                                .keySet());
-                for (Object itemId : detailItemIds) {
-                    detailComponentManager.destroyDetails(itemId);
-                }
-
                 listeners.clear();
                 activeRowHandler.activeRange = Range.withLength(0, 0);
 
+                for (Object itemId : visibleDetails) {
+                    detailComponentManager.destroyDetails(itemId);
+                }
+
                 /* Mark as dirty to push changes in beforeClientResponse */
                 bareItemSetTriggeredSizeChange = true;
                 markAsDirty();
@@ -982,6 +838,13 @@ public class RpcDataProviderExtension extends AbstractExtension {
     /** Size possibly changed with a bare ItemSetChangeEvent */
     private boolean bareItemSetTriggeredSizeChange = false;
 
+    /**
+     * This map represents all the details that are user-defined as visible.
+     * This does not reflect the status in the DOM.
+     */
+    // TODO this should probably be inside DetailComponentManager
+    private Set<Object> visibleDetails = new HashSet<Object>();
+
     private final DetailComponentManager detailComponentManager = new DetailComponentManager();
 
     private Set<DataGenerator> dataGenerators = new LinkedHashSet<DataGenerator>();
@@ -1233,15 +1096,11 @@ public class RpcDataProviderExtension extends AbstractExtension {
 
     private void internalUpdateRowData(Object itemId) {
         int index = container.indexOfId(itemId);
-        if (index >= 0) {
+        if (activeRowHandler.activeRange.contains(index)) {
             JsonValue row = getRowData(getGrid().getColumns(), itemId);
             JsonArray rowArray = Json.createArray();
             rowArray.set(0, row);
             rpc.setRowData(index, rowArray);
-
-            if (isDetailsVisible(itemId)) {
-                detailComponentManager.createDetails(itemId, index);
-            }
         }
     }
 
@@ -1318,37 +1177,21 @@ public class RpcDataProviderExtension extends AbstractExtension {
      *            hide
      */
     public void setDetailsVisible(Object itemId, boolean visible) {
-        final boolean modified;
-
         if (visible) {
-            modified = detailComponentManager.visibleDetails.add(itemId);
+            visibleDetails.add(itemId);
 
             /*
-             * We don't want to create the component here, since the component
-             * might be out of view, and thus we don't know where the details
-             * should end up on the client side. This is also a great thing to
-             * optimize away, so that in case a lot of things would be opened at
-             * once, a huge chunk of data doesn't get sent over immediately.
+             * This might be an issue with a huge number of open rows, but as of
+             * now this works in most of the cases.
              */
-
+            detailComponentManager.createDetails(itemId);
         } else {
-            modified = detailComponentManager.visibleDetails.remove(itemId);
+            visibleDetails.remove(itemId);
 
-            /*
-             * Here we can try to destroy the component no matter what. The
-             * component has been removed and should be detached from the
-             * component hierarchy. The details row will be closed on the client
-             * side automatically.
-             */
             detailComponentManager.destroyDetails(itemId);
         }
 
-        int rowIndex = indexOf(itemId);
-        boolean modifiedRowIsActive = activeRowHandler.activeRange
-                .contains(rowIndex);
-        if (modified && modifiedRowIsActive) {
-            updateRowData(itemId);
-        }
+        updateRowData(itemId);
     }
 
     /**
@@ -1362,7 +1205,7 @@ public class RpcDataProviderExtension extends AbstractExtension {
      *         visible in the DOM
      */
     public boolean isDetailsVisible(Object itemId) {
-        return detailComponentManager.visibleDetails.contains(itemId);
+        return visibleDetails.contains(itemId);
     }
 
     /**
@@ -1371,21 +1214,12 @@ public class RpcDataProviderExtension extends AbstractExtension {
      * @since 7.5.0
      */
     public void refreshDetails() {
-        for (Object itemId : ImmutableSet
-                .copyOf(detailComponentManager.visibleDetails)) {
+        for (Object itemId : ImmutableSet.copyOf(visibleDetails)) {
             detailComponentManager.refresh(itemId);
+            updateRowData(itemId);
         }
     }
 
-    private int indexOf(Object itemId) {
-        /*
-         * It would be great if we could optimize this method away, since the
-         * normal usage of Grid doesn't need any indices to be known. It was
-         * already optimized away once, maybe we can do away with these as well.
-         */
-        return container.indexOfId(itemId);
-    }
-
     /**
      * Gets the detail component manager for this data provider
      * 
@@ -1395,14 +1229,4 @@ public class RpcDataProviderExtension extends AbstractExtension {
     public DetailComponentManager getDetailComponentManager() {
         return detailComponentManager;
     }
-
-    @Override
-    public void detach() {
-        for (Object itemId : ImmutableSet
-                .copyOf(detailComponentManager.visibleDetails)) {
-            detailComponentManager.destroyDetails(itemId);
-        }
-
-        super.detach();
-    }
 }

server/src/com/vaadin/ui/Grid.java

@@ -4183,13 +4183,6 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
                 }
             }
 
-            @Override
-            public void sendDetailsComponents(int fetchId) {
-                getRpcProxy(GridClientRpc.class).setDetailsConnectorChanges(
-                        detailComponentManager.getAndResetConnectorChanges(),
-                        fetchId);
-            }
-
             @Override
             public void editorOpen(String rowKey) {
                 fireEvent(new EditorOpenEvent(Grid.this, getKeyMapper()
@@ -6558,8 +6551,6 @@ public class Grid extends AbstractFocusable implements SelectionNotifier,
         this.detailsGenerator = detailsGenerator;
 
         datasourceExtension.refreshDetails();
-        getRpcProxy(GridClientRpc.class).setDetailsConnectorChanges(
-                detailComponentManager.getAndResetConnectorChanges(), -1);
     }
 
     /**

shared/src/com/vaadin/shared/ui/grid/GridClientRpc.java

@@ -15,8 +15,6 @@
  */
 package com.vaadin.shared.ui.grid;
 
-import java.util.Set;
-
 import com.vaadin.shared.communication.ClientRpc;
 
 /**
@@ -57,19 +55,4 @@ public interface GridClientRpc extends ClientRpc {
      * Command client Grid to recalculate column widths.
      */
     public void recalculateColumnWidths();
-
-    /**
-     * Informs the GridConnector on how the indexing of details connectors has
-     * changed.
-     * 
-     * @since 7.5.0
-     * @param connectorChanges
-     *            the indexing changes of details connectors
-     * @param fetchId
-     *            the id of the request for fetching the changes. A negative
-     *            number indicates a push (not requested by the client side)
-     */
-    public void setDetailsConnectorChanges(
-            Set<DetailsConnectorChange> connectorChanges, int fetchId);
-
 }

shared/src/com/vaadin/shared/ui/grid/GridServerRpc.java

@@ -85,23 +85,6 @@ public interface GridServerRpc extends ServerRpc {
     void columnsReordered(List<String> newColumnOrder,
             List<String> oldColumnOrder);
 
-    /**
-     * This is a trigger for Grid to send whatever has changed regarding the
-     * details components.
-     * <p>
-     * The components can't be sent eagerly, since they are generated as a side
-     * effect in
-     * {@link com.vaadin.data.RpcDataProviderExtension#beforeClientResponse(boolean)}
-     * , and that is too late to change the hierarchy. So we need this
-     * round-trip to work around that limitation.
-     * 
-     * @since 7.5.0
-     * @param fetchId
-     *            an unique identifier for the request
-     * @see com.vaadin.ui.Grid#setDetailsVisible(Object, boolean)
-     */
-    void sendDetailsComponents(int fetchId);
-
     /**
      * Informs the server that the column's visibility has been changed.
      * 

uitest/src/com/vaadin/tests/components/combobox/ComboBoxEmptyItemsKeyboardNavigation.java

@@ -0,0 +1,15 @@
+package com.vaadin.tests.components.combobox;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.ComboBox;
+
+public class ComboBoxEmptyItemsKeyboardNavigation extends AbstractTestUI {
+    @Override
+    protected void setup(VaadinRequest request) {
+        ComboBox comboBox = new ComboBox();
+        comboBox.addItems("foo", "bar");
+
+        addComponent(comboBox);
+    }
+}

uitest/src/com/vaadin/tests/components/combobox/ComboBoxEmptyItemsKeyboardNavigationTest.java

@@ -0,0 +1,30 @@
+package com.vaadin.tests.components.combobox;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.collection.IsEmptyCollection.empty;
+
+import java.util.List;
+
+import org.junit.Test;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.By;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import com.vaadin.tests.tb3.newelements.ComboBoxElement;
+
+public class ComboBoxEmptyItemsKeyboardNavigationTest extends MultiBrowserTest {
+
+    @Test
+    public void navigatingUpOnAnEmptyMenuDoesntThrowErrors() {
+        setDebug(true);
+        openTestURL();
+
+        ComboBoxElement combobox = $(ComboBoxElement.class).first();
+        combobox.sendKeys("a", Keys.ARROW_UP);
+
+        List<WebElement> errors = findElements(By.className("SEVERE"));
+
+        assertThat(errors, empty());
+    }
+}

uitest/src/com/vaadin/tests/components/combobox/ComboBoxLargeIconsTest.java

@@ -0,0 +1,58 @@
+package com.vaadin.tests.components.combobox;
+
+import org.junit.Test;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.interactions.Actions;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.NativeSelectElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+import com.vaadin.tests.tb3.newelements.ComboBoxElement;
+
+public class ComboBoxLargeIconsTest extends MultiBrowserTest {
+    @Override
+    protected Class<?> getUIClass() {
+        return com.vaadin.tests.components.combobox.Comboboxes.class;
+    }
+
+    @Test
+    public void testComboBoxIcons() throws Exception {
+        openTestURL();
+        NativeSelectElement iconSelect = $(NativeSelectElement.class).first();
+        iconSelect.selectByText("16x16");
+
+        ComboBoxElement cb = $(ComboBoxElement.class).caption(
+                "Undefined wide select with 50 items").first();
+        cb.openPopup();
+        compareScreen("icons-16x16-page1");
+        cb.openNextPage();
+        compareScreen("icons-16x16-page2");
+        cb.findElement(By.vaadin("#popup/item0")).click();
+        compareScreen("icons-16x16-selected-1-3-5-9");
+
+        iconSelect.selectByText("32x32");
+        cb.openPopup();
+        compareScreen("icons-32x32-page2");
+
+        // Closes the popup
+        cb.openPopup();
+
+        iconSelect.selectByText("64x64");
+
+        ComboBoxElement pageLength0cb = $(ComboBoxElement.class).caption(
+                "Pagelength 0").first();
+        pageLength0cb.openPopup();
+        pageLength0cb.findElement(By.vaadin("#popup/item1")).click();
+
+        ComboBoxElement cb200px = $(ComboBoxElement.class).caption(
+                "200px wide select with 50 items").first();
+        cb200px.openPopup();
+        cb200px.findElement(By.vaadin("#popup/item1")).click();
+
+        ComboBoxElement cb150px = $(ComboBoxElement.class).caption(
+                "150px wide select with 5 items").first();
+        new Actions(driver).sendKeys(cb150px, Keys.DOWN).perform();
+
+        compareScreen("icons-64x64-page1-highlight-first");
+    }
+}

uitest/src/com/vaadin/tests/components/grid/GridDetailsDetachTest.java

@@ -70,4 +70,28 @@ public class GridDetailsDetachTest extends MultiBrowserTest {
         Assert.assertEquals("Spacer content not visible",
                 "Extra data for Bean 5", spacers.get(1).getText());
     }
+
+    @Test
+    public void testDetachAndImmediateReattach() {
+        setDebug(true);
+        openTestURL();
+
+        $(GridElement.class).first().getCell(3, 0).click();
+        $(GridElement.class).first().getCell(5, 0).click();
+
+        assertNoErrorNotifications();
+
+        // Detach and Re-attach Grid
+        $(ButtonElement.class).get(1).click();
+
+        assertNoErrorNotifications();
+
+        List<WebElement> spacers = findElements(By.className("v-grid-spacer"));
+        Assert.assertEquals("Not enough spacers in DOM", 2, spacers.size());
+        Assert.assertEquals("Spacer content not visible",
+                "Extra data for Bean 3", spacers.get(0).getText());
+        Assert.assertEquals("Spacer content not visible",
+                "Extra data for Bean 5", spacers.get(1).getText());
+    }
+
 }

uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridColumnHidingTest.java

@@ -870,7 +870,10 @@ public class GridColumnHidingTest extends GridBasicClientFeaturesTest {
         selectMenuPath("Component", "Columns", "Column 0", "Hidable");
         getSidebarOpenButton().click();
         verifySidebarOpened();
-        findElement(By.className("v-app")).click();
+        // Click somewhere far from Grid.
+        new Actions(getDriver())
+                .moveToElement(findElement(By.className("v-app")), 600, 600)
+                .click().perform();
         verifySidebarClosed();
     }
 

uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridDetailsClientTest.java

@@ -35,8 +35,8 @@ import com.vaadin.shared.ui.grid.ScrollDestination;
 import com.vaadin.testbench.By;
 import com.vaadin.testbench.ElementQuery;
 import com.vaadin.testbench.TestBenchElement;
-import com.vaadin.testbench.elements.NotificationElement;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicClientFeaturesTest;
+import com.vaadin.tests.tb3.newelements.FixedNotificationElement;
 
 public class GridDetailsClientTest extends GridBasicClientFeaturesTest {
 
@@ -112,12 +112,12 @@ public class GridDetailsClientTest extends GridBasicClientFeaturesTest {
     @Test
     public void errorUpdaterShowsErrorNotification() {
         assertFalse("No notifications should've been at the start",
-                $(NotificationElement.class).exists());
+                $(FixedNotificationElement.class).exists());
 
         toggleDetailsFor(1);
         selectMenuPath(SET_FAULTY_GENERATOR);
 
-        ElementQuery<NotificationElement> notification = $(NotificationElement.class);
+        ElementQuery<FixedNotificationElement> notification = $(FixedNotificationElement.class);
         assertTrue("Was expecting an error notification here",
                 notification.exists());
         notification.first().close();
@@ -131,7 +131,7 @@ public class GridDetailsClientTest extends GridBasicClientFeaturesTest {
         toggleDetailsFor(1);
 
         selectMenuPath(SET_FAULTY_GENERATOR);
-        $(NotificationElement.class).first().close();
+        $(FixedNotificationElement.class).first().close();
         selectMenuPath(SET_GENERATOR);
 
         assertNotEquals(

uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridDetailsServerTest.java

@@ -22,7 +22,6 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.openqa.selenium.By;
 import org.openqa.selenium.NoSuchElementException;
@@ -196,14 +195,11 @@ public class GridDetailsServerTest extends GridBasicFeaturesTest {
         assertEquals("Two", getGridElement().getDetails(0).getText());
     }
 
-    @Ignore("This use case is not currently supported by Grid. If the detail "
-            + "is out of view, the component is detached from the UI and a "
-            + "new instance is generated when scrolled back. Support will "
-            + "maybe be incorporated at a later time")
     @Test
     public void hierarchyChangesWorkInDetailsWhileOutOfView() {
         selectMenuPath(DETAILS_GENERATOR_HIERARCHICAL);
         selectMenuPath(OPEN_FIRST_ITEM_DETAILS);
+        assertEquals("One", getGridElement().getDetails(0).getText());
         scrollGridVerticallyTo(10000);
         selectMenuPath(CHANGE_HIERARCHY);
         scrollGridVerticallyTo(0);

uitest/src/com/vaadin/tests/components/table/TableColumnWidthsAndSorting.java

@@ -0,0 +1,50 @@
+/*
+ * 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.table;
+
+import com.vaadin.annotations.Theme;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.tests.fieldgroup.ComplexPerson;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Button.ClickListener;
+import com.vaadin.ui.Table;
+
+@Theme("valo")
+public class TableColumnWidthsAndSorting extends AbstractTestUIWithLog {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        final Table t = new Table();
+        t.setContainerDataSource(ComplexPerson.createContainer(100));
+        t.setVisibleColumns("firstName", "lastName", "age", "gender", "salary");
+        t.setColumnWidth("firstName", 200);
+        t.setColumnWidth("lastName", 200);
+        t.setSelectable(true);
+        addComponent(t);
+
+        Button b = new Button("Sort according to gender", new ClickListener() {
+
+            @Override
+            public void buttonClick(ClickEvent event) {
+                t.sort(new Object[] { "gender" }, new boolean[] { true });
+            }
+        });
+
+        addComponent(b);
+    }
+}

uitest/src/com/vaadin/tests/components/table/TableColumnWidthsAndSortingTest.java

@@ -0,0 +1,52 @@
+/*
+ * 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.table;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import com.vaadin.testbench.elements.TableElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class TableColumnWidthsAndSortingTest extends MultiBrowserTest {
+
+    @Test
+    public void testHeaderHeight() {
+        openTestURL();
+        TableElement t = $(TableElement.class).first();
+
+        assertHeaderCellHeight(t);
+
+        // Sort according to age
+        t.getHeaderCell(2).click();
+        assertHeaderCellHeight(t);
+
+        // Sort again according to age
+        t.getHeaderCell(2).click();
+        assertHeaderCellHeight(t);
+
+    }
+
+    private void assertHeaderCellHeight(TableElement t) {
+        // Assert all headers are correct height (37px according to default
+        // Valo)
+        for (int i = 0; i < 5; i++) {
+            Assert.assertEquals("Height of header cell " + i + " is wrong", 37,
+                    t.getHeaderCell(0).getSize().getHeight());
+        }
+
+    }
+}

uitest/tb2/com/vaadin/tests/components/combobox/ComboBoxLargeIcons.html

@@ -1,152 +0,0 @@
-<?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>New Test</title>
-</head>
-<body>
-<table cellpadding="1" cellspacing="1" border="1">
-<thead>
-<tr><td rowspan="1" colspan="3">New Test</td></tr>
-</thead><tbody>
-<tr>
-	<td>open</td>
-	<td>/run/com.vaadin.tests.components.combobox.Comboboxes?restartApplication</td>
-	<td></td>
-</tr>
-<tr>
-	<td>select</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::PID_Sselectaction-Icon/domChild[0]</td>
-	<td>label=16x16</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[4]/VFilterSelect[0]/domChild[1]</td>
-	<td>13,8</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::Root/VFilterSelect$SuggestionPopup[0]/domChild[0]/domChild[2]/domChild[0]</td>
-	<td>116,6</td>
-</tr>
-<!--  Open twice to avoid IE6 css issues -->
-<tr>
-	<td>open</td>
-	<td>/run/com.vaadin.tests.components.combobox.Comboboxes?restartApplication</td>
-	<td></td>
-</tr>
-<tr>
-	<td>select</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::PID_Sselectaction-Icon/domChild[0]</td>
-	<td>label=16x16</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[4]/VFilterSelect[0]/domChild[1]</td>
-	<td>13,8</td>
-</tr>
-<tr>
-	<td>screenCapture</td>
-	<td></td>
-	<td>icons-16x16-page1</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::Root/VFilterSelect$SuggestionPopup[0]/domChild[0]/domChild[2]/domChild[0]</td>
-	<td>116,6</td>
-</tr>
-<tr>
-	<td>screenCapture</td>
-	<td></td>
-	<td>icons-16x16-page2</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::Root/VFilterSelect$SuggestionPopup[0]/VFilterSelect$SuggestionMenu[0]#item0</td>
-	<td>378,1</td>
-</tr>
-<tr>
-	<td>screenCapture</td>
-	<td></td>
-	<td>icons-16x16-selected-1-3-5-9</td>
-</tr>
-<tr>
-	<td>select</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::PID_Sselectaction-Icon/domChild[0]</td>
-	<td>label=32x32</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[4]/VFilterSelect[0]/domChild[2]</td>
-	<td>8,13</td>
-</tr>
-<tr>
-	<td>screenCapture</td>
-	<td></td>
-	<td>icons-32x32-page2</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/domChild[1]</td>
-	<td>409,27</td>
-</tr>
-<tr>
-	<td>select</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::PID_Sselectaction-Icon/domChild[0]</td>
-	<td>label=64x64</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[9]/VFilterSelect[0]/domChild[1]</td>
-	<td>11,13</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::Root/VFilterSelect$SuggestionPopup[0]/VFilterSelect$SuggestionMenu[0]#item1</td>
-	<td>213,57</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/domChild[4]</td>
-	<td>535,43</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[8]/VFilterSelect[0]/domChild[1]</td>
-	<td>7,12</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::Root/VFilterSelect$SuggestionPopup[0]/VFilterSelect$SuggestionMenu[0]#item1</td>
-	<td>158,25</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[7]/VFilterSelect[0]/domChild[0]</td>
-	<td>16,9</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[7]/VFilterSelect[0]/domChild[0]</td>
-	<td>80,7</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[7]/VFilterSelect[0]/domChild[0]</td>
-	<td>down</td>
-</tr>
-<tr>
-	<td>screenCapture</td>
-	<td></td>
-	<td>icons-64x64-page1-highlight-first</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentscomboboxComboboxes::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/domChild[6]/domChild[0]</td>
-	<td>510,1</td>
-</tr>
-</tbody></table>
-</body>
-</html>