Tweaks to Grid/Escalator column size handling (#12145)

- ScrollbarBundle: removed delays in scroll handling that were only
needed for IE8, added possibility to update offsetSize and scrollSize at
the same time in order to avoid triggering unnecessary scrollbar
visibility change events during the intermediate state.
- ColumnConfigurator: added new method that allows setting column widths
without triggering element size recalculations.
- EscalatorProxy: added implementation of the new method to
ColumnConfigurationProxy.
- Escalator: switched to use new methods in ScrollbarBundle and
ColumnConfigurator, added a pixel to a scrollbar offsetSize calculation
that was for some reason consistently one pixel too low, removed
duplicate method calls from sectionHeightCalculated handling as those
are already handled by the calling method and can cause incorrect
intermediate state and unnecessary scrollbar visibility change events,
added implementation of the new method to ColumnConfigurationImpl with
the element size recalculations made optional.
- Grid: updated column minimum width calculations to take into account
the potential presence of a resize handle, updated expand ratio handling
to not trigger element size recalculations until the entire handling is
finished.
- Test for column width handling when there are multiple columns with
setMinimumWidthFromContent(false)

Fixes #12139

client/src/main/java/com/vaadin/client/widget/escalator/ColumnConfiguration.java

     public double getColumnWidth(int index) throws IllegalArgumentException;
 
     /**
-     * Sets widths for a set of columns.
+     * Sets widths for a set of columns. Triggers element size recalculation for
+     * elements that require manual calculations.
      *
      * @param indexWidthMap
      *            a map from column index to its respective width to be set. If
     public void setColumnWidths(Map<Integer, Double> indexWidthMap)
             throws IllegalArgumentException;
 
+    /**
+     * Sets widths for a set of columns.
+     *
+     * @param indexWidthMap
+     *            a map from column index to its respective width to be set. If
+     *            the given width for a column index is negative, the column is
+     *            resized-to-fit.
+     * @param recalculateElementSizes
+     *            should the element size recalculation be triggered for
+     *            elements that require manual calculation
+     * @throws IllegalArgumentException
+     *             if {@code indexWidthMap} is {@code null}
+     * @throws IllegalArgumentException
+     *             if any column index in {@code indexWidthMap} is invalid
+     * @throws NullPointerException
+     *             If any value in the map is <code>null</code>
+     */
+    public void setColumnWidths(Map<Integer, Double> indexWidthMap,
+            boolean recalculateElementSizes) throws IllegalArgumentException;
+
     /**
      * Returns the actual width of a column.
      *

client/src/main/java/com/vaadin/client/widget/escalator/ScrollbarBundle.java

 
     private final ScrollEventFirer scrollEventFirer = new ScrollEventFirer();
 
-    private HandlerRegistration scrollSizeTemporaryScrollHandler;
-    private HandlerRegistration offsetSizeTemporaryScrollHandler;
     private HandlerRegistration scrollInProgress;
 
     private ScrollbarBundle() {
      *
      * @param px
      *            the length of the scrollbar in pixels
+     * @see #setOffsetSizeAndScrollSize(double, double)
      */
     public final void setOffsetSize(final double px) {
 
-        /*
-         * This needs to be made step-by-step because IE8 flat-out refuses to
-         * fire a scroll event when the scroll size becomes smaller than the
-         * offset size. All other browser need to suffer alongside.
-         */
-
         boolean newOffsetSizeIsGreaterThanScrollSize = px > getScrollSize();
         boolean offsetSizeBecomesGreaterThanScrollSize = showsScrollHandle()
                 && newOffsetSizeIsGreaterThanScrollSize;
+
         if (offsetSizeBecomesGreaterThanScrollSize && getScrollPos() != 0) {
-            if (offsetSizeTemporaryScrollHandler != null) {
-                offsetSizeTemporaryScrollHandler.removeHandler();
-            }
-            // must be a field because Java insists.
-            offsetSizeTemporaryScrollHandler = addScrollHandler(
-                    event -> setOffsetSizeNow(px));
             setScrollPos(0);
-        } else {
+            setOffsetSizeNow(px);
+        } else if (px != getOffsetSize()) {
             setOffsetSizeNow(px);
         }
     }
         recalculateMaxScrollPos();
         forceScrollbar(showsScrollHandle());
         fireVisibilityChangeIfNeeded();
-        if (offsetSizeTemporaryScrollHandler != null) {
-            offsetSizeTemporaryScrollHandler.removeHandler();
-            offsetSizeTemporaryScrollHandler = null;
+    }
+
+    /**
+     * Sets the length of the scrollbar and the amount of pixels the scrollbar
+     * needs to be able to scroll through.
+     *
+     * @param offsetPx
+     *            the length of the scrollbar in pixels
+     * @param scrollPx
+     *            the number of pixels the scrollbar should be able to scroll
+     *            through
+     */
+    public final void setOffsetSizeAndScrollSize(final double offsetPx,
+            final double scrollPx) {
+
+        boolean newOffsetSizeIsGreaterThanScrollSize = offsetPx > scrollPx;
+        boolean offsetSizeBecomesGreaterThanScrollSize = showsScrollHandle()
+                && newOffsetSizeIsGreaterThanScrollSize;
+
+        boolean needsMoreHandling = false;
+        if (offsetSizeBecomesGreaterThanScrollSize && getScrollPos() != 0) {
+            setScrollPos(0);
+            if (offsetPx != getOffsetSize()) {
+                internalSetOffsetSize(Math.max(0, offsetPx));
+            }
+            if (scrollPx != getScrollSize()) {
+                internalSetScrollSize(Math.max(0, scrollPx));
+            }
+            needsMoreHandling = true;
+        } else {
+            if (offsetPx != getOffsetSize()) {
+                internalSetOffsetSize(Math.max(0, offsetPx));
+                needsMoreHandling = true;
+            }
+            if (scrollPx != getScrollSize()) {
+                internalSetScrollSize(Math.max(0, scrollPx));
+                needsMoreHandling = true;
+            }
+        }
+
+        if (needsMoreHandling) {
+            recalculateMaxScrollPos();
+            forceScrollbar(showsScrollHandle());
+            fireVisibilityChangeIfNeeded();
         }
     }
 
      * @param px
      *            the number of pixels the scrollbar should be able to scroll
      *            through
+     * @see #setOffsetSizeAndScrollSize(double, double)
      */
     public final void setScrollSize(final double px) {
 
-        /*
-         * This needs to be made step-by-step because IE8 flat-out refuses to
-         * fire a scroll event when the scroll size becomes smaller than the
-         * offset size. All other browser need to suffer alongside.
-         *
-         * This really should be changed to not use any temporary scroll
-         * handlers at all once IE8 support is dropped, like now done only for
-         * Firefox.
-         */
-
         boolean newScrollSizeIsSmallerThanOffsetSize = px <= getOffsetSize();
         boolean scrollSizeBecomesSmallerThanOffsetSize = showsScrollHandle()
                 && newScrollSizeIsSmallerThanOffsetSize;
+
         if (scrollSizeBecomesSmallerThanOffsetSize && getScrollPos() != 0) {
-            /*
-             * For whatever reason, Firefox loses the scroll event in this case
-             * and the onscroll handler is never called (happens when reducing
-             * size from 1000 items to 1 while being scrolled a bit down, see
-             * #19802). Based on the comment above, only IE8 should really use
-             * 'delayedSizeSet'
-             */
-            boolean delayedSizeSet = !BrowserInfo.get().isFirefox();
-            if (delayedSizeSet) {
-                if (scrollSizeTemporaryScrollHandler != null) {
-                    scrollSizeTemporaryScrollHandler.removeHandler();
-                }
-                scrollSizeTemporaryScrollHandler = addScrollHandler(
-                        event -> setScrollSizeNow(px));
-            }
             setScrollPos(0);
-            if (!delayedSizeSet) {
-                setScrollSizeNow(px);
-            }
-        } else {
+            setScrollSizeNow(px);
+        } else if (px != getScrollSize()) {
             setScrollSizeNow(px);
         }
     }
         recalculateMaxScrollPos();
         forceScrollbar(showsScrollHandle());
         fireVisibilityChangeIfNeeded();
-        if (scrollSizeTemporaryScrollHandler != null) {
-            scrollSizeTemporaryScrollHandler.removeHandler();
-            scrollSizeTemporaryScrollHandler = null;
-        }
     }
 
     /**
     public boolean isWorkPending() {
         // Need to include scrollEventFirer.isBeingFired as it might use
         // requestAnimationFrame - which is not automatically checked
-        return scrollSizeTemporaryScrollHandler != null
-                || offsetSizeTemporaryScrollHandler != null
-                || scrollInProgress != null || scrollEventFirer.isBeingFired;
+        return scrollInProgress != null || scrollEventFirer.isBeingFired;
     }
 }

client/src/main/java/com/vaadin/client/widgets/Escalator.java

             double headerHeight = header.getHeightOfSection();
             double vScrollbarHeight = Math.max(0,
                     tableWrapperHeight - footerHeight - headerHeight);
-            verticalScrollbar.setOffsetSize(vScrollbarHeight);
-            verticalScrollbar.setScrollSize(scrollContentHeight);
+            verticalScrollbar.setOffsetSizeAndScrollSize(vScrollbarHeight,
+                    scrollContentHeight);
 
             /*
              * If decreasing the amount of frozen columns, and scrolled to the
                             columnConfiguration.getColumnCount()));
             double frozenPixels = scrollContentWidth - unfrozenPixels;
             double hScrollOffsetWidth = tableWrapperWidth - frozenPixels;
-            horizontalScrollbar.setOffsetSize(hScrollOffsetWidth);
-            horizontalScrollbar.setScrollSize(unfrozenPixels);
+            horizontalScrollbar.setOffsetSizeAndScrollSize(hScrollOffsetWidth,
+                    unfrozenPixels);
             horizontalScrollbar.getElement().getStyle().setLeft(frozenPixels,
                     Unit.PX);
             horizontalScrollbar.setScrollPos(prevScrollPos);
                             Integer col = Integer.valueOf(i);
                             colWidths.put(col, width);
                         }
-                        getColumnConfiguration().setColumnWidths(colWidths);
+                        getColumnConfiguration().setColumnWidths(colWidths,
+                                true);
                     });
                 }
             }
                  */
                 verticalScrollbar.setOffsetSize(
                         heightOfEscalator - header.getHeightOfSection()
-                                - footer.getHeightOfSection());
+                                - footer.getHeightOfSection() + 1);
 
                 body.verifyEscalatorCount();
                 body.spacerContainer.updateSpacerDecosVisibility();
 
         @Override
         protected void sectionHeightCalculated() {
-            double headerHeight = header.getHeightOfSection();
-            double footerHeight = footer.getHeightOfSection();
-            int vscrollHeight = (int) Math
-                    .floor(heightOfEscalator - headerHeight - footerHeight);
-
-            final boolean horizontalScrollbarNeeded = columnConfiguration
-                    .calculateRowWidth() > widthOfEscalator;
-            if (horizontalScrollbarNeeded) {
-                vscrollHeight -= horizontalScrollbar.getScrollbarThickness();
-            }
-
             footerDeco.getStyle().setHeight(footer.getHeightOfSection(),
                     Unit.PX);
-
-            verticalScrollbar.setOffsetSize(vscrollHeight);
         }
     }
 
                     Integer col = Integer.valueOf(i);
                     colWidths.put(col, width);
                 }
