diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index 7a4410960..e837cb027 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -37,6 +37,7 @@ + 40414 - fixed selected/active sheet after removing sheet from workbook 44523 - fixed workbook sheet selection and focus 45000 - Fixed NPE in ListLevel when numberText is null 44985 - Properly update TextSpecInfoAtom when the parent text is changed diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 200c62e12..0d87990cd 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -34,6 +34,7 @@ + 40414 - fixed selected/active sheet after removing sheet from workbook 44523 - fixed workbook sheet selection and focus 45000 - Fixed NPE in ListLevel when numberText is null 44985 - Properly update TextSpecInfoAtom when the parent text is changed diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index d49c601dc..896ef19c1 100644 --- 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.

+ * + * 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).
+ * + * 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 e1afb453b..587e90792 100644 --- 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()); } }