Reworked and cleaned up client-side TabSheet and Accordion. (#12357)

- Added and corrected JavaDocs.
- Deprecated unused public methods.
- Fixed first tab style logic in TabSheet.
- Fixed navigation focus logic in TabSheet.
- Fixed tab width bookkeeping for scrolling TabSheet tabs.
- Renamed private methods and variables for clarity.
- Removed unnecessary or duplicated private methods.
- Reworked some logic to clarify it and to better match my understanding
of what's supposed to happen within those methods.
- Updated some deprecated method calls to use currently recommended
solutions.
- Added and updated regression tests.

client/src/main/java/com/vaadin/client/ui/VAccordion.java

@@ -32,14 +32,23 @@ import com.vaadin.client.ComponentConnector;
 import com.vaadin.client.VCaption;
 import com.vaadin.client.WidgetUtil;
 import com.vaadin.client.ui.TouchScrollDelegate.TouchScrollHandler;
+import com.vaadin.client.ui.VAccordion.StackItem;
 import com.vaadin.shared.ComponentConstants;
 import com.vaadin.shared.ui.accordion.AccordionState;
 import com.vaadin.shared.ui.tabsheet.TabState;
 import com.vaadin.shared.ui.tabsheet.TabsheetServerRpc;
 import com.vaadin.shared.util.SharedUtil;
 
+/**
+ * Widget class for the Accordion component. Displays one child item's contents
+ * at a time.
+ *
+ * @author Vaadin Ltd
+ *
+ */
 public class VAccordion extends VTabsheetBase {
 
+    /** Default classname for this widget. */
     public static final String CLASSNAME = AccordionState.PRIMARY_STYLE_NAME;
 
     private Set<Widget> widgets = new HashSet<>();
@@ -53,16 +62,19 @@ public class VAccordion extends VTabsheetBase {
 
     private int tabulatorIndex;
 
+    /**
+     * Constructs a widget for an Accordion.
+     */
     public VAccordion() {
         super(CLASSNAME);
 
         touchScrollHandler = TouchScrollDelegate.enableTouchScrolling(this);
     }
 
+    @SuppressWarnings("deprecation")
     @Override
     public void renderTab(TabState tabState, int index) {
         StackItem item;
-        int itemIndex;
 
         if (getWidgetCount() <= index) {
             // Create stackItem and render caption
@@ -70,12 +82,9 @@ public class VAccordion extends VTabsheetBase {
             if (getWidgetCount() == 0) {
                 item.addStyleDependentName("first");
             }
-            itemIndex = getWidgetCount();
             add(item, getElement());
         } else {
             item = getStackItem(index);
-
-            itemIndex = index;
         }
         item.updateCaption(tabState);
 
@@ -103,6 +112,12 @@ public class VAccordion extends VTabsheetBase {
         updateStyleNames(style);
     }
 
+    /**
+     * Updates the primary style name base for all stack items.
+     *
+     * @param primaryStyleName
+     *            the new primary style name base
+     */
     protected void updateStyleNames(String primaryStyleName) {
         for (Widget w : getChildren()) {
             if (w instanceof StackItem) {
@@ -114,6 +129,12 @@ public class VAccordion extends VTabsheetBase {
 
     /**
      * For internal use only. May be renamed or removed in a future release.
+     * <p>
+     * Sets the tabulator index for the active stack item. The active stack item
+     * represents the entire accordion in the browser's focus cycle (excluding
+     * any focusable elements within the content panel).
+     * <p>
+     * This value is delegated from the TabsheetState via AccordionState.
      *
      * @param tabIndex
      *            tabulator index for the open stack item
@@ -127,7 +148,12 @@ public class VAccordion extends VTabsheetBase {
         }
     }
 
-    /** For internal use only. May be removed or replaced in the future. */
+    /**
+     * For internal use only. May be removed or replaced in the future.
+     *
+     * @param itemIndex
+     *            the index of the stack item to open
+     */
     public void open(int itemIndex) {
         StackItem item = (StackItem) getWidget(itemIndex);
         boolean alreadyOpen = false;
@@ -148,7 +174,12 @@ public class VAccordion extends VTabsheetBase {
 
     }
 
-    /** For internal use only. May be removed or replaced in the future. */
+    /**
+     * For internal use only. May be removed or replaced in the future.
+     *
+     * @param item
+     *            the stack item to close
+     */
     public void close(StackItem item) {
         if (!item.isOpen()) {
             return;
@@ -160,6 +191,12 @@ public class VAccordion extends VTabsheetBase {
 
     }
 
+    /**
+     * Handle stack item selection.
+     *
+     * @param item
+     *            the selected stack item
+     */
     public void onSelectTab(StackItem item) {
         final int index = getWidgetIndex(item);
 
@@ -182,6 +219,13 @@ public class VAccordion extends VTabsheetBase {
         private Widget widget;
         private String id;
 
+        /**
+         * Sets the height for this stack item's contents.
+         *
+         * @param height
+         *            the height to set (in pixels), or {@code -1} to remove
+         *            height
+         */
         public void setHeight(int height) {
             if (height == -1) {
                 super.setHeight("");
@@ -193,6 +237,12 @@ public class VAccordion extends VTabsheetBase {
             }
         }
 
+        /**
+         * Sets the identifier for this stack item.
+         *
+         * @param newId
+         *            the identifier to set
+         */
         public void setId(String newId) {
             if (!SharedUtil.equals(newId, id)) {
                 if (id != null) {
@@ -205,15 +255,23 @@ public class VAccordion extends VTabsheetBase {
             }
         }
 
+        /**
+         * Returns the wrapped widget of this stack item.
+         *
+         * @return the widget
+         *
+         * @deprecated This method is not called by the framework code anymore.
+         *             Use {@link #getChildWidget()} instead.
+         */
+        @Deprecated
         public Widget getComponent() {
-            return widget;
-        }
-
-        @Override
-        public void setVisible(boolean visible) {
-            super.setVisible(visible);
+            return getChildWidget();
         }
 
+        /**
+         * Queries the height from the wrapped widget and uses it to set this
+         * stack item's height.
+         */
         public void setHeightFromWidget() {
             Widget widget = getChildWidget();
             if (widget == null) {
@@ -228,7 +286,8 @@ public class VAccordion extends VTabsheetBase {
         /**
          * Returns caption width including padding.
          *
-         * @return
+         * @return the width of the caption (in pixels), or zero if there is no
+         *         caption element (not possible via the default implementation)
          */
         public int getCaptionWidth() {
             if (caption == null) {
@@ -241,6 +300,14 @@ public class VAccordion extends VTabsheetBase {
             return captionWidth + padding;
         }
 
+        /**
+         * Sets the width of the stack item, or removes it if given value is
+         * {@code -1}.
+         *
+         * @param width
+         *            the width to set (in pixels), or {@code -1} to remove
+         *            width
+         */
         public void setWidth(int width) {
             if (width == -1) {
                 super.setWidth("");
@@ -249,10 +316,24 @@ public class VAccordion extends VTabsheetBase {
             }
         }
 
+        /**
+         * Returns the offset height of this stack item.
+         *
+         * @return the height in pixels
+         *
+         * @deprecated This method is not called by the framework code anymore.
+         *             Use {@link #getOffsetHeight()} instead.
+         */
+        @Deprecated
         public int getHeight() {
             return getOffsetHeight();
         }
 
+        /**
+         * Returns the offset height of the caption node.
+         *
+         * @return the height in pixels
+         */
         public int getCaptionHeight() {
             return captionNode.getOffsetHeight();
         }
@@ -263,6 +344,11 @@ public class VAccordion extends VTabsheetBase {
         private Element captionNode = DOM.createDiv();
         private String styleName;
 
+        /**
+         * Constructs a stack item. The content widget should be set later when
+         * the stack item is opened.
+         */
+        @SuppressWarnings("deprecation")
         public StackItem() {
             setElement(DOM.createDiv());
             caption = new VCaption(client);
@@ -295,14 +381,31 @@ public class VAccordion extends VTabsheetBase {
             onSelectTab(this);
         }
 
+        /**
+         * Returns the container element for the content widget.
+         *
+         * @return the content container element
+         */
+        @SuppressWarnings("deprecation")
         public com.google.gwt.user.client.Element getContainerElement() {
             return DOM.asOld(content);
         }
 
+        /**
+         * Returns the wrapped widget of this stack item.
+         *
+         * @return the widget
+         */
         public Widget getChildWidget() {
             return widget;
         }
 
+        /**
+         * Replaces the existing wrapped widget (if any) with a new widget.
+         *
+         * @param newWidget
+         *            the new widget to wrap
+         */
         public void replaceWidget(Widget newWidget) {
             if (widget != null) {
                 widgets.remove(widget);
@@ -318,6 +421,9 @@ public class VAccordion extends VTabsheetBase {
 
         }
 
+        /**
+         * Opens the stack item and clears any previous visibility settings.
+         */
         public void open() {
             add(widget, content);
             open = true;
@@ -328,10 +434,20 @@ public class VAccordion extends VTabsheetBase {
             getElement().setTabIndex(tabulatorIndex);
         }
 
+        /**
+         * Hides the stack item content but does not close the stack item.
+         *
+         * @deprecated This method is not called by the framework code anymore.
+         */
+        @Deprecated
         public void hide() {
             content.getStyle().setVisibility(Visibility.HIDDEN);
         }
 
+        /**
+         * Closes this stack item and removes the wrapped widget from the DOM
+         * tree and this stack item.
+         */
         public void close() {
             if (widget != null) {
                 remove(widget);
@@ -346,6 +462,11 @@ public class VAccordion extends VTabsheetBase {
             getElement().setTabIndex(-1);
         }
 
+        /**
+         * Returns whether this stack item is open or not.
+         *
+         * @return {@code true} if open, {@code false} otherwise
+         */
         public boolean isOpen() {
             return open;
         }
@@ -377,8 +498,17 @@ public class VAccordion extends VTabsheetBase {
             onSelectTab(this);
         }
 
+        /**
+         * Updates the caption to match the current tab state.
+         *
+         * @param tabState
+         *            the state for this stack item
+         */
+        @SuppressWarnings("deprecation")
         public void updateCaption(TabState tabState) {
-            // TODO need to call this because the caption does not have an owner
+            // Need to call this because the caption does not have an owner, and
+            // cannot have an owner, because only the selected stack item's
+            // connector is sent to the client.
             caption.setCaptionAsHtml(isTabCaptionsAsHtml());
             caption.updateCaptionWithoutOwner(tabState.caption,
                     !tabState.enabled, hasAttribute(tabState.description),
@@ -394,10 +524,10 @@ public class VAccordion extends VTabsheetBase {
         }
 
         /**
-         * Updates a tabs stylename from the child UIDL
+         * Updates the stack item's style name from the TabState.
          *
-         * @param uidl
-         *            The child UIDL of the tab
+         * @param newStyleName
+         *            the new style name
          */
         private void updateTabStyleName(String newStyleName) {
             if (newStyleName != null && !newStyleName.isEmpty()) {
@@ -419,20 +549,55 @@ public class VAccordion extends VTabsheetBase {
             }
         }
 
+        /**
+         * Returns the offset width of the wrapped widget.
+         *
+         * @return the offset width in pixels, or zero if no widget is set
+         */
         public int getWidgetWidth() {
-            return DOM.getFirstChild(content).getOffsetWidth();
+            if (widget == null) {
+                return 0;
+            }
+            return widget.getOffsetWidth();
         }
 
+        /**
+         * Returns whether the given container's widget is this stack item's
+         * wrapped widget. Does not check whether the given container's widget
+         * is a child of the wrapped widget.
+         *
+         * @param p
+         *            the container whose widget to set
+         * @return {@code true} if the container's widget matches wrapped
+         *         widget, {@code false} otherwise
+         *
+         * @deprecated This method is not called by the framework code anymore.
+         */
+        @Deprecated
         public boolean contains(ComponentConnector p) {
             return (getChildWidget() == p.getWidget());
         }
 
+        /**
+         * Returns whether the caption element is visible or not.
+         *
+         * @return {@code true} if visible, {@code false} otherwise
+         *
+         * @deprecated This method is not called by the framework code anymore.
+         */
+        @Deprecated
         public boolean isCaptionVisible() {
             return caption.isVisible();
         }
 
     }
 
+    /**
+     * {@inheritDoc}
+     *
+     * @deprecated This method is not called by the framework code anymore.
+     */
+    @Deprecated
     @Override
     protected void clearPaintables() {
         clear();
@@ -474,15 +639,32 @@ public class VAccordion extends VTabsheetBase {
         return null;
     }
 
-    /** For internal use only. May be removed or replaced in the future. */
+    /**
+     * For internal use only. May be removed or replaced in the future.
+     *
+     * @param index
+     *            the index of the stack item to get
+     * @return the stack item
+     */
     public StackItem getStackItem(int index) {
         return (StackItem) getWidget(index);
     }
 
+    /**
+     * Returns an iterable over all the stack items.
+     *
+     * @return the iterable
+     */
+    @SuppressWarnings({ "rawtypes", "unchecked" })
     public Iterable<StackItem> getStackItems() {
         return (Iterable) getChildren();
     }
 
+    /**
+     * Returns the currently open stack item.
+     *
+     * @return the open stack item, or {@code null} if one does not exist
+     */
     public StackItem getOpenStackItem() {
         return openTab;
     }

client/src/main/java/com/vaadin/client/ui/VTabsheetBase.java

@@ -30,6 +30,11 @@ import com.vaadin.client.ComponentConnector;
 import com.vaadin.client.ConnectorMap;
 import com.vaadin.shared.ui.tabsheet.TabState;
 
+/**
+ * Base class for a multi-view widget such as TabSheet or Accordion.
+ *
+ * @author Vaadin Ltd.
+ */
 public abstract class VTabsheetBase extends ComplexPanel implements HasEnabled {
 
     /** For internal use only. May be removed or replaced in the future. */
@@ -52,6 +57,13 @@ public abstract class VTabsheetBase extends ComplexPanel implements HasEnabled {
 
     private boolean tabCaptionsAsHtml = false;
 
+    /**
+     * Constructs a multi-view widget with the given classname.
+     *
+     * @param classname
+     *            the style name to set
+     */
+    @SuppressWarnings("deprecation")
     public VTabsheetBase(String classname) {
         setElement(DOM.createDiv());
         setStyleName(classname);
@@ -64,48 +76,69 @@ public abstract class VTabsheetBase extends ComplexPanel implements HasEnabled {
 
     /**
      * Clears current tabs and contents.
+     *
+     * @deprecated This method is not called by the framework code anymore.
      */
+    @Deprecated
     protected abstract void clearPaintables();
 
     /**
      * Implement in extending classes. This method should render needed elements
-     * and set the visibility of the tab according to the 'selected' parameter.
+     * and set the visibility of the tab according to the 'visible' parameter.
+     * This method should not update the selection, the connector should handle
+     * that separately.
+     *
+     * @param tabState
+     *            shared state of a single tab
+     * @param index
+     *            the index of that tab
      */
     public abstract void renderTab(TabState tabState, int index);
 
     /**
      * Implement in extending classes. This method should return the number of
      * tabs currently rendered.
+     *
+     * @return the number of currently rendered tabs
      */
     public abstract int getTabCount();
 
     /**
-     * Implement in extending classes. This method should return the Paintable
+     * Implement in extending classes. This method should return the connector
      * corresponding to the given index.
+     *
+     * @param index
+     *            the index of the tab whose connector to find
+     * @return the connector of the queried tab, or {@code null} if not found
      */
     public abstract ComponentConnector getTab(int index);
 
     /**
      * Implement in extending classes. This method should remove the rendered
      * tab with the specified index.
+     *
+     * @param index
+     *            the index of the tab to remove
      */
     public abstract void removeTab(int index);
 
     /**
-     * Returns true if the width of the widget is undefined, false otherwise.
+     * Returns whether the width of the widget is undefined.
      *
      * @since 7.2
-     * @return true if width of the widget is determined by its content
+     * @return {@code true} if width of the widget is determined by its content,
+     *         {@code false} otherwise
      */
     protected boolean isDynamicWidth() {
         return getConnectorForWidget(this).isUndefinedWidth();
     }
 
     /**
-     * Returns true if the height of the widget is undefined, false otherwise.
+     * Returns whether the height of the widget is undefined.
      *
      * @since 7.2
-     * @return true if width of the height is determined by its content
+     * @return {@code true} if height of the widget is determined by its
+     *         content, {@code false} otherwise
      */
     protected boolean isDynamicHeight() {
         return getConnectorForWidget(this).isUndefinedHeight();
@@ -119,6 +152,7 @@ public abstract class VTabsheetBase extends ComplexPanel implements HasEnabled {
      *
      * @since 7.2
      * @param connector
+     *            the connector of this widget
      */
     public void setConnector(AbstractComponentConnector connector) {
         this.connector = connector;
@@ -130,7 +164,15 @@ public abstract class VTabsheetBase extends ComplexPanel implements HasEnabled {
         disabledTabKeys.clear();
     }
 
-    /** For internal use only. May be removed or replaced in the future. */
+    /**
+     * For internal use only. May be removed or replaced in the future.
+     *
+     * @param key
+     *            an internal key that corresponds with a tab
+     * @param disabled
+     *            {@code true} if the tab should be disabled, {@code false}
+     *            otherwise
+     */
     public void addTabKey(String key, boolean disabled) {
         tabKeys.add(key);
         if (disabled) {
@@ -138,12 +180,22 @@ public abstract class VTabsheetBase extends ComplexPanel implements HasEnabled {
         }
     }
 
-    /** For internal use only. May be removed or replaced in the future. */
+    /**
+     * For internal use only. May be removed or replaced in the future.
+     *
+     * @param client
+     *            the current application connection instance
+     */
     public void setClient(ApplicationConnection client) {
         this.client = client;
     }
 
-    /** For internal use only. May be removed or replaced in the future. */
+    /**
+     * For internal use only. May be removed or replaced in the future.
+     *
+     * @param activeTabIndex
+     *            the index of the currently active tab
+     */
     public void setActiveTabIndex(int activeTabIndex) {
         this.activeTabIndex = activeTabIndex;
     }
@@ -154,17 +206,34 @@ public abstract class VTabsheetBase extends ComplexPanel implements HasEnabled {
         disabled = !enabled;
     }
 
-    /** For internal use only. May be removed or replaced in the future. */
+    /**
+     * For internal use only. May be removed or replaced in the future.
+     *
+     * @param readonly
+     *            {@code true} if this widget should be read-only, {@code false}
+     *            otherwise
+     */
     public void setReadonly(boolean readonly) {
         this.readonly = readonly;
     }
 
-    /** For internal use only. May be removed or replaced in the future. */
+    /**
+     * For internal use only. May be removed or replaced in the future.
+     *
+     * @param widget
+     *            the widget whose connector to find
+     * @return the connector
+     */
     protected ComponentConnector getConnectorForWidget(Widget widget) {
         return ConnectorMap.get(client).getConnector(widget);
     }
 
-    /** For internal use only. May be removed or replaced in the future. */
+    /**
+     * For internal use only. May be removed or replaced in the future.
+     *
+     * @param index
+     *            the index of the tab to select
+     */
     public abstract void selectTab(int index);
 
     @Override
@@ -176,6 +245,8 @@ public abstract class VTabsheetBase extends ComplexPanel implements HasEnabled {
      * Sets whether the caption is rendered as HTML.
      * <p>
      * The default is false, i.e. render tab captions as plain text
+     * <p>
+     * This value is delegated from the TabsheetState.
      *
      * @since 7.4
      * @param tabCaptionsAsHtml

client/src/main/java/com/vaadin/client/ui/VTabsheetPanel.java

@@ -27,12 +27,11 @@ import com.vaadin.client.ui.TouchScrollDelegate.TouchScrollHandler;
 
 /**
  * A panel that displays all of its child widgets in a 'deck', where only one
- * can be visible at a time. It is used by
- * {@link com.vaadin.client.ui.VTabsheet}.
+ * can be visible at a time. It is used by {@link VTabsheet}.
  *
- * This class has the same basic functionality as the GWT DeckPanel
- * {@link com.google.gwt.user.client.ui.DeckPanel}, with the exception that it
- * doesn't manipulate the child widgets' width and height attributes.
+ * This class has the same basic functionality as the GWT
+ * {@link com.google.gwt.user.client.ui.DeckPanel DeckPanel}, with the exception
+ * that it doesn't manipulate the child widgets' width and height attributes.
  */
 public class VTabsheetPanel extends ComplexPanel {
 
@@ -43,6 +42,7 @@ public class VTabsheetPanel extends ComplexPanel {
     /**
      * Creates an empty tabsheet panel.
      */
+    @SuppressWarnings("deprecation")
     public VTabsheetPanel() {
         setElement(DOM.createDiv());
         touchScrollHandler = TouchScrollDelegate.enableTouchScrolling(this);
@@ -107,7 +107,7 @@ public class VTabsheetPanel extends ComplexPanel {
                 visibleWidget = null;
             }
             if (parent != null) {
-                DOM.removeChild(getElement(), parent);
+                getElement().removeChild(parent);
             }
             touchScrollHandler.removeElement(parent);
         }
@@ -150,6 +150,19 @@ public class VTabsheetPanel extends ComplexPanel {
         e.getStyle().clearVisibility();
     }
 
+    /**
+     * Updates the size of the visible widget.
+     *
+     * @param width
+     *            the width to set (in pixels), or negative if the width should
+     *            be dynamic (final width might get overridden by the minimum
+     *            width if that is larger)
+     * @param height
+     *            the height to set (in pixels), or negative if the height
+     *            should be dynamic
+     * @param minWidth
+     *            the minimum width (in pixels) that can be set
+     */
     public void fixVisibleTabSize(int width, int height, int minWidth) {
         if (visibleWidget == null) {
             return;
@@ -190,6 +203,14 @@ public class VTabsheetPanel extends ComplexPanel {
         }
     }
 
+    /**
+     * Removes the old component and sets the new component to its place.
+     *
+     * @param oldComponent
+     *            the component to remove
+     * @param newComponent
+     *            the component to add to the old location
+     */
     public void replaceComponent(Widget oldComponent, Widget newComponent) {
         boolean isVisible = (visibleWidget == oldComponent);
         int widgetIndex = getWidgetIndex(oldComponent);

client/src/main/java/com/vaadin/client/ui/accordion/AccordionConnector.java

@@ -56,6 +56,7 @@ public class AccordionConnector extends TabsheetBaseConnector
             StackItem selectedItem = widget
                     .getStackItem(widget.selectedItemIndex);
 
+            // Only the visible child widget is present in the collection.
             ComponentConnector contentConnector = getChildComponents().get(0);
             if (contentConnector != null) {
                 selectedItem.setContent(contentConnector.getWidget());

client/src/main/java/com/vaadin/client/ui/tabsheet/TabsheetBaseConnector.java

@@ -63,11 +63,22 @@ public abstract class TabsheetBaseConnector
         // Update member references
         widget.setEnabled(isEnabled());
 
-        // Widgets in the TabSheet before update
+        // Widgets in the TabSheet before update (should be max 1)
         List<Widget> oldWidgets = new ArrayList<>();
         for (Iterator<Widget> iterator = widget.getWidgetIterator(); iterator
                 .hasNext();) {
-            oldWidgets.add(iterator.next());
+            Widget child = iterator.next();
+            // filter out any current widgets (should be max 1)
+            boolean found = false;
+            for (ComponentConnector childComponent : getChildComponents()) {
+                if (childComponent.getWidget().equals(child)) {
+                    found = true;
+                    break;
+                }
+            }
+            if (!found) {
+                oldWidgets.add(child);
+            }
         }
 
         // Clear previous values
@@ -95,14 +106,6 @@ public abstract class TabsheetBaseConnector
             widget.removeTab(index);
         }
 
-        for (int i = 0; i < widget.getTabCount(); i++) {
-            ComponentConnector p = widget.getTab(i);
-            // null for PlaceHolder widgets
-            if (p != null) {
-                oldWidgets.remove(p.getWidget());
-            }
-        }
-
         // Detach any old tab widget, should be max 1
         for (Widget oldWidget : oldWidgets) {
             if (oldWidget.isAttached()) {

uitest/src/main/java/com/vaadin/tests/components/tabsheet/ScrolledTabSheetHiddenTabsResize.java

@@ -0,0 +1,20 @@
+package com.vaadin.tests.components.tabsheet;
+
+import com.vaadin.ui.Label;
+import com.vaadin.ui.TabSheet;
+import com.vaadin.ui.TabSheet.Tab;
+
+public class ScrolledTabSheetHiddenTabsResize extends ScrolledTabSheetResize {
+
+    @Override
+    protected void populate(TabSheet tabSheet) {
+        for (int i = 0; i < 40; i++) {
+            String caption = "Tab " + i;
+            Label label = new Label(caption);
+
+            Tab tab = tabSheet.addTab(label, caption);
+            tab.setClosable(true);
+            tab.setVisible(i % 2 != 0);
+        }
+    }
+}

uitest/src/main/java/com/vaadin/tests/components/tabsheet/ScrolledTabSheetResize.java

@@ -21,6 +21,15 @@ public class ScrolledTabSheetResize extends AbstractTestUI {
         TabSheet tabSheet = new TabSheet();
         tabSheet.setSizeFull();
 
+        populate(tabSheet);
+
+        addComponent(tabSheet);
+        addComponent(new Button("use reindeer", e -> {
+            setTheme("reindeer");
+        }));
+    }
+
+    protected void populate(TabSheet tabSheet) {
         for (int i = 0; i < 20; i++) {
             String caption = "Tab " + i;
             Label label = new Label(caption);
@@ -28,11 +37,6 @@ public class ScrolledTabSheetResize extends AbstractTestUI {
             Tab tab = tabSheet.addTab(label, caption);
             tab.setClosable(true);
         }
-
-        addComponent(tabSheet);
-        addComponent(new Button("use reindeer", e -> {
-            setTheme("reindeer");
-        }));
     }
 
     @Override

uitest/src/test/java/com/vaadin/tests/components/tabsheet/ScrolledTabSheetHiddenTabsResizeTest.java

@@ -0,0 +1,31 @@
+package com.vaadin.tests.components.tabsheet;
+
+import java.util.List;
+
+import org.openqa.selenium.WebElement;
+
+public class ScrolledTabSheetHiddenTabsResizeTest
+        extends ScrolledTabSheetResizeTest {
+
+    @Override
+    public void setup() throws Exception {
+        lastVisibleTabCaption = "Tab 39";
+        super.setup();
+    }
+
+    @Override
+    protected WebElement getFirstHiddenViewable(List<WebElement> tabs) {
+        // every other tab is hidden on server, return the second-to-last tab
+        // before the first one that is visible on client
+        WebElement previous = null;
+        WebElement older = null;
+        for (WebElement tab : tabs) {
+            if (hasCssClass(tab, "v-tabsheet-tabitemcell-first")) {
+                break;
+            }
+            older = previous;
+            previous = tab;
+        }
+        return older;
+    }
+}

uitest/src/test/java/com/vaadin/tests/components/tabsheet/ScrolledTabSheetResizeTest.java

@@ -7,7 +7,9 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 import org.junit.Test;
 import org.openqa.selenium.By;
@@ -16,10 +18,15 @@ import org.openqa.selenium.WebElement;
 import com.vaadin.testbench.TestBenchElement;
 import com.vaadin.testbench.elements.ButtonElement;
 import com.vaadin.testbench.elements.TabSheetElement;
+import com.vaadin.testbench.elements.UIElement;
 import com.vaadin.tests.tb3.MultiBrowserTest;
 
 public class ScrolledTabSheetResizeTest extends MultiBrowserTest {
 
+    protected String lastVisibleTabCaption = "Tab 19";
+
+    private WebElement pendingTab = null;
+
     @Override
     public void setup() throws Exception {
         super.setup();
@@ -29,13 +36,14 @@ public class ScrolledTabSheetResizeTest extends MultiBrowserTest {
     @Test
     public void testReindeer() throws IOException, InterruptedException {
         $(ButtonElement.class).first().click();
+        Map<String, Integer> sizes = saveWidths();
         StringBuilder exceptions = new StringBuilder();
         boolean failed = false;
         // upper limit is determined by the amount of tabs,
         // lower end by limits set by Selenium version
         for (int i = 1400; i >= 650; i = i - 50) {
             try {
-                testResize(i);
+                testResize(i, sizes);
             } catch (Exception e) {
                 if (failed) {
                     exceptions.append(" --- ");
@@ -61,13 +69,14 @@ public class ScrolledTabSheetResizeTest extends MultiBrowserTest {
 
     @Test
     public void testValo() throws IOException, InterruptedException {
+        Map<String, Integer> sizes = saveWidths();
         StringBuilder exceptions = new StringBuilder();
         boolean failed = false;
         // 1550 would be better for the amount of tabs (wider than for
         // reindeer), but IE11 can't adjust that far
         for (int i = 1500; i >= 650; i = i - 50) {
             try {
-                testResize(i);
+                testResize(i, sizes);
             } catch (Exception e) {
                 if (failed) {
                     exceptions.append(" --- ");
@@ -91,10 +100,56 @@ public class ScrolledTabSheetResizeTest extends MultiBrowserTest {
         }
     }
 
-    private void testResize(int start)
+    private Map<String, Integer> saveWidths() {
+        // save the tab widths before any scrolling
+        TabSheetElement ts = $(TabSheetElement.class).first();
+        Map<String, Integer> sizes = new HashMap<>();
+        for (WebElement tab : ts
+                .findElements(By.className("v-tabsheet-tabitemcell"))) {
+            if (hasCssClass(tab, "v-tabsheet-tabitemcell-first")) {
+                // skip the first visible for now, it has different styling and
+                // we are interested in the non-styled width
+                pendingTab = tab;
+                continue;
+            }
+            if (pendingTab != null && tab.isDisplayed()) {
+                String currentLeft = tab.getCssValue("padding-left");
+                String pendingLeft = pendingTab.getCssValue("padding-left");
+                if (currentLeft == null || "0px".equals(currentLeft)) {
+                    currentLeft = tab.findElement(By.className("v-caption"))
+                            .getCssValue("margin-left");
+                    pendingLeft = pendingTab
+                            .findElement(By.className("v-caption"))
+                            .getCssValue("margin-left");
+                }
+                if (currentLeft != pendingLeft && currentLeft.endsWith("px")
+                        && pendingLeft.endsWith("px")) {
+                    WebElement caption = pendingTab
+                            .findElement(By.className("v-captiontext"));
+                    sizes.put(caption.getAttribute("innerText"),
+                            pendingTab.getSize().getWidth()
+                                    - intValue(pendingLeft)
+                                    + intValue(currentLeft));
+                }
+                pendingTab = null;
+            }
+            WebElement caption = tab.findElement(By.className("v-captiontext"));
+            sizes.put(caption.getAttribute("innerText"),
+                    tab.getSize().getWidth());
+        }
+        return sizes;
+    }
+
+    private Integer intValue(String pixelString) {
+        return Integer
+                .valueOf(pixelString.substring(0, pixelString.indexOf("px")));
+    }
+
+    private void testResize(int start, Map<String, Integer> sizes)
             throws IOException, InterruptedException {
-        testBench().resizeViewPortTo(start, 600);
+        resizeViewPortTo(start);
         waitUntilLoadingIndicatorNotVisible();
+        sleep(100); // a bit more for layouting
 
         int iterations = 0;
         while (scrollRight() && iterations < 50) {
@@ -102,28 +157,35 @@ public class ScrolledTabSheetResizeTest extends MultiBrowserTest {
             ++iterations;
         }
 
-        // FIXME: TabSheet definitely still has issues,
-        // but it's moving to a better direction.
-
-        // Sometimes the test never realises that scrolling has
-        // reached the end, but this is not critical as long as
-        // the other criteria is fulfilled.
-        // If we decide otherwise, uncomment the following check:
-        // if (iterations >= 50) {
-        // fail("scrolling right never reaches the end");
-        // }
-
-        // This fails on some specific widths by ~15-20 pixels, likewise
-        // deemed as non-critical for now so commented out.
-        // assertNoExtraRoom(start);
+        if (iterations >= 50) {
+            fail("scrolling right never reaches the end");
+        }
+        assertNoExtraRoom(start, sizes);
 
-        testBench().resizeViewPortTo(start + 150, 600);
+        resizeViewPortTo(start + 150);
         waitUntilLoadingIndicatorNotVisible();
+        sleep(100); // a bit more for layouting
 
-        assertNoExtraRoom(start + 150);
+        assertNoExtraRoom(start + 150, sizes);
     }
 
-    private void assertNoExtraRoom(int width) {
+    private void resizeViewPortTo(int width) {
+        try {
+            testBench().resizeViewPortTo(width, 600);
+        } catch (UnsupportedOperationException e) {
+            // sometimes this exception is thrown even if resize succeeded, test
+            // validity
+            waitUntilLoadingIndicatorNotVisible();
+            UIElement ui = $(UIElement.class).first();
+            int currentWidth = ui.getSize().width;
+            if (currentWidth != width) {
+                // only throw the exception if the size didn't change
+                throw e;
+            }
+        }
+    }
+
+    private void assertNoExtraRoom(int width, Map<String, Integer> sizes) {
         TabSheetElement ts = $(TabSheetElement.class).first();
         WebElement scroller = ts
                 .findElement(By.className("v-tabsheet-scroller"));
@@ -131,19 +193,35 @@ public class ScrolledTabSheetResizeTest extends MultiBrowserTest {
                 .findElements(By.className("v-tabsheet-tabitemcell"));
         WebElement lastTab = tabs.get(tabs.size() - 1);
 
-        assertEquals("Tab 19",
+        assertEquals("Unexpected last visible tab,", lastVisibleTabCaption,
                 lastTab.findElement(By.className("v-captiontext")).getText());
 
+        WebElement firstHidden = getFirstHiddenViewable(tabs);
+        if (firstHidden == null) {
+            // nothing to scroll to
+            return;
+        }
+        // the sizes change during a tab's life-cycle, use the recorded size
+        // approximation for how much extra space adding this tab would need
+        // (measuring a hidden tab would definitely give too small width)
+        WebElement caption = firstHidden
+                .findElement(By.className("v-captiontext"));
+        String captionText = caption.getAttribute("innerText");
+        Integer firstHiddenWidth = sizes.get(captionText);
+        if (firstHiddenWidth == null) {
+            firstHiddenWidth = sizes.get("Tab 3");
+        }
+
         int tabWidth = lastTab.getSize().width;
         int tabRight = lastTab.getLocation().x + tabWidth;
-        int scrollerLeft = scroller.findElement(By.tagName("button"))
-                .getLocation().x;
+        assertThat("Unexpected tab width", tabRight, greaterThan(20));
 
-        assertThat("Not scrolled to the end (width: " + width + ")",
-                scrollerLeft, greaterThan(tabRight));
-        // technically this should probably be just greaterThan,
+        int scrollerLeft = scroller.getLocation().x;
+        // technically these should probably be just greaterThan,
         // but one pixel's difference is irrelevant for now
-        assertThat("Too big gap (width: " + width + ")", tabWidth,
+        assertThat("Not scrolled to the end (width: " + width + ")",
+                scrollerLeft, greaterThanOrEqualTo(tabRight));
+        assertThat("Too big gap (width: " + width + ")", firstHiddenWidth,
                 greaterThanOrEqualTo(scrollerLeft - tabRight));
     }
 
@@ -163,4 +241,18 @@ public class ScrolledTabSheetResizeTest extends MultiBrowserTest {
         }
     }
 
+    /*
+     * There is no way to differentiate between hidden-on-server and
+     * hidden-on-client here, so this method has to be overridable.
+     */
+    protected WebElement getFirstHiddenViewable(List<WebElement> tabs) {
+        WebElement previous = null;
+        for (WebElement tab : tabs) {
+            if (hasCssClass(tab, "v-tabsheet-tabitemcell-first")) {
+                break;
+            }
+            previous = tab;
+        }
+        return previous;
+    }
 }

uitest/src/test/java/com/vaadin/tests/components/tabsheet/TabSheetFocusedTabTest.java

@@ -24,22 +24,53 @@ public class TabSheetFocusedTabTest extends MultiBrowserTest {
 
         getTab(1).click();
 
-        assertTrue(isFocused(getTab(1)));
+        assertTrue("Tab 1 should have been focused but wasn't.",
+                isFocused(getTab(1)));
 
         new Actions(getDriver()).sendKeys(Keys.ARROW_RIGHT).perform();
 
-        assertFalse(isFocused(getTab(1)));
-        assertTrue(isFocused(getTab(3)));
+        assertFalse("Tab 1 was focused but shouldn't have been.",
+                isFocused(getTab(1)));
+        assertTrue("Tab 3 should have been focused but wasn't.",
+                isFocused(getTab(3)));
 
         getTab(5).click();
 
-        assertFalse(isFocused(getTab(3)));
-        assertTrue(isFocused(getTab(5)));
+        assertFalse("Tab 3 was focused but shouldn't have been.",
+                isFocused(getTab(3)));
+        assertTrue("Tab 5 should have been focused but wasn't.",
+                isFocused(getTab(5)));
 
         getTab(1).click();
 
-        assertFalse(isFocused(getTab(5)));
-        assertTrue(isFocused(getTab(1)));
+        assertFalse("Tab 5 was focused but shouldn't have been.",
+                isFocused(getTab(5)));
+        assertTrue("Tab 1 should have been focused but wasn't.",
+                isFocused(getTab(1)));
+    }
+
+    @Test
+    public void scrollingChangesFocusedTab() {
+        openTestURL();
+
+        getTab(7).click();
+
+        assertTrue("Tab 7 should have been focused but wasn't.",
+                isFocused(getTab(7)));
+
+        findElement(By.className("v-tabsheet-scrollerNext")).click();
+
+        assertFalse("Tab 7 was focused but shouldn't have been.",
+                isFocused(getTab(7)));
+        assertTrue("Tab 3 should have been focused but wasn't.",
+                isFocused(getTab(3)));
+
+        new Actions(getDriver()).sendKeys(Keys.ARROW_RIGHT).perform();
+
+        assertFalse("Tab 3 was focused but shouldn't have been.",
+                isFocused(getTab(3)));
+        assertTrue("Tab 5 should have been focused but wasn't.",
+                isFocused(getTab(5)));
     }
 
     private WebElement getTab(int index) {
@@ -49,7 +80,6 @@ public class TabSheetFocusedTabTest extends MultiBrowserTest {
     }
 
     private boolean isFocused(WebElement tab) {
-
         return tab.getAttribute("class").contains("v-tabsheet-tabitem-focus");
     }