-                getColumnConfiguration().setColumnWidths(colWidths);
+                getColumnConfiguration().setColumnWidths(colWidths, true);
             }
 
             // Adjust scrollbar
         public void setColumnWidth(int index, double px)
                 throws IllegalArgumentException {
             setColumnWidths(Collections.singletonMap(Integer.valueOf(index),
-                    Double.valueOf(px)));
+                    Double.valueOf(px)), true);
         }
 
         @Override
         public void setColumnWidths(Map<Integer, Double> indexWidthMap)
                 throws IllegalArgumentException {
+            setColumnWidths(indexWidthMap, true);
+        }
+
+        @Override
+        public void setColumnWidths(Map<Integer, Double> indexWidthMap,
+                boolean recalculateElementSizes)
+                throws IllegalArgumentException {
 
             if (indexWidthMap == null) {
                 throw new IllegalArgumentException("indexWidthMap was null");
                 body.reapplyColumnWidths();
                 footer.reapplyColumnWidths();
 
-                recalculateElementSizes();
+                if (recalculateElementSizes) {
+                    recalculateElementSizes();
+                }
 
             } finally {
                 Profiler.leave(

client/src/main/java/com/vaadin/client/widgets/Grid.java

         private boolean columnsAreGuaranteedToBeWiderThanGrid() {
             double freeSpace = escalator.getInnerWidth();
             for (Column<?, ?> column : getVisibleColumns()) {
+                /*
+                 * Check the width and min width and ensure that no column can
+                 * be expected to be narrower than what the resize handler
+                 * requires, if one is present.
+                 */
                 if (column.getWidth() >= 0) {
-                    freeSpace -= column.getWidth();
+                    if (column.isResizable() && resizeHandleWidth > 0) {
+                        freeSpace -= Math.max(resizeHandleWidth,
+                                column.getWidth());
+                    } else {
+                        freeSpace -= column.getWidth();
+                    }
                 } else if (column.getMinimumWidth() >= 0) {
-                    freeSpace -= column.getMinimumWidth();
+                    if (column.isResizable() && resizeHandleWidth > 0) {
+                        freeSpace -= Math.max(resizeHandleWidth,
+                                column.getMinimumWidth());
+                    } else {
+                        freeSpace -= column.getMinimumWidth();
+                    }
+                } else if (column.isResizable() && resizeHandleWidth > 0) {
+                    freeSpace -= resizeHandleWidth;
                 }
             }
             return freeSpace < 0;
                 selfWidths.put(index, columns.get(index).getWidth());
             }
             Grid.this.escalator.getColumnConfiguration()
-                    .setColumnWidths(selfWidths);
+                    .setColumnWidths(selfWidths, true);
 
             /*
              * Step 2: Make sure that each column ends up obeying their min/max
                 }
             }
             Grid.this.escalator.getColumnConfiguration()
-                    .setColumnWidths(constrainedWidths);
+                    .setColumnWidths(constrainedWidths, true);
         }
 
         private void applyColumnWidthsWithExpansion() {
             for (Column<?, T> column : visibleColumns) {
                 final double widthAsIs = column.getWidth();
                 final boolean isFixedWidth = widthAsIs >= 0;
-                // Check for max width just to be sure we don't break the limits
-                final double widthFixed = Math.max(
-                        Math.min(getMaxWidth(column), widthAsIs),
-                        column.getMinimumWidth());
                 defaultExpandRatios = defaultExpandRatios
                         && (column.getExpandRatio() == -1
                                 || column == selectionColumn);
 
                 if (isFixedWidth) {
+                    // Check for min & max width just to be sure we don't break
+                    // the limits
+                    double widthFixed = Math.max(
+                            Math.min(getMaxWidth(column), widthAsIs),
+                            column.getMinimumWidth());
+                    if (column.isResizable() && resizeHandleWidth > 0) {
+                        // Ensure the resize handle fits
+                        widthFixed = Math.max(widthFixed, resizeHandleWidth);
+                    }
+
                     columnSizes.put(visibleColumns.indexOf(column), widthFixed);
                     reservedPixels += widthFixed;
                 } else {
                 }
             }
 
-            setColumnSizes(columnSizes);
+            /*
+             * Set column sizes so that it's possible to measure non-fixed
+             * actual sizes without previously applied expand ratio tweaks, but
+             * don't trigger the element size recalculation before the rest of
+             * this method has also been processed.
+             */
+            setColumnSizes(columnSizes, false);
 
             for (Column<?, T> column : nonFixedColumns) {
                 final int expandRatio = defaultExpandRatios ? 1
                 final double maxWidth = getMaxWidth(column);
                 double newWidth;
                 if (column.isMinimumWidthFromContent()) {
-                    newWidth = Math.min(maxWidth, column.getWidthActual());
+                    if (column.isResizable() && resizeHandleWidth > 0) {
+                        // Ensure the resize handle fits
+                        newWidth = Math.max(
+                                Math.min(maxWidth, column.getWidthActual()),
+                                resizeHandleWidth);
+                    } else {
+                        newWidth = Math.min(maxWidth, column.getWidthActual());
+                    }
                 } else {
-                    newWidth = 0;
+                    if (column.isResizable() && resizeHandleWidth > 0) {
+                        // Ensure the resize handle fits
+                        newWidth = resizeHandleWidth;
+                    } else {
+                        newWidth = 0;
+                    }
                 }
 
                 boolean shouldExpand = newWidth < maxWidth && expandRatio > 0
             if (pixelsToDistribute <= 0 || totalRatios <= 0) {
                 if (pixelsToDistribute <= 0) {
                     // Set column sizes for expanding columns
-                    setColumnSizes(columnSizes);
+                    setColumnSizes(columnSizes, true);
                 }
+                /*
+                 * If pixelsToDistribute > 0 the element size recalculation
+                 * isn't done at all, even if some column sizes were set
+                 * earlier, but this doesn't appear to be detrimental while
+                 * attempting to trigger the recalculation here breaks a
+                 * GridEditRow test.
+                 */
 
                 return;
             }
             } while (aColumnHasMaxedOut);
 
             if (totalRatios <= 0 && columnsToExpand.isEmpty()) {
-                setColumnSizes(columnSizes);
+                setColumnSizes(columnSizes, true);
                 return;
             }
             assert pixelsToDistribute > 0 : "We've run out of pixels to distribute ("
             } while (minWidthsCausedReflows);
 
             // Finally set all the column sizes.
-            setColumnSizes(columnSizes);
+            setColumnSizes(columnSizes, true);
         }
 
-        private void setColumnSizes(Map<Integer, Double> columnSizes) {
+        private void setColumnSizes(Map<Integer, Double> columnSizes,
+                boolean recalculateElementSizes) {
             // Set all widths at once
-            escalator.getColumnConfiguration().setColumnWidths(columnSizes);
+            escalator.getColumnConfiguration().setColumnWidths(columnSizes,
+                    recalculateElementSizes);
         }
 
         private int getExpandRatio(Column<?, ?> column,
 
     private boolean refreshBodyRequested = false;
 
+    private double resizeHandleWidth = 0;
+
     private DragAndDropHandler.DragAndDropCallback headerCellDndCallback = new DragAndDropCallback() {
 
         private final AutoScrollerCallback autoScrollerCallback = new AutoScrollerCallback() {
                     final DragHandle dragger = new DragHandle(
                             getStylePrimaryName() + "-column-resize-handle");
                     dragger.addTo(td);
+                    // Save the newest resize handle's width with the assumption
+                    // that all the resize handles are the same size. This is
+                    // used in column's minimum width calculations, so the
+                    // border of the cell is also included.
+                    resizeHandleWidth = dragger.getElement().getOffsetWidth()
+                            + WidgetUtil.getBorderLeftAndRightThickness(td);
 
                     // Common functionality for drag handle callback
                     // implementations

uitest/src/main/java/com/vaadin/tests/components/grid/GridColumnsNoMinimumWidthFromContent.java

+package com.vaadin.tests.components.grid;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Random;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.components.grid.FooterRow;
+
+public class GridColumnsNoMinimumWidthFromContent extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        Random random = new Random();
+
+        List<DummyGridRow> gridRows = new ArrayList<DummyGridRow>();
+        gridRows.add(new DummyGridRow(random));
+
+        Grid<DummyGridRow> grid = new Grid<DummyGridRow>();
+        for (int i = 0; i < 20; i++) {
+            grid.addColumn(DummyGridRow::getValue)
+                    .setCaption("[" + i + "] Quite dummy column")
+                    .setMinimumWidthFromContent(false);
+        }
+
+        grid.setItems(gridRows);
+        FooterRow defaultFooter = grid.appendFooterRow();
+        grid.getColumns().forEach(column -> defaultFooter.getCell(column)
+                .setText(grid.getDefaultHeaderRow().getCell(column).getText()));
+        grid.setFooterVisible(true);
+        grid.setHeightByRows(gridRows.size());
+        grid.setWidthFull();
+
+        getLayout().addComponent(grid);
+    }
+
+    class DummyGridRow {
+        private Random random = null;
+
+        public DummyGridRow(Random random) {
+            this.random = random;
+        }
+
+        public int getValue() {
+            return random.nextInt(1000000000);
+        }
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 12139;
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Loading the UI should not get stuck in an eternal loop "
+                + "and the columns should be narrow with ellipsis "
+                + "until the page is resized small enough that "
+                + "the resize handles alone force a scrollbar. "
+                + "No overflowing of header cells should occur "
+                + "when resized very near to the cutoff point "
+                + "between no scrollbar and a scrollbar.";
+    }
+}

uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java

                 throws IllegalArgumentException {
             columnConfiguration.setColumnWidths(indexWidthMap);
         }
+
+        @Override
+        public void setColumnWidths(Map<Integer, Double> indexWidthMap,
+                boolean recalculateElementSizes)
+                throws IllegalArgumentException {
+            columnConfiguration.setColumnWidths(indexWidthMap,
+                    recalculateElementSizes);
+        }
     }
 
     private class BodyRowContainerProxy extends RowContainerProxy

uitest/src/test/java/com/vaadin/tests/components/grid/GridColumnsNoMinimumWidthFromContentTest.java

+package com.vaadin.tests.components.grid;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.number.IsCloseTo.closeTo;
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+import org.openqa.selenium.Dimension;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.testbench.elements.GridElement.GridCellElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class GridColumnsNoMinimumWidthFromContentTest extends MultiBrowserTest {
+
+    @Test
+    public void testResizing() {
+        openTestURL();
+
+        GridElement grid = $(GridElement.class).first();
+        WebElement hScrollbar = grid
+                .findElement(By.className("v-grid-scroller-horizontal"));
+
+        // initial state, should have no scrollbar
+        GridCellElement lastColumn = grid.getHeaderCell(0, 19);
+        ensureScrollbarVisibility(hScrollbar, false);
+        ensureNoGap(grid, lastColumn);
+
+        // resize small enough to get a scrollbar
+        getDriver().manage().window().setSize(new Dimension(810, 800));
+        ensureScrollbarVisibility(hScrollbar, true);
+
+        // resize just enough to lose the scrollbar
+        getDriver().manage().window().setSize(new Dimension(840, 800));
+        ensureScrollbarVisibility(hScrollbar, false);
+        ensureNoGap(grid, lastColumn);
+
+        int lastColumnWidth = lastColumn.getSize().getWidth();
+        assertGreater("Unexpected last column width: " + lastColumnWidth
+                + " (should be over 30)", lastColumnWidth, 30);
+    }
+
+    private void ensureNoGap(GridElement grid, GridCellElement lastColumn) {
+        int gridRightEdge = grid.getLocation().getX()
+                + grid.getSize().getWidth();
+        int lastColumnRightEdge = lastColumn.getLocation().getX()
+                + lastColumn.getSize().getWidth();
+        assertThat("Unexpected positioning.", (double) gridRightEdge,
+                closeTo(lastColumnRightEdge, 1d));
+    }
+
+    private void ensureScrollbarVisibility(WebElement scrollbar,
+            boolean displayed) {
+        assertEquals(displayed ? "block" : "none",
+                scrollbar.getCssValue("display"));
+    }
+}