Fix NPE and scroll behavior in TabSheet (#14348).

After removing a tab, the TabSheet now scrolls when it is necessary to
show the remaining tabs. A test (TabSheetScrollOnTabCloseTest) ensures
that no scrolling occurs when the removed tab is not in the visible
area.

Change-Id: I81e4e504167ec4d0a527e6bfe94dba8b29fb26bc

5 files changed, 462 insertions, 2 deletions

diff --git a/client/src/com/vaadin/client/ui/VTabsheet.java b/client/src/com/vaadin/client/ui/VTabsheet.java
index 745f2bca61..bcca117395 100644
--- a/client/src/com/vaadin/client/ui/VTabsheet.java
+++ b/client/src/com/vaadin/client/ui/VTabsheet.java
@@ -512,6 +512,18 @@ public class VTabsheet extends VTabsheetBase implements Focusable, SubPartAware
             return (Tab) super.getWidget(index);
         }
 
+        private int getTabIndex(String tabId) {
+            if (tabId == null) {
+                return -1;
+            }
+            for (int i = 0; i < getTabCount(); i++) {
+                if (tabId.equals(getTab(i).id)) {
+                    return i;
+                }
+            }
+            return -1;
+        }
+
         public void selectTab(int index) {
             final Tab newSelected = getTab(index);
             final Tab oldSelected = selected;
@@ -572,8 +584,40 @@ public class VTabsheet extends VTabsheetBase implements Focusable, SubPartAware
             if (tab == selected) {
                 selected = null;
             }
-
             // FIXME: Shouldn't something be selected instead?
+
+            int scrollerIndexCandidate = getTabIndex(getTabsheet().scrollerPositionTabId);
+            if (scrollerIndexCandidate < 0) {
+                // The tab with id scrollerPositionTabId has been removed
+                scrollerIndexCandidate = getTabsheet().scrollerIndex;
+            }
+            scrollerIndexCandidate = selectNewShownTab(scrollerIndexCandidate);
+            if (scrollerIndexCandidate >= 0
+                    && scrollerIndexCandidate < getTabCount()) {
+                getTabsheet().scrollIntoView(getTab(scrollerIndexCandidate));
+            }
+        }
+
+        private int selectNewShownTab(int oldPosition) {
+            // After removing a tab, find a new scroll position. In most
+            // cases the scroll position does not change, but if the tab
+            // at the scroll position was removed, need to find a nearby
+            // tab that is visible.
+            for (int i = oldPosition; i < getTabCount(); i++) {
+                Tab tab = getTab(i);
+                if (!tab.isHiddenOnServer()) {
+                    return i;
+                }
+            }
+
+            for (int i = oldPosition - 1; i >= 0; i--) {
+                Tab tab = getTab(i);
+                if (!tab.isHiddenOnServer()) {
+                    return i;
+                }
+            }
+
+            return -1;
         }
 
         private boolean isFirstVisibleTab(int index) {
@@ -685,6 +729,14 @@ public class VTabsheet extends VTabsheetBase implements Focusable, SubPartAware
      * The index of the first visible tab (when scrolled)
      */
     private int scrollerIndex = 0;
+    /**
+     * The id of the tab at position scrollerIndex. This is used for keeping the
+     * scroll position unchanged when a tab is removed from the server side and
+     * the removed tab lies to the left of the current scroll position. For other
+     * cases scrollerIndex alone would be sufficient. Since the tab at the current
+     * scroll position can be removed, scrollerIndex is required in addition to this variable.
+     */
+    private String scrollerPositionTabId;
 
     final TabBar tb = new TabBar(this);
     /** For internal use only. May be removed or replaced in the future. */
@@ -911,6 +963,7 @@ public class VTabsheet extends VTabsheetBase implements Focusable, SubPartAware
 
         if (newFirstIndex != -1) {
             scrollerIndex = newFirstIndex;
+            scrollerPositionTabId = tb.getTab(scrollerIndex).id;
             updateTabScroller();
         }
 
@@ -1206,6 +1259,8 @@ public class VTabsheet extends VTabsheetBase implements Focusable, SubPartAware
     /** For internal use only. May be removed or replaced in the future. */
     public void showAllTabs() {
         scrollerIndex = tb.getFirstVisibleTab();
+        scrollerPositionTabId = scrollerIndex < 0 ? null : tb
+                .getTab(scrollerIndex).id;
         for (int i = 0; i < tb.getTabCount(); i++) {
             Tab t = tb.getTab(i);
             if (!t.isHiddenOnServer()) {
@@ -1780,12 +1835,18 @@ public class VTabsheet extends VTabsheetBase implements Focusable, SubPartAware
                 }
                 updateTabScroller();
             }
+            if (scrollerIndex >= 0 && scrollerIndex < tb.getTabCount()) {
+                scrollerPositionTabId = tb.getTab(scrollerIndex).id;
+            }
+            else{
+                scrollerPositionTabId = null;
+            }
         }
     }
 
     /**
      * Makes tab bar visible.
-     *
+     * 
      * @since 7.2
      */
     public void showTabs() {
diff --git a/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetClose.java b/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetClose.java
new file mode 100644
index 0000000000..2cadde567f
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetClose.java
@@ -0,0 +1,72 @@
+/*
+ * 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.tabsheet;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.CssLayout;
+import com.vaadin.ui.TabSheet;
+import com.vaadin.ui.TabSheet.Tab;
+
+/**
+ * This test UI is used for checking that when a tab is closed, another one is
+ * scrolled into view.
+ * 
+ * @since
+ * @author Vaadin Ltd
+ */
+public class TabSheetClose extends AbstractTestUI {
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#setup(com.vaadin.server.
+     * VaadinRequest)
+     */
+    @Override
+    protected void setup(VaadinRequest request) {
+        TabSheet tabsheet = new TabSheet();
+        for (int loop = 0; loop < 3; loop++) {
+            Tab tab = tabsheet.addTab(new CssLayout(), "tab " + loop);
+            tab.setClosable(true);
+            tab.setId("tab" + loop);
+        }
+        CssLayout layout = new CssLayout();
+        layout.addComponent(tabsheet);
+        layout.setWidth("150px");
+        addComponent(layout);
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription()
+     */
+    @Override
+    protected String getTestDescription() {
+        return "When all tabs have not been closed, at least one tab should be visible. ";
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTicketNumber()
+     */
+    @Override
+    protected Integer getTicketNumber() {
+        return 14348;
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetCloseTest.java b/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetCloseTest.java
new file mode 100644
index 0000000000..5314038d72
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetCloseTest.java
@@ -0,0 +1,58 @@
+/*
+ * 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.tabsheet;
+
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+import org.openqa.selenium.By;
+
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * Tests that when closing the last tab on a TabSheet, another tab gets selected
+ * with no error. Only the last tab should be visible, so the actual TabSheet
+ * width should be small.
+ * 
+ * @since
+ * @author Vaadin Ltd
+ */
+public class TabSheetCloseTest extends MultiBrowserTest {
+
+    private static final String TAB_CLOSE = "//span[@class = 'v-tabsheet-caption-close']";
+    private static final String LAST_TAB = "//*[@id = 'tab2']/div/div";
+    private static final String SCROLLER_NEXT = "//button[@class = 'v-tabsheet-scrollerNext']";
+    private static final String FIRST_TAB = "//*[@id = 'tab0']";
+    private static final String SECOND_TAB = "//*[@id = 'tab1']";
+
+    @Test
+    public void testClosingOfLastTab() throws Exception {
+        openTestURL();
+
+        // Click next button twice to get to the last tab
+        findElement(By.xpath(SCROLLER_NEXT)).click();
+        findElement(By.xpath(SCROLLER_NEXT)).click();
+
+        findElement(By.xpath(LAST_TAB)).click();
+
+        // Closing last tab will take back to the second tab. Closing that
+        // will leave the first tab visible.
+        findElements(By.xpath(TAB_CLOSE)).get(2).click();
+        assertTrue(findElement(By.xpath(SECOND_TAB)).isDisplayed());
+        findElements(By.xpath(TAB_CLOSE)).get(1).click();
+        assertTrue(findElement(By.xpath(FIRST_TAB)).isDisplayed());
+    }
+}
\ No newline at end of file
diff --git a/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetScrollOnTabClose.java b/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetScrollOnTabClose.java
new file mode 100644
index 0000000000..1400100f3b
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetScrollOnTabClose.java
@@ -0,0 +1,98 @@
+/*
+ * 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.tabsheet;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.CssLayout;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.TabSheet;
+import com.vaadin.ui.TabSheet.Tab;
+
+/**
+ * This testUI is used for testing that the scroll position of a tab sheet does
+ * not change when tabs are removed. The exception is removing the leftmost
+ * visible tab.
+ * 
+ * @since
+ * @author Vaadin Ltd
+ */
+public class TabSheetScrollOnTabClose extends AbstractTestUI {
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#setup(com.vaadin.server.
+     * VaadinRequest)
+     */
+    @Override
+    protected void setup(VaadinRequest request) {
+        final TabSheet tabSheet = new TabSheet();
+        for (int i = 0; i < 10; i++) {
+            Tab tab = tabSheet.addTab(new CssLayout(), "tab " + i);
+            tab.setClosable(true);
+            tab.setId("tab" + i);
+        }
+        tabSheet.setWidth(250, Unit.PIXELS);
+        addComponent(tabSheet);
+        addComponent(new Label("Close tab number"));
+        for (int i = 0; i < 10; i++) {
+            final String tabCaption = "tab " + i;
+            final Button b = new Button("" + i);
+            b.addClickListener(new Button.ClickListener() {
+
+                @Override
+                public void buttonClick(ClickEvent event) {
+                    b.setEnabled(false);
+                    tabSheet.removeTab(getTab(tabSheet, tabCaption));
+                }
+            });
+            addComponent(b);
+        }
+    }
+
+    private Tab getTab(TabSheet ts, String tabCaption) {
+        for (int i = 0; i < ts.getComponentCount(); i++) {
+            String caption = ts.getTab(i).getCaption();
+            if (tabCaption.equals(caption)) {
+                return ts.getTab(i);
+            }
+        }
+        return null;
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription()
+     */
+    @Override
+    protected String getTestDescription() {
+        return "Scroll position should not change when closing tabs.";
+    }
+
+    /*
+     * (non-Javadoc)
+     * 
+     * @see com.vaadin.tests.components.AbstractTestUI#getTicketNumber()
+     */
+    @Override
+    protected Integer getTicketNumber() {
+        return 14348;
+    }
+}
\ No newline at end of file
diff --git a/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetScrollOnTabCloseTest.java b/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetScrollOnTabCloseTest.java
new file mode 100644
index 0000000000..8247b436d0
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/tabsheet/TabSheetScrollOnTabCloseTest.java
@@ -0,0 +1,171 @@
+/*
+ * 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.tabsheet;
+
+import java.util.List;
+import java.util.NoSuchElementException;
+
+import org.junit.Test;
+import org.openqa.selenium.StaleElementReferenceException;
+import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.support.ui.ExpectedCondition;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.TabSheetElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * Tests removing tabs that have been scrolled out of view. This should cause no
+ * change to the scroll position.
+ * 
+ * @since
+ * @author Vaadin Ltd
+ */
+public class TabSheetScrollOnTabCloseTest extends MultiBrowserTest {
+
+    @Test
+    public void testScrollPositionAfterClosing() throws Exception {
+        openTestURL();
+        TabSheetElement ts = $(TabSheetElement.class).first();
+        WebElement tabSheetScroller = ts.findElement(By
+                .className("v-tabsheet-scrollerNext"));
+        // scroll to the right
+        for (int i = 0; i < 4; i++) {
+            tabSheetScroller.click();
+        }
+        // check that tab 4 is the first visible tab
+        checkDisplayedStatus(ts, "tab3", false);
+        checkDisplayedStatus(ts, "tab4", true);
+        // remove tabs from the left, check that tab4 is still the first visible
+        // tab
+        for (int i = 0; i < 4; i++) {
+            $(ButtonElement.class).get(i).click();
+            checkDisplayedStatus(ts, "tab3" + i, false);
+            checkDisplayedStatus(ts, "tab4", true);
+            checkDisplayedStatus(ts, "tab6", true);
+        }
+        // remove tabs from the right and check scroll position
+        for (int i = 7; i < 10; i++) {
+            $(ButtonElement.class).get(i).click();
+            checkFirstTab(ts, "tab4");
+            checkDisplayedStatus(ts, "tab6", true);
+        }
+    }
+
+    /**
+     * Checks that the visible status of the tab with the given id is equal to
+     * shouldBeVisible. That is, the tab with the given id should be visible if
+     * and only if shouldBeVisible is true. Used for checking that the leftmost
+     * visible tab is the expected one when there should be tabs (hidden because
+     * of scroll position) to the left of tabId.
+     * 
+     * If there is no tab with the specified id, the tab is considered not to be
+     * visible.
+     */
+    private void checkDisplayedStatus(TabSheetElement tabSheet, String tabId,
+            boolean shouldBeVisible) {
+        org.openqa.selenium.By locator = By.cssSelector("#" + tabId);
+        waitUntil(visibilityOfElement(locator, shouldBeVisible));
+    }
+
+    /**
+     * Checks that there are no hidden tabs in tabSheet and that the id of the
+     * first tab is tabId. Used for checking that the leftmost visible tab is
+     * the expected one when there are no tabs to the left of the tab with the
+     * given id. When there are tabs to the left of tabId, check instead that
+     * tabId is visible and the previous tab is hidden (see
+     * checkDisplayedStatus).
+     */
+    private void checkFirstTab(TabSheetElement tabSheet, String tabId) {
+        waitUntil(visibilityOfElement(
+                By.cssSelector(".v-tabsheet-tabitemcell[aria-hidden]"), false));
+        waitUntil(leftmostTabHasId(tabSheet, tabId));
+    }
+
+    /**
+     * An expectation for checking that the visibility status of the specified
+     * element is correct. If the element does not exist in the DOM, it is
+     * considered not to be visible. If several elements match the locator, only
+     * the visibility of the first matching element is considered.
+     * 
+     * @param locator
+     *            used to find the element
+     * @param expectedVisibility
+     *            whether the element should be visible
+     */
+    private static ExpectedCondition<Boolean> visibilityOfElement(
+            final org.openqa.selenium.By locator,
+            final boolean expectedVisibility) {
+        return new ExpectedCondition<Boolean>() {
+            @Override
+            public Boolean apply(WebDriver driver) {
+                List<WebElement> matchingElements = driver
+                        .findElements(locator);
+                if (matchingElements.isEmpty()) {
+                    return !expectedVisibility;
+                } else {
+                    try {
+                        WebElement first = matchingElements.get(0);
+                        return first.isDisplayed() == expectedVisibility;
+                    } catch (StaleElementReferenceException e) {
+                        // The element was initially in DOM but has been
+                        // removed.
+                        return !expectedVisibility;
+                    }
+                }
+            }
+
+            @Override
+            public String toString() {
+                return "element " + (expectedVisibility ? "" : "not ")
+                        + "expected to be visible: " + locator;
+            }
+        };
+    }
+
+    /**
+     * An expectation for checking that the leftmost tab has id equal to tabId.
+     * 
+     * @param tabSheet
+     *            the tab sheet containing the tab
+     * @param tabId
+     *            the id of the tab that should be the leftmost tab
+     */
+    private static ExpectedCondition<Boolean> leftmostTabHasId(
+            final TabSheetElement tabSheet, final String tabId) {
+        return new ExpectedCondition<Boolean>() {
+            @Override
+            public Boolean apply(WebDriver driver) {
+                try {
+                    WebElement leftElement = tabSheet.findElement(By
+                            .cssSelector(".v-tabsheet-tabitemcell"));
+                    String leftId = leftElement.getAttribute("id");
+                    return leftId.equals(tabId);
+                } catch (NoSuchElementException e) {
+                    return false;
+                }
+            }
+
+            @Override
+            public String toString() {
+                return "expected tab index of the leftmost tab in the tab sheet: "
+                        + tabId;
+            }
+        };
+    }
+}
\ No newline at end of file