Bug 59687

Summary: XSSFSheet.RemoveRow doesn't handle row gaps properly when removing row comments (typo)
Product: POI Reporter: Greg Woolsey <greg.woolsey>
Component: XSSFAssignee: POI Developers List <dev>
Severity: normal    
Priority: P2    
Version: 3.15-dev   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: simple file that causes trouble when removing rows with comments and missing rows above.

Description Greg Woolsey 2016-06-09 17:30:17 UTC
Created attachment 33936 [details]
simple file that causes trouble when removing rows with comments and missing rows above.

The attached file was created with Excel 2016.  It has only one cell with data, A4, with a comment on that cell.  The XML for the sheet shows only that one row defined.

The following test method fails, because XSSFSheet.removeRow(row) uses the physical offset, not the logical row #, when looking for row comments to remove.  If there are undefined rows (gaps in the physical sequence) these are different, and the comments remain after the row is deleted.

The fix is to store the rowNum from the row being deleted, and use that rather than the physical idx in the comment removal loop in XSSFSheet.removeRow(row).

I'd do this as a patch, but I have another patch pending, and would rather not revert and reapply for so small a change.  If that's what it takes, I'll do it, though.

Add this test to TestXSSFSheetShiftRows, and the attached file to test-data/spreadsheet:

    public void testRemoveRowWithCommentAndGapAbove() throws IOException {
        final XSSFWorkbook wb = XSSFTestDataSamples.openSampleWorkbook("removeRowWithCommentBug.xlsx");
        final XSSFSheet sheet = wb.getSheetAt(0);

        // comment exists
        CellAddress commentCellAddress = new CellAddress(3, 0);
        assertEquals("Wrong starting # of comments",  1, sheet.getCommentsTable(true).getNumberOfComments());
        assertEquals("There should not be any comments left!",  0, sheet.getCommentsTable(true).getNumberOfComments());
Comment 1 Javen O'Neal 2016-09-22 07:52:36 UTC
Thanks for catching this error, writing a unit test, and suggesting a fix!

Applied in r1761860 and r1761861. Will be included in POI 3.16 beta 1.