Summary: | Shifting rows with comment result - Unreadable content error and comment deletion | ||
---|---|---|---|
Product: | POI | Reporter: | yevsjunk |
Component: | XSSF | Assignee: | POI Developers List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | major | CC: | merlusha |
Priority: | P2 | ||
Version: | 3.11-FINAL | ||
Target Milestone: | --- | ||
Hardware: | PC | ||
OS: | All | ||
Bug Depends on: | |||
Bug Blocks: | 55814, 57828 | ||
Attachments: |
Test file - before running the code
Broken excel 2010 file Proposed patch for comments above shifted rows |
Description
yevsjunk
2014-01-16 10:04:58 UTC
Created attachment 31215 [details]
Test file - before running the code
I have added a test-case in r1563587 and r1563588 which runs this, I fixed a problem with comment-refs which only affected the in-memory state of comments, as I don't have a Windows-machine with Excel at hand to verify/reproduce the problem, so I am not sure if it is fixed in latest trunk or not... Thanks, I will test it latter. Meanwhile, I made some work around in shiftRows method of XSSFSheet... Anyway, will the next POI release include the fix to the issue? This bug persists in 3.10 final release and 3.11 dev. Is there a target release for a fix ? Best regards. In: org.apache.poi.xssf.usermodel.XSSFSheet I believe line: 2451 if(rownum < startRow || rownum > endRow) continue; Should be above: 2439 if(sheetComments != null){ The exception I was getting started here: 2445 ref = new CellReference(rownum + n, ref.getCol()); Looks as though it's trying to update the comments table with the new cell references after the rows are shifted, but the iterator is over all rows and the check for the shifted range is after this block. Therefore, an attempt could be made for a negative row value. Fixed the issue for me anyway and hope it helps. In: org.apache.poi.xssf.usermodel.XSSFSheet I believe line: 2451 if(rownum < startRow || rownum > endRow) continue; Should be above: 2439 if(sheetComments != null){ The exception I was getting started here: 2445 ref = new CellReference(rownum + n, ref.getCol()); Looks as though it's trying to update the comments table with the new cell references after the rows are shifted, but the iterator is over all rows and the check for the shifted range is after this block. Therefore, an attempt could be made for a negative row value. I would assume if you do not get an exception the workbook may corrupt. Fixed the issue for me anyway and hope it helps. In the shiftRows method when it adjusts comments/rowheight it does all rows(!) in the current sheet. It doesn't always cause exception or error when opening xlsx, but always makes worse all of comments. For example: if I call it: sheet.shiftRows(3, 4, 20, true, false); then it shift all comments on 0, 1, and 2 rows too instead of leave in peace. It's a simply, but annoying bug. Please fix it asap! Created attachment 32431 [details]
Broken excel 2010 file
I was able to reproduce this bug with Excel 2010. (poi 3.11 Final) My further investigation shows that problem lies in following code: XSSFSheet.java:2567 if(sheetComments != null){ //TODO shift Note's anchor in the associated /xl/drawing/vmlDrawings#.vml CTCommentList lst = sheetComments.getCTComments().getCommentList(); for (CTComment comment : lst.getCommentArray()) { String oldRef = comment.getRef(); CellReference ref = new CellReference(oldRef); if(ref.getRow() == rownum){ ref = new CellReference(rownum + n, ref.getCol()); comment.setRef(ref.formatAsString()); sheetComments.referenceUpdated(oldRef, comment); } } } I looked at /xl/drawing/vmlDrawings#.vml and note's anchor was not shifted. So I updated this file manually and was able to open excel file without problem. Can somebody implement this TODO?? I believe we should add above: if(sheetComments != null){ if(n>0 & rownum < startRow) continue; if(n<0 & rownum < startRow+n) continue; This solves problem only if comments are above shifted rows. However if comments are below shifted rows the problem is present. CellReference for all comments which are below shifted rows are set up to the last row. Attached you can find patch with included fix for comments which are above shifted rows. Created attachment 32517 [details]
Proposed patch for comments above shifted rows
I gave this a try and ended up changing the implementation of XSSFSheet.shiftRows() quite a bit, see r1666843. The current code did not handle a few things well, e.g. removing comments in overwritten rows and cases where comments are moved around. Moving the comments while still iterating caused cases where comments where moved multiple times, messing up things. Also XSSFComment.setRow() was prone to a xmlbeans bug when resetting row-nunmbers. A few new test-cases now verify the most common cases, please report new bugs with sample code if there are still cases that are handled incorrectly. |