Fix for bug 40414 - update selected/active sheet after removing sheet from workbook

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@657167 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml
index 7a44109601a1113fec991072505ee56a5fc2bafb..e837cb027c4ea201e881d39414f6a0480417566d 100644 (file)
--- a/src/documentation/content/xdocs/changes.xml
+++ b/src/documentation/content/xdocs/changes.xml
@@ -37,6 +37,7 @@
 
                <!-- Don't forget to update status.xml too! -->
         <release version="3.1-beta2" date="2008-05-??">
+           <action dev="POI-DEVELOPERS" type="fix">40414 - fixed selected/active sheet after removing sheet from workbook</action>
            <action dev="POI-DEVELOPERS" type="fix">44523 - fixed workbook sheet selection and focus</action>
            <action dev="POI-DEVELOPERS" type="fix">45000 - Fixed NPE in ListLevel when numberText is null</action>
            <action dev="POI-DEVELOPERS" type="fix">44985 - Properly update TextSpecInfoAtom when the parent text is changed</action>

diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml
index 200c62e1272b09c14f1c77fbeea701d1d3c76560..0d87990cdbbc786ad7c8f5f0b57d3bac69db026f 100644 (file)
--- a/src/documentation/content/xdocs/status.xml
+++ b/src/documentation/content/xdocs/status.xml
@@ -34,6 +34,7 @@
        <!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.1-beta2" date="2008-05-??">
+           <action dev="POI-DEVELOPERS" type="fix">40414 - fixed selected/active sheet after removing sheet from workbook</action>
            <action dev="POI-DEVELOPERS" type="fix">44523 - fixed workbook sheet selection and focus</action>
            <action dev="POI-DEVELOPERS" type="fix">45000 - Fixed NPE in ListLevel when numberText is null</action>
            <action dev="POI-DEVELOPERS" type="fix">44985 - Properly update TextSpecInfoAtom when the parent text is changed</action>

diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
index d49c601dcb7716be4a478de8b484507d373102c8..896ef19c14b027ec706003ce5626d2d39d662e0b 100644 (file)
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java
@@ -140,7 +140,7 @@ public class HSSFWorkbook extends POIDocument
 
     protected HSSFWorkbook( Workbook book )
     {
-       super(null, null);
+        super(null, null);
         workbook = book;
         sheets = new ArrayList( INITIAL_CAPACITY );
         names = new ArrayList( INITIAL_CAPACITY );
@@ -754,14 +754,54 @@ public class HSSFWorkbook extends POIDocument
     }
 
     /**
-     * removes sheet at the given index
+     * Removes sheet at the given index.<p/>
+     * 
+     * Care must be taken if the removed sheet is the currently active or only selected sheet in 
+     * the workbook. There are a few situations when Excel must have a selection and/or active 
+     * sheet. (For example when printing - see Bug 40414).<br/>
+     * 
+     * This method makes sure that if the removed sheet was active, another sheet will become
+     * active in its place.  Furthermore, if the removed sheet was the only selected sheet, another
+     * sheet will become selected.  The newly active/selected sheet will have the same index, or 
+     * one less if the removed sheet was the last in the workbook.
+     * 
      * @param index of the sheet  (0-based)
      */
