diff --git a/main/HSSF/UserModel/HSSFWorkbook.cs b/main/HSSF/UserModel/HSSFWorkbook.cs index cf45b4f06..1df4863b2 100644 --- a/main/HSSF/UserModel/HSSFWorkbook.cs +++ b/main/HSSF/UserModel/HSSFWorkbook.cs @@ -460,20 +460,64 @@ public void SetSheetOrder(String sheetname, int pos) } workbook.UpdateNamesAfterCellShift(shifter); + UpdateNamedRangesAfterSheetReorder(oldSheetIndex, pos); + UpdateActiveSheetAfterSheetReorder(oldSheetIndex, pos); + } + + /** + * copy-pasted from XSSFWorkbook#updateNamedRangesAfterSheetReorder(int, int) + *

+ * update sheet-scoped named ranges in this workbook after changing the sheet order + * of a sheet at oldIndex to newIndex. + * Sheets between these indices will move left or right by 1. + * + * @param oldIndex the original index of the re-ordered sheet + * @param newIndex the new index of the re-ordered sheet + */ + private void UpdateNamedRangesAfterSheetReorder(int oldIndex, int newIndex) + { + // update sheet index of sheet-scoped named ranges + foreach (HSSFName name in names) + { + int i = name.SheetIndex; + // name has sheet-level scope + if (i != -1) + { + // name refers to this sheet + if (i == oldIndex) + { + name.SheetIndex = newIndex; + } + // if oldIndex > newIndex then this sheet moved left and sheets between newIndex and oldIndex moved right + else if (newIndex <= i && i < oldIndex) + { + name.SheetIndex = i + 1; + } + // if oldIndex < newIndex then this sheet moved right and sheets between oldIndex and newIndex moved left + else if (oldIndex < i && i <= newIndex) + { + name.SheetIndex = i - 1; + } + } + } + } + + private void UpdateActiveSheetAfterSheetReorder(int oldIndex, int newIndex) + { // adjust active sheet if necessary int active = ActiveSheetIndex; - if (active == oldSheetIndex) + if (active == oldIndex) { // moved sheet was the active one - SetActiveSheet(pos); + SetActiveSheet(newIndex); } - else if ((active < oldSheetIndex && active < pos) || - (active > oldSheetIndex && active > pos)) + else if ((active < oldIndex && active < newIndex) || + (active > oldIndex && active > newIndex)) { // not affected } - else if (pos > oldSheetIndex) + else if (newIndex > oldIndex) { // moved sheet was below before and is above now => active is one less SetActiveSheet(active - 1); diff --git a/ooxml/XSSF/UserModel/XSSFWorkbook.cs b/ooxml/XSSF/UserModel/XSSFWorkbook.cs index b0d842f1a..0d7ebd7bf 100644 --- a/ooxml/XSSF/UserModel/XSSFWorkbook.cs +++ b/ooxml/XSSF/UserModel/XSSFWorkbook.cs @@ -1665,6 +1665,7 @@ public void SetSheetOrder(String sheetname, int pos) XSSFSheet sheet = sheets[idx]; sheets.RemoveAt(idx); sheets.Insert(pos, sheet); + // Reorder CT_Sheets CT_Sheets ct = workbook.sheets; CT_Sheet cts = ct.GetSheetArray(idx).Copy(); @@ -1674,34 +1675,75 @@ public void SetSheetOrder(String sheetname, int pos) //notify sheets List sheetArray = ct.sheet; - for (int i = 0; i < sheets.Count; i++) + for (int i = 0; i < sheetArray.Count; i++) { sheets[i].sheet = sheetArray[i]; } + UpdateNamedRangesAfterSheetReorder(idx, pos); + UpdateActiveSheetAfterSheetReorder(idx, pos); + } + + /** + * update sheet-scoped named ranges in this workbook after changing the sheet order + * of a sheet at oldIndex to newIndex. + * Sheets between these indices will move left or right by 1. + * + * @param oldIndex the original index of the re-ordered sheet + * @param newIndex the new index of the re-ordered sheet + */ + private void UpdateNamedRangesAfterSheetReorder(int oldIndex, int newIndex) + { + // update sheet index of sheet-scoped named ranges + foreach (XSSFName name in namedRanges) + { + int i = name.SheetIndex; + // name has sheet-level scope + if (i != -1) + { + // name refers to this sheet + if (i == oldIndex) + { + name.SheetIndex = newIndex; + } + // if oldIndex > newIndex then this sheet moved left and sheets between newIndex and oldIndex moved right + else if (newIndex <= i && i < oldIndex) + { + name.SheetIndex = i + 1; + } + // if oldIndex < newIndex then this sheet moved right and sheets between oldIndex and newIndex moved left + else if (oldIndex < i && i <= newIndex) + { + name.SheetIndex = i - 1; + } + } + } + } + + private void UpdateActiveSheetAfterSheetReorder(int oldIndex, int newIndex) + { // adjust active sheet if necessary int active = ActiveSheetIndex; - if (active == idx) + if (active == oldIndex) { - // Moved sheet was the active one - SetActiveSheet(pos); + // moved sheet was the active one + SetActiveSheet(newIndex); } - else if ((active < idx && active < pos) || - (active > idx && active > pos)) + else if ((active < oldIndex && active < newIndex) || + (active > oldIndex && active > newIndex)) { // not affected } - else if (pos > idx) + else if (newIndex > oldIndex) { - // Moved sheet was below before and is above now => active is one less + // moved sheet was below before and is above now => active is one less SetActiveSheet(active - 1); } else { - // remaining case: Moved sheet was higher than active before and is lower now => active is one more + // remaining case: moved sheet was higher than active before and is lower now => active is one more SetActiveSheet(active + 1); } - } /** diff --git a/testcases/main/SS/UserModel/BaseTestBugzillaIssues.cs b/testcases/main/SS/UserModel/BaseTestBugzillaIssues.cs index d6564c3aa..f6fb3920e 100644 --- a/testcases/main/SS/UserModel/BaseTestBugzillaIssues.cs +++ b/testcases/main/SS/UserModel/BaseTestBugzillaIssues.cs @@ -348,7 +348,7 @@ private static String CreateFunction(String name, int maxArgs) return fmla.ToString(); } - + [Test] public void Bug50681_TestAutoSize() { @@ -481,7 +481,7 @@ private double ComputeCellWidthManually(ICell cell0, IFont font) //TextLayout layout = new TextLayout(str.getIterator(), fontRenderContext); //width = ((layout.getBounds().getWidth() / 1) / 8); Font wfont = SheetUtil.IFont2Font(font); - width= (double)TextMeasurer.Measure(txt, new TextOptions(wfont)).Width; + width = (double)TextMeasurer.Measure(txt, new TextOptions(wfont)).Width; return width; } @@ -492,7 +492,7 @@ private double ComputeCellWidthFixed(IFont font, String txt) width = (double)TextMeasurer.Measure(txt, new TextOptions(wfont)).Width; return width; } - + //private static void copyAttributes(Font font, AttributedString str, int startIdx, int endIdx) //{ // str.addAttribute(TextAttribute.FAMILY, font.getFontName(), startIdx, endIdx); @@ -1098,7 +1098,7 @@ public void Test57973() IFont font = wb.CreateFont(); font.FontName = ("Arial"); font.FontHeightInPoints = ((short)14); - font.IsBold=true; + font.IsBold = true; font.Color = (IndexedColors.Red.Index); str2.ApplyFont(font); @@ -1660,5 +1660,102 @@ private void checkFormulaPreevaluatedString(IWorkbook readFile) } } + // bug 60197: setSheetOrder should update sheet-scoped named ranges to maintain references to the sheets before the re-order + [Test] + public virtual void bug60197_NamedRangesReferToCorrectSheetWhenSheetOrderIsChanged() + { + using (IWorkbook wb = _testDataProvider.CreateWorkbook()) + { + ISheet sheet1 = wb.CreateSheet("Sheet1"); + ISheet sheet2 = wb.CreateSheet("Sheet2"); + ISheet sheet3 = wb.CreateSheet("Sheet3"); + + IName nameOnSheet1 = wb.CreateName(); + nameOnSheet1.SheetIndex = wb.GetSheetIndex(sheet1); + nameOnSheet1.NameName = "NameOnSheet1"; + nameOnSheet1.RefersToFormula = "Sheet1!A1"; + + IName nameOnSheet2 = wb.CreateName(); + nameOnSheet2.SheetIndex = wb.GetSheetIndex(sheet2); + nameOnSheet2.NameName = "NameOnSheet2"; + nameOnSheet2.RefersToFormula = "Sheet2!A1"; + + IName nameOnSheet3 = wb.CreateName(); + nameOnSheet3.SheetIndex = wb.GetSheetIndex(sheet3); + nameOnSheet3.NameName = "NameOnSheet3"; + nameOnSheet3.RefersToFormula = "Sheet3!A1"; + + // workbook-scoped name + IName name = wb.CreateName(); + name.NameName = "WorkbookScopedName"; + name.RefersToFormula = "Sheet2!A1"; + + Assert.AreEqual("Sheet1", nameOnSheet1.SheetName); + Assert.AreEqual("Sheet2", nameOnSheet2.SheetName); + Assert.AreEqual("Sheet3", nameOnSheet3.SheetName); + Assert.AreEqual(-1, name.SheetIndex); + Assert.AreEqual("Sheet2!A1", name.RefersToFormula); + + // rearrange the sheets several times to make sure the names always refer to the right sheet + for (int i = 0; i <= 9; i++) + { + wb.SetSheetOrder("Sheet3", i % 3); + + // Current bug in XSSF: + // Call stack: + // XSSFWorkbook.write(OutputStream) + // XSSFWorkbook.commit() + // XSSFWorkbook.saveNamedRanges() + // This dumps the current namedRanges to CTDefinedName and writes these to the CTWorkbook + // Then the XSSFName namedRanges list is cleared and rebuilt + // Thus, any XSSFName object becomes invalid after a write + // This re-assignment to the XSSFNames is not necessary if wb.write is not called. + nameOnSheet1 = wb.GetName("NameOnSheet1"); + nameOnSheet2 = wb.GetName("NameOnSheet2"); + nameOnSheet3 = wb.GetName("NameOnSheet3"); + name = wb.GetName("WorkbookScopedName"); + + // The name should still refer to the same sheet after the sheets are re-ordered + Assert.AreEqual(i % 3, wb.GetSheetIndex("Sheet3")); + Assert.AreEqual("Sheet1", nameOnSheet1.SheetName); + Assert.AreEqual("Sheet2", nameOnSheet2.SheetName); + Assert.AreEqual("Sheet3", nameOnSheet3.SheetName); + Assert.AreEqual(-1, name.SheetIndex); + Assert.AreEqual("Sheet2!A1", name.RefersToFormula); + + // make sure the changes to the names stick after writing out the workbook + using (IWorkbook wb2 = _testDataProvider.WriteOutAndReadBack(wb)) + { + // See note above. XSSFNames become invalid after workbook write + // Without reassignment here, an XmlValueDisconnectedException may occur + nameOnSheet1 = wb2.GetName("NameOnSheet1"); + nameOnSheet2 = wb2.GetName("NameOnSheet2"); + nameOnSheet3 = wb2.GetName("NameOnSheet3"); + name = wb2.GetName("WorkbookScopedName"); + + // Saving the workbook should not change the sheet names + Assert.AreEqual(i % 3, wb2.GetSheetIndex("Sheet3")); + Assert.AreEqual("Sheet1", nameOnSheet1.SheetName); + Assert.AreEqual("Sheet2", nameOnSheet2.SheetName); + Assert.AreEqual("Sheet3", nameOnSheet3.SheetName); + Assert.AreEqual(-1, name.SheetIndex); + Assert.AreEqual("Sheet2!A1", name.RefersToFormula); + + // Verify names in wb2 + nameOnSheet1 = wb2.GetName("NameOnSheet1"); + nameOnSheet2 = wb2.GetName("NameOnSheet2"); + nameOnSheet3 = wb2.GetName("NameOnSheet3"); + name = wb2.GetName("WorkbookScopedName"); + + Assert.AreEqual(i % 3, wb2.GetSheetIndex("Sheet3")); + Assert.AreEqual("Sheet1", nameOnSheet1.SheetName); + Assert.AreEqual("Sheet2", nameOnSheet2.SheetName); + Assert.AreEqual("Sheet3", nameOnSheet3.SheetName); + Assert.AreEqual(-1, name.SheetIndex); + Assert.AreEqual("Sheet2!A1", name.RefersToFormula); + } + } + } + } } -} \ No newline at end of file +} diff --git a/testcases/ooxml/XSSF/Streaming/TestSXSSFBugs.cs b/testcases/ooxml/XSSF/Streaming/TestSXSSFBugs.cs index 90ad09bae..211817782 100644 --- a/testcases/ooxml/XSSF/Streaming/TestSXSSFBugs.cs +++ b/testcases/ooxml/XSSF/Streaming/TestSXSSFBugs.cs @@ -1,8 +1,10 @@ using NPOI.SS.UserModel; using NPOI.SS.Util; +using NPOI.Util; using NPOI.XSSF; using NPOI.XSSF.Streaming; using NUnit.Framework; +using System.IO; using TestCases.SS.UserModel; namespace TestCases.XSSF.Streaming @@ -24,7 +26,7 @@ public TestSXSSFBugs() [Ignore("Reading data is not supported")] public override void Bug57798() { /* Reading data is not supported */ } [Test] - public void Tug49253() + public void Bug49253() { IWorkbook wb1 = new SXSSFWorkbook(); IWorkbook wb2 = new SXSSFWorkbook(); @@ -57,5 +59,30 @@ public void Tug49253() wb1.Close(); wb2.Close(); } + + // bug 60197: setSheetOrder should update sheet-scoped named ranges to maintain references to the sheets before the re-order + [Test] + override + public void bug60197_NamedRangesReferToCorrectSheetWhenSheetOrderIsChanged() + { + try + { + base.bug60197_NamedRangesReferToCorrectSheetWhenSheetOrderIsChanged(); + } + catch (RuntimeException e) + { + var cause = e.InnerException; + if (cause is IOException && cause.Message == "Stream closed") + { + // expected on the second time that _testDataProvider.writeOutAndReadBack(SXSSFWorkbook) is called + // if the test makes it this far, then we know that XSSFName sheet indices are updated when sheet + // order is changed, which is the purpose of this test. Therefore, consider this a passing test. + } + else + { + throw e; + } + } + } } }