+    public void removeSheetAt(int index) {
+        validateSheetIndex(index);
+        boolean wasActive = getSheetAt(index).isActive();
+        boolean wasSelected = getSheetAt(index).isSelected();
 
-    public void removeSheetAt(int index)
-    {
         sheets.remove(index);
         workbook.removeSheet(index);
+
+        // set the remaining active/selected sheet
+        int nSheets = sheets.size();
+        if (nSheets < 1) {
+            // nothing more to do if there are no sheets left
+            return;
+        }
+        // the index of the closest remaining sheet to the one just deleted
+        int newSheetIndex = index;
+        if (newSheetIndex >= nSheets) {
+            newSheetIndex = nSheets-1;
+        }
+        if (wasActive) {
+            setActiveSheet(newSheetIndex);
+        }
+
+        if (wasSelected) {
+            boolean someOtherSheetIsStillSelected = false;
+            for (int i =0; i < nSheets; i++) {
+                if (getSheetAt(i).isSelected()) {
+                    someOtherSheetIsStillSelected = true;
+                    break;
+                }
+            }
+            if (!someOtherSheetIsStillSelected) {
+                setSelectedTab(newSheetIndex);
+            }
+        }
     }
 
     /**

diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
index e1afb453bcfe59384cb150bf4c18a6d129ba7442..587e9079252afd27740387d1479af1f3b15efad5 100644 (file)
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
@@ -299,8 +299,81 @@ public final class TestHSSFWorkbook extends TestCase {
         }
     }
 
+
+    public void testActiveSheetAfterDelete_bug40414() {
+        HSSFWorkbook wb=new HSSFWorkbook();
+        HSSFSheet sheet0 = wb.createSheet("Sheet0");
+        HSSFSheet sheet1 = wb.createSheet("Sheet1");
+        HSSFSheet sheet2 = wb.createSheet("Sheet2");
+        HSSFSheet sheet3 = wb.createSheet("Sheet3");
+        HSSFSheet sheet4 = wb.createSheet("Sheet4");
+
+        // confirm default activation/selection
+        confirmActiveSelected(sheet0, true);
+        confirmActiveSelected(sheet1, false);
+        confirmActiveSelected(sheet2, false);
+        confirmActiveSelected(sheet3, false);
+        confirmActiveSelected(sheet4, false);
+
+        wb.setActiveSheet(3);
+        wb.setSelectedTab(3);
+
+        confirmActiveSelected(sheet0, false);
+        confirmActiveSelected(sheet1, false);
+        confirmActiveSelected(sheet2, false);
+        confirmActiveSelected(sheet3, true);
+        confirmActiveSelected(sheet4, false);
+
+        wb.removeSheetAt(3);
+        // after removing the only active/selected sheet, another should be active/selected in its place
+        if (!sheet4.isSelected()) {
+            throw new AssertionFailedError("identified bug 40414 a");
+        }
+        if (!sheet4.isActive()) {
+            throw new AssertionFailedError("identified bug 40414 b");
+        }
+
+        confirmActiveSelected(sheet0, false);
+        confirmActiveSelected(sheet1, false);
+        confirmActiveSelected(sheet2, false);
+        confirmActiveSelected(sheet4, true);
+
+        sheet3 = sheet4; // re-align local vars in this test case
+
+        // Some more cases of removing sheets
+
+        // Starting with a multiple selection, and different active sheet
+        wb.setSelectedTabs(new int[] { 1, 3, });
+        wb.setActiveSheet(2);
+        confirmActiveSelected(sheet0, false, false);
+        confirmActiveSelected(sheet1, false, true);
+        confirmActiveSelected(sheet2, true,  false);
+        confirmActiveSelected(sheet3, false, true);
+
+        // removing a sheet that is not active, and not the only selected sheet
+        wb.removeSheetAt(3);
+        confirmActiveSelected(sheet0, false, false);
+        confirmActiveSelected(sheet1, false, true);
+        confirmActiveSelected(sheet2, true,  false);
+
+        // removing the only selected sheet
+        wb.removeSheetAt(1);
+        confirmActiveSelected(sheet0, false, false);
+        confirmActiveSelected(sheet2, true,  true);
+
+        // The last remaining sheet should always be active+selected
+        wb.removeSheetAt(1);
+        confirmActiveSelected(sheet0, true,  true);
+    }
+
     private static void confirmActiveSelected(HSSFSheet sheet, boolean expected) {
-        assertEquals(expected, sheet.isActive());
-        assertEquals(expected, sheet.isSelected());
+        confirmActiveSelected(sheet, expected, expected);
+    }
+
+
+    private static void confirmActiveSelected(HSSFSheet sheet,
+            boolean expectedActive, boolean expectedSelected) {
+        assertEquals("active", expectedActive, sheet.isActive());
+        assertEquals("selected", expectedSelected, sheet.isSelected());
     }
